espruino / Espruino

The Espruino JavaScript interpreter - Official Repo
http://www.espruino.com/
Other
2.73k stars 741 forks source link

special character on firmware 2v19.62 (seen on Bangle.Js2) #2429

Closed nxdefiant closed 7 months ago

nxdefiant commented 7 months ago

The following snippet containing a german umlaut fails on firmware 2v19.62 on Bangle.Js2:

const s = "\u00FC";
const o = {};
o[s] = undefined;
print(o);

What I get is:

{ F 8 F 8 "F 8

and a reboot

gfwilliams commented 7 months ago

Thanks - I'll look into it. Just tested and this happens in 2v19 too.

How did you hit this? was it with normal code from the app store (eg with German translations), or something you were working on yourself?

Just to add that the following works ok, because the string is then not Unicode.

const s = "\xFC";
const o = {};
o[s] = undefined;
print(o);
nxdefiant commented 7 months ago

I hit this while testing waypoints for espruino/BangleApps#3099 The waypoint with umlaut was uploaded with the interface.html. After modifying the file with the waypoints editor on the Banglejs I hit that bug in https://github.com/espruino/BangleApps/blob/master/apps/waypoints/waypoints.app.js#L38

Changing the waypoints.json on the device also changes the umlaut in the file from "ü" (which works) to "\u00FC", somewhere between the require('Storage').readJSON() and writeJSON() call. Might also be a bug?

gfwilliams commented 7 months ago

Changing the waypoints.json on the device also changes the umlaut in the file from "ü" (which works) to "\u00FC", somewhere between the require('Storage').readJSON() and writeJSON() call. Might also be a bug?

Yes, you mean like:

var s = "\xFC";
require("Storage").writeJSON("tmp",s);
var b = require("Storage").readJSON("tmp");

trace(s); // #244[r1,l1] String [1 blocks] "\xFC"
trace(b); // #238[r1,l1] UTF8String  #237[r1,l1] String [1 blocks] "\xC3\xBC"

This is a tricky one as if we're actually writing JSON, JSON.stringify is supposed to always write \u00FC which then gets interpreted as unicode.

But... I feel like in this case it might be worth ensuring that writeJSON doesn't quite write JSON, and instead just writes something that makes sense to Espruino. It could still be read by parseJSON on desktop.

gfwilliams commented 7 months ago

Just fixed that one with 5b06cd629e1156fc73a03cbbc0e95af000b0eca3 - this was actually causing some issues with Gadgetbridge, where bitmap-ified text was getting written using unicode escape sequences.

nxdefiant commented 7 months ago

Cool, thank you very much.

nxdefiant commented 7 months ago

@gfwilliams I fear this introduced a new problem: If you upload the following as test.json

{"name":"\xFC"}

and try to download it with e.g.

Util.readStorage('test.json', data=>{
  console.log(data);
  console.log(JSON.parse(data));
});

you will get

Uncaught SyntaxError: Bad escaped character in JSON at position 10 (line 1 column 11)

because the string is: '{"name":"\\xFC"}'

seen in multiple interface.html of the Bangle.js app store.

gfwilliams commented 7 months ago

Argh, ok. That's a pain - for some reason I really thought JSON handled parsing \x## escapes even if .stringify didn't create them. I don't think we can do too much about what we save on Espruino, as I don't think the issue with saving non-unicode as unicode can really be fixed any other way (also it seems crazy using twice as much space to store images/etc on the Bangle).

So I'm open to ideas, but I think we could do:

Any thoughts?

nxdefiant commented 7 months ago

+1 for option 1 with additional compression.

Option 3: I see about 22 calls to Util.readStorage() in the Bangle.js app loader, about 16 of them do pass the data to JSON.parse(), so thats definately a pattern. A new function Util.readStorageJSON(filename, defaultValueIfFIleDoesNotExist, callback) would be handy, but that would require an update to a dozen or so interface.html, doable.

gfwilliams commented 7 months ago

Yes, that sounds good. Just done this with https://github.com/espruino/BangleApps/pull/3110

Didn't spot your default value request but I made it return undefined if it can't parse, which is easy enough to pick up - a lot of code already did parse() || [] which should work fine here

nxdefiant commented 7 months ago

Awesome, thanks.