-
Notifications
You must be signed in to change notification settings - Fork 278
Reencode submission source in UTF-8 when displaying in monaco #3221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
vmcj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss this in person, this might be a proper fix but I'm not sure if we should change submissions for participants (out of principle)
So mostly blocking merging now.
|
This is not changing submissions. It is changing the display of submissions. The submissions it changes would not be displayed previously, see bug and screenshot |
1 similar comment
|
This is not changing submissions. It is changing the display of submissions. The submissions it changes would not be displayed previously, see bug and screenshot |
As we'll meet this week anyway, I don't see a reason not to discuss in person so I'll wait with merging. |
Wrongly read this as that we do this on import. |
|
I'll postpone merging this until after a small in-person discussion about #3217 (comment) |
e2e68fc to
4c25407
Compare
15145f6 to
7fda4e8
Compare
Try to re-encode the submission source if it currently is not valid in UTF-8. Add some extra safeguard by forcing another sanity check re-encode from UTF-8 to the original encoding, which filters out binary blobs and verifies that the re-encoding is non-destructive.
7fda4e8 to
224dc7e
Compare
Try to reencode the submission source if it currently is not valid in UTF-8. Add some extra safeguard by forcing another sanity check reencode from UTF-8 to UTF-8, which filters out binary blobs.
Before:

After:

Blobs still filtered out:

Fixes #3217