1742168 - "NS_ERROR_ILLEGAL_VALUE" error when searching on npmjs.com for certain packages

Status: RESOLVED FIXED

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

Description

[Ksenia Berezina [:ksenia]]

This was initially reported in https://github.com/webcompat/web-bugs/issues/93773

To reproduce:

1. Visit https://www.npmjs.com/
2. Enter "firebase" in the search box (don't push enter as it will redirect to a different page)
3. Click on the first result in the dropdown list

Expected
Redirect to https://www.npmjs.com/package/firebase
Actual:
"error loading page" error

A similar issue was addressed in https://bugzilla.mozilla.org/show_bug.cgi?id=1542673, but seems that the problem still persists.

P.S. This error also results in "Weekly Downloads" graph not rendered on https://www.npmjs.com/package/firebase page

Comment 1

[Ksenia Berezina [:ksenia]]

Probably worth mentioning that if I increase browser.history.maxStateObjectSize , the search starts working.

Comment 2

[Ksenia Berezina [:ksenia]]

Hi Olli, wonder if you could take a look at this?

Comment 3

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

Not sure what could be done here other than increasing the max state object size.

Could you check what is the maximum size in other browsers?

Comment 4

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

Chrome allows up to around 0.5GB states. That is a lot.

var s = new Array(530000000).join("a"); history.pushState(s, ""); seem to work in Chrome
var s = new Array(540000000).join("a"); history.pushState(s, ""); fails.

It is easy to hang Chrome when pushing lots of data to session history and trying to close the browser. It is the whole browser which hangs, not only a renderer process.

Comment 5

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

farre, what would happen in our session store code if we allowed very large states?
Right now we have https://searchfox.org/mozilla-central/rev/55a826a9ef74e92988e56cd9615d4fc6a470695e/modules/libpref/init/all.js#3769

Comment 6

[Chris Peterson [:cpeterson]]

Olli says Firefox's current session store limit is 2 MB. Should we increase it? Nika says the IPC message size limit is 256 MB, but she wouldn't want to increase the limit above 64 MB.

Comment 7

[Ksenia Berezina [:ksenia]]

Chrome allows up to around 0.5GB states. That is a lot.

Thanks for checking it Olli. I've tried the test case from the original issue and increased the object up to 50MB, but couldn't make it fail in Chrome. Didn't realize it's that much.

Comment 8

[Andreas Farre [:farre]]

We have similar limits for storage and form fields in session store prefs.

Form field collection and storage collection are both bound by IPC message sice since forms are collected in the content child process, and storage on the PBackground actor. With SHIP I guess we actually do history collection without involving IPC.

I think that it'd be good if we could add a browser.sessionstore.dom_history_state_limit that's separate from browser.history.maxStateObjectSize. We should probably make them have the same value initially, but it would be good to be able to allow pushState to push more state than we actually would save in the session store.

Comment 9

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

Wouldn't that be a bit odd. Then when restoring the data would be broken

Comment 10

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

(browser.sessionstore.dom_storage_limit is surprisingly low)

Comment 11

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

I can't now get the error from the STR, but https://www.npmjs.com/package/firebase has the issue.
in Chrome I can see that history.state has a massive object in it. If it is JSON.stringified, it is over 3.1MB. It seems to contain all the version numbers, and very long links to tarballs and some avatars and what not.

Ksenia, do you think you could contact the site and hint that they are using history.state in an unusual way?

I think I'll try increase out limit to 16MB. That is already high, but not crazy high.

Comment 12

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

Attachment: Bug 1742168, increase maxStateObjectSize to 16MB and make the pref a StaticPref, r=peterv

Keeping the pref as signed, since the existing code explicitly handles that case, so someone may have -1 as the pref value.

Comment 13

[Ksenia Berezina [:ksenia]]

Following up on comment 11, I've reached out to them on GitHub.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0de3705203a4
increase maxStateObjectSize to 16MB and make the pref a StaticPref, r=peterv

Comment 15

[Atila Butkovits]

Backed out for causing failures at browser_multiple_pushState.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/42a64fe0c062cd296da3ab22a14f938fd2663caa

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=aIi7NgLFQJ2rWyNOT6W0jg.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=0de3705203a4fb7d62b5c016939af401b5eee51b

Failure log: https://treeherder.mozilla.org/logviewer?job_id=359705730&repo=autoland&lineNumber=3610

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc462c61e604
increase maxStateObjectSize to 16MB and make the pref a StaticPref, r=peterv

Comment 17

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/cc462c61e604