Bug 238662 - REGRESSION (iOS 15.4): JSON.parse fails on sketchfab.com
Summary: REGRESSION (iOS 15.4): JSON.parse fails on sketchfab.com
Status: RESOLVED DUPLICATE of bug 237180
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari 15
Hardware: iPhone / iPad iOS 15
: P2 Normal
Assignee: Alexey Shvayka
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-04-01 02:43 PDT by paul
Modified: 2022-06-15 09:03 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description paul 2022-04-01 02:43:43 PDT
Those url ios Safari 15.4 

https://sketchfab.com/models/df93411c475c4e4eb450f71437b5ad0d/embed?autostart=1
https://sketchfab.com/models/9c2a37f7436949b1b3081ff549f7dc14/embed?autostart=1

gives "JSON.parse error"
where it works on all other browser

22k users of sketchfab had the bugs, here I reproduced on an "iphone X 15.4" hardware
Comment 1 Alexey Proskuryakov 2022-04-01 14:46:00 PDT
I cannot reproduce this with Mac Safari.

It seems likely that the issue in not with JSON.parse, but with the string content that is is being parse. Would you be willing to isolate the issue further, to see what is going on?

Also, is this a new issue in Safari 15.4 that didn't happen in Safari 15.3?
Comment 2 paul 2022-04-02 01:38:01 PDT
it doesn't happen in any other browser nor previous safari 15.3 or any before.
(we detected that using sentry and it reports browser version)

We did try to isolate the json and feeded it to a simple json parse to get a minimal repro case without being able to reproduce the bug.
Comment 3 paul 2022-04-02 01:51:46 PDT
isolated that doesnt' repro: https://jsfiddle.net/sketchfab/rgq4Lafz/4/
Note that it seems to happen on relatively "big json".

There is much more than just than happening (String.fromCharCode, slice from uint8 array, etc) before that json parse, but adding it the whole stack might take much more longer. 

Hopefully checking changelog or git bisect since 15.3 might gives pointers to a changes that might have produce that faster ?
Comment 4 paul 2022-04-02 01:52:25 PDT
isolated that doesnt' repro: https://jsfiddle.net/sketchfab/rgq4Lafz/4/
Note that it seems to happen on relatively "big json".

There is much more than just than happening (String.fromCharCode, slice from uint8 array, etc) before that json parse, but adding it the whole stack might take much more longer. 

Hopefully checking changelog or git bisect since 15.3 might gives pointers to a changes that might have produce that faster ?
Comment 5 Alexey Proskuryakov 2022-04-02 11:27:21 PDT
When loading these links on iPhone, I observe that "Loading 3D model" progress bar gets stuck early on. I do not see "JSON.parse error" in Web Inspector console or anywhere else.

Yes, bisecting changes will be a better way assuming that folks working on this can reproduce. Given that I could reproduce something being clearly wrong, we do have a thread to pull, but if you can isolate it in any way on your end, that will be helpful.

This reminds bug 231138 a little, but that one was strictly about HTML, and it was fixed in iOS 15.2.
Comment 6 Radar WebKit Bug Importer 2022-04-02 11:27:31 PDT
<rdar://problem/91203128>
Comment 7 paul 2022-04-04 02:52:07 PDT
- On sketchfab.com production servers all errors are catched and reported to "sentry" saas tool only. (no log either)

- On this model it happens Bug happen "randomly" on ios safari 15.4:
https://sketchfab.com/3d-models/view-beyond-2aaf41188149422f97f474681f6b47a7
(sometimes parsing succeed, all data always the same though afaict)

Doing a minimal repro is definitely quite a hard task, random makes it even harder
Comment 8 paul 2022-04-04 03:01:51 PDT
if it helps json parse error gives really strange token error reported:

SyntaxError: JSON Parse error: Unrecognized token 'ô'
SyntaxError: JSON Parse error: Unrecognized token '»'
SyntaxError: JSON Parse error: Unrecognized token 'Í'

amongst many other, which makes it looks like it reads out of bounds memory when parsing.

As it happens that we produce the json string from String.fromCharCode, slicde from uint8 array, and that it appends only on "big json" fails, it also point to that memory allocation/bounds problem.
Comment 9 paul 2022-04-04 03:11:53 PDT
(and yes those character are definitely not in the json sent)

while trying to do a repro, something that might be a culprit too is that code relies on slice doing the out of bounds check, so maybe regression is in there


///////////////////

var content = '';
var raw = new Uint8Array(msg);
var step = 65535;
for (var i = 0; i < msg.length; i += step) {
    content = content + String.fromCharCode.apply(null, raw.slice(i, i + step));
}
JSON.parse(content);

///////////////////
Comment 10 paul 2022-04-05 03:32:14 PDT
Just to let you know that I was not able to do a minimal repro case, despite trying to mirror a lot of the code.

We have now more 40k user event on this bug, and only on latest safari.
Works on every other browser and version/
Comment 11 paul 2022-04-05 23:58:40 PDT
Might be related, as we produce the json from wasm:
https://bugs.webkit.org/show_bug.cgi?id=237180
Comment 12 paul 2022-04-08 01:57:19 PDT
We fixed on sketchfab.com by shipping wasm with no optimisations.
So all url here won't crash anymore.
I'm closing the bug as it's a duplicate of the bug listed and fixed I linked above

*** This bug has been marked as a duplicate of bug 237180 ***
Comment 13 Alexey Shvayka 2022-04-11 14:52:47 PDT
(In reply to paul from comment #12)
> We fixed on sketchfab.com by shipping wasm with no optimisations.
> So all url here won't crash anymore.
> I'm closing the bug as it's a duplicate of the bug listed and fixed I linked
> above
> 
> *** This bug has been marked as a duplicate of bug 237180 ***

Hey Paul, we so much appreciate you filing & updating the issue. WASM optimiser bugs are utmost important for us.

If there is any way you could provide us with a repro so we can ensure it's the same issue as https://bugs.webkit.org/show_bug.cgi?id=237180, please let us know. Like some sort of query parameter or HTTP header that loads optimized WASM bundle.
Comment 14 paul 2022-04-11 23:29:50 PDT
Hi, 

Thanks for the follow-up, as unoptimized bundle -O0 is way bigger than -O2 compiled wasm, we serve it only to safari 15.4, you can still reproduce the bug if user agent is not safari 15.4.
Comment 15 Saam Barati 2022-06-15 09:03:51 PDT
Does Safari 15.5 fix this issue for you?