Skip to content
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

JSON vs Javascript lists #2

Closed
anforowicz opened this issue Jun 23, 2020 · 1 comment · Fixed by #13
Closed

JSON vs Javascript lists #2

anforowicz opened this issue Jun 23, 2020 · 1 comment · Fixed by #13

Comments

@anforowicz
Copy link
Collaborator

What is your plan for handling scenarios where the first 1024 bytes look like a beginning of list?

[ 1, 2, 3, "1024 bytes like that", ....

Such a prefix can be extended to form valid JSON and invalid Javascript:

[ 1, 2, 3, "foobar" ]

or it can be extended to form valid Javascript and invalid JSON:

[ 1, 2, 3, "foobar" ].map(x => console.log(x))

I assume that step 14 works with the first 1024 bytes (otherwise we increase further the latency of making the first bytes available in the renderer process). OTOH, step 14 says "response's body" rather than "bytes", so maybe this is a wrong assumption.

PS. Is there a Mozilla bug we can follow to see if the proposed algorithm works well in practice? If Javascript sniffing turns out to be robust against real websites, then the proposed algorithm seems like an important improvement in coverage of CORB.

@annevk
Copy link
Owner

annevk commented Jun 24, 2020

Ah, I guess I should add another step before step 14 that waits for the complete response to make that more clear. Defining a parser for partial input seemed rather involved, error-prone, and hard to standardize. Also, it might not necessarily be that bad as depending on your architecture you could perhaps transfer the output of the parser across the process boundary. But also, the slight performance hit should give sites even more reason to migrate to using the correct MIME type and such.

https://bugzilla.mozilla.org/show_bug.cgi?id=1532642 tracks this, but it's not a priority currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants