Use json.Marshal instead of strconv.Quote to correctly support unicode #2509
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Thanks for opening this and raising the issue. What testing have you done on this so far and on what platforms? Thanks! |
|
@leaanthony I was only able to test this on Mac, but after making this change all the weird unicode issues I was having went away. |
I checked the basic behavior, go side responds with a unicode smiley, the frontend prints it as an escaped string. If I apply @joshbuddy's changes the unicode is parsed properly on the frontend. I'm currently on Ubuntu 22.04 LTS master: PR applied: |
| f.ExecJS(`window.wails.Callback(` + strconv.Quote(message) + `);`) | ||
| escaped, err := json.Marshal(message) | ||
| if err != nil { | ||
| panic(err) |
There was a problem hiding this comment.
Can we make it not panic here? That would look like a crash for a user and be really hard to locate.
There was a problem hiding this comment.
What would you do though? If your callbacks fail then you have a pretty catastrophic error. I'm trying to think of an alternative
There was a problem hiding this comment.
That is a good question! Let me think about it.. In the meantime I don't think we should block merging this.
There was a problem hiding this comment.
As far as I can tell, a panic is appropriate here because json.Marshal should never return an error in response to marshaling a string, so if it did that, I would consider that to be unspecified behavior.
On a separate note, I notice that json.Marshall will do a certain amount of html escaping on a string. I can't think how it would be a problem, but worth noticing. See: https://pkg.go.dev/encoding/json#Marshal
String values encode as JSON strings coerced to valid UTF-8, replacing invalid bytes with the Unicode replacement rune. So that the JSON will be safe to embed inside HTML <script> tags, the string is encoded using HTMLEscape, which replaces "<", ">", "&", U+2028, and U+2029 are escaped to "\u003c","\u003e", "\u0026", "\u2028", and "\u2029". This replacement can be disabled when using an Encoder, by calling SetEscapeHTML(false).
|
@joshbuddy - could you please add an entry to the changelog in |
joshbuddy
force-pushed
the
bugfix/unicode-encoding
branch
from
0707c1b to
144e0f3
Compare
|
@leaanthony done |
I came across a bug where unicode values such as 🫣 would not get correctly escaped. This is due to using
strconv.Quotewhich would encode this value as "\U0001fae3" which is not a valid js string. This would, in turn, get turned into "U0001fae3" in the frontend.To correctly represent these values we need to use the new js format for multi-rune characters "\u{0001fae3}"[1], but I thought, instead of figuring out the correct quoting strategy, why not just use
json.Marshalhere instead. Certainly open to finding a different approach if you don't think this is a good idea.Also decided to panic on error here as this error should never come up, and I saw similar panics in the code encountering things like this.
[1] https://exploringjs.com/es6/ch_unicode.html#sec_escape-sequences