1542673 - npmjs search broken for some results, NS_ERROR_ILLEGAL_VALUE reported to console

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P3)

Description

[Karl Dubost💡 :karlcow]

This is https://webcompat.com/issues/27973
(not sure this is the right component)

Steps to reproduce

1. https://www.npmjs.com/search?q=mysql
2. click on mysql (the first result)
3. See the error message

When we click, an xhr request is made to
https://www.npmjs.com/package/mysql

This download a JSON file where one of the values is huge.

the "readme" in this JSON is 356629 characters long.

When I shorten the value of the readme, it loads without issue.

Comment 1

[Jan de Mooij [:jandem]]

If I download the JSON file manually (a 365K file) and JSON.parse it in the JS shell, everything works fine.

The exception is thrown somewhere on the DOM side.

Comment 2

[Jan de Mooij [:jandem]]

A debug build shows we're failing this check:

  NS_ENSURE_TRUE(scSize <= (uint32_t)maxStateObjSize, NS_ERROR_ILLEGAL_VALUE);

Here: https://searchfox.org/mozilla-central/rev/6db0a6a56653355fcbb25c4fa79c6e7ffc6f88e9/docshell/base/nsDocShell.cpp#10940

And indeed, changing the browser.history.maxStateObjectSize pref does fix this.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

I'm not seeing any limitation in Chrome. Pushing very state object just ends up crashing the child process in Chrome eventually. And we have very small limit
https://searchfox.org/mozilla-central/rev/6db0a6a56653355fcbb25c4fa79c6e7ffc6f88e9/modules/libpref/init/all.js#5181-5182

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

I wonder how sessionstore should work with massive jsons.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

(Testing Chrome is a bit hard since its session history seems to be rather broken when one uses pushState)

Comment 6

[Jan de Mooij [:jandem]]

Short term should we just bump the limit to work around this?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Probably, but need to ensure nothing catastrophic happens with sessionstore.
Perhaps Mike knows that part better? Do we somehow limit the size of data in session store?

Comment 8

[Mike de Boer [:mikedeboer]]

Well, it'll effectively grow the entries array per tab and increase the potential parse-time of the JSON when read from disk. We don't cap or limit the amount of entries we keep, nor the total size of data.

So nothing catastrophic will happen, but I would advise to keep browser.history.maxStateObjectSize, but set it to something sane that would make it unlikely to crash the process.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

ok, so the tricky question is then what would be a reasonable value.
But then, we don't really limit how many times pushState can be called, so the storage is already unlimited in some sense.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

ah, yes, there is limit 50. So currently state objects may store < 32768000 bytes.

Based on nothing, perhaps we could increase the per state limit to 2097151 ((1 << 21) - 1),
altogether max 104857550

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1542673, increase history.state size limit, r=qdot

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

I think in the future (I guess once we have the new sessionstore implementation), we should probably strip session history entries which contain too large state objects. Just store the initial page load for the site.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49205d157f4d
increase history.state size limit, r=qdot

Comment 14

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/49205d157f4d