-
Notifications
You must be signed in to change notification settings - Fork 137
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
percent-encode ' in queries of URLs with special schemes #348
Comments
In both Chrome and Firefox here, the |
Exactly. We need to add it to the list of characters to be escaped if the URL has a special scheme like http. |
Ah, sorry I misunderstood your original issue. |
cc @valenting |
I think this is a good move. Since it's the behaviour already present in Chrome and Firefox, the chance of breaking sites is very low. Let's do it. |
I'm wondering why Referring to the new language that was added:
I tested query parsing for all ASCII characters in Chrome and Firefox: Special URL: new URL("http://example.com/? !\"$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~").search Chrome:
Firefox:
(Both encode <0x21, 0x22, 0x27, 0x3C, 0x3E, >0x7E — agreement with each other and spec.) Non-special URL: new URL("ssh://example.com/? !\"$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~").search Chrome:
Firefox:
Further experimentation shows that Chrome encodes <0x20 while Firefox encodes <0x21. No other ASCII characters are encoded. So it seems that there's nothing special about (Having said all this, it looks like the path and other components are similarly not very well encoded for non-special URLs so there isn't much point fixing this for query unless it's fixed everywhere.) |
This is needed for compatibility, and it makes some injection attacks harder by preventing ' from being sent to the server.
Chrome, Firefox, and Safari all have this behavior. I haven't tested Edge or IE. This can be verified as simply as this:
alert(new URL("http://host/pa'th?qu'ery#fra'gment"));
alert(new URL("asdf://host/pa'th?qu'ery#fra'gment"));
The text was updated successfully, but these errors were encountered: