-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Update section 7.7.4 - History notes to call out abuse of replaceState in addition to pushState #982
Comments
Wouldn't that be a more general OOM issue and not necessarily a history thing? I guess it might still be worth to call out there. |
@beidson, I think this issue betrays a bit of a misunderstanding of what that section is about. It's specifically talking about the "attack" where the user's back button stops working, due to excessive calls to pushState(). And it's recommending that browsers provide two "back modes": one that ignores pushState states, and one that doesn't. You seem to be talking about a different type of attack, involving something like large payloads to cause high memory usage. That isn't what this section is about, and I tend to agree with @annevk that it falls under the more general clause in the spec:
|
There's clearly a general OOM issue covered by other clauses, (like and 2.2.1 and 8.1.3.5), but this is also specifically about History. Clarification to follow.
I understand precisely what the current section is currently about. I'm simply arguing replaceState also deserves a callout in the section. There's not many web APIs that give JS direct control browser chrome. pushState is one of them, but replaceState is as well. Calling that out specifically here isn't so much for the UA developers as it is for JS authors who are being good citizens by trying to scour the spec for concerns that they should have. And that concern is not purely theoretical. :) |
Then we must be misunderstanding each other. replaceState doesn't have any impact on back button behavior. |
For context, this issue stems from the discussion at https://bugs.webkit.org/show_bug.cgi?id=156115 |
I think we could definitely add a new paragraph making it explicit that 2.2.1 etc. apply here, and maybe pointing out some of the dangers or providing advice. I just don't think expanding the existing note is really applicable, since it's about back button behavior, which doesn't apply to replaceState. |
For clarity, when I said "also deserves a callout in the section", I meant "in section 7.7.4", not in "the specific paragraph talking about replaceState.
Just for clarity, this is what I was asking.
I did not mean to suggest that the replaceState concern was the same as the pushState concern. |
Ah OK! Re-reading your OP I apologize; I was reading suggestions that weren't there. We can work this up, for sure. I'll try to work on it later today. |
Fixes #982. The "a user agent could ignore calls" is moved from the non-normative implementation notes to the normative algorithm, and the implementation notes are updated with a pointer to this.
Fixes #982. The "a user agent could ignore calls" is moved from the non-normative implementation notes to the normative algorithm, and the implementation notes are updated with a pointer to this.
Fixes whatwg#982. The "a user agent could ignore calls" is moved from the non-normative implementation notes to the normative algorithm, and the implementation notes are updated with a pointer to this.
Update section 7.7.4 - History notes to call out abuse of replaceState in addition to pushState
There is a non-normative but still quite useful section on "Implementation notes for session history" -
https://html.spec.whatwg.org/multipage/browsers.html#history-notes
It specifically suggests that browsers disallow abuse of browser's session history facilities, but only calls out pushState by name.
I suggest it call out replaceState by name, as high frequency calls to replaceState - especially with large payloads - are just as abusive of the session history as replaceState can be.
I seem to recall this was mentioned years ago when the state APIs were under development with their initial implementations, but somehow mentioning replaceState was overlooked.
The text was updated successfully, but these errors were encountered: