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

Percent encode NULLs in fragments #440

Closed
rmisev opened this issue May 23, 2019 · 6 comments · Fixed by #486
Closed

Percent encode NULLs in fragments #440

rmisev opened this issue May 23, 2019 · 6 comments · Fixed by #486

Comments

@rmisev
Copy link
Member

rmisev commented May 23, 2019

Currently URL parser removes NULL characters from fragments. But is this really needed?
My tests show that NULLs are percent encoded in most browsers (I tested URLs "https://example.com/#abc\u0000xyz" and "non-spec://example.com/#abc\u0000xyz"):

Browser Special URL's hash Non-special URL's hash
Chrome #abcxyz #abc%00xyz
Edge, IE #abc #abc
Firefox #abc%00xyz #abc%00xyz
Safari #abc%00xyz #abc%00xyz

So NULLs are removed in Chrome in special URLs only. In the Edge and IE a NULL character
denotes the end of string.

One reason, why Chrome removes NULLs, I found in the source code:
https://chromium.googlesource.com/chromium/src/+/refs/tags/76.0.3803.1/url/url_canon_etc.cc#304

  for (int i = ref.begin; i < end; i++) {
    if (spec[i] == 0) {
      // IE just strips NULLs, so we do too.
      continue;
    }

But as we see NULLs in Chrome and IE (and Edge) are handled differently.

I suggest to follow Firefox, Safari and percent encode NULLs in fragments.

@annevk
Copy link
Member

annevk commented May 24, 2019

Okay, so this is specifically about removing the U+0000 branch from https://url.spec.whatwg.org/#fragment-state.

This was last changed five years ago in https://www.w3.org/Bugs/Public/show_bug.cgi?id=27252 but no tests were added specifically for U+0000.

@mikewest @sleevi thoughts for Chrome?

@sleevi
Copy link

sleevi commented May 24, 2019

That seems reasonable to me, especially since Safari and Firefox are shipping. Perhaps @ericlaw1979 knows who to redirect this to for IE/Edge for any compat concerns?

@mikewest
Copy link
Member

I agree with @sleevi.

@ericlaw1979
Copy link

IE/Spartan are extremely unlikely to change this.

Vis-a-vis Chrome, I'm not sure what "special URLs" include, but would this change the behavior of URLs passed out to application protocol handlers? AppProtocols tend to be the easiest routes out of the browser sandbox, and the native-code full-trust applications parsing the URLs handed to them do so with varying levels of paranoia.

@TimothyGu
Copy link
Member

@ericlaw1979 So-called special schemes are just ftp, file, gopher, http, https, ws, wss. So it's probably not too likely for any third-party application to be affected by this. (I'm not sure about gopher though… who still uses that, and if so how?)

@mikewest
Copy link
Member

AppProtocols tend to be the easiest routes out of the browser sandbox, and the native-code full-trust applications parsing the URLs handed to them do so with varying levels of paranoia.

I'm pretty sure that Chrome, at least, hands everything in the URL to the app (the "non-special" case in the table above). If apps are vulnerable to some aspect of parsing nulls, they're already in trouble. This wouldn't add much trouble, I think.

(I'm not sure about gopher though… who still uses that, and if so how?)

#342 :)

annevk added a commit that referenced this issue Apr 26, 2020
rmisev added a commit to rmisev/web-platform-tests that referenced this issue Apr 26, 2020
annevk pushed a commit to web-platform-tests/wpt that referenced this issue Apr 27, 2020
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 1, 2020
…a=testonly

Automatic update from web-platform-tests
URL: percent encode NULLs in fragments

See whatwg/url#440 and whatwg/url#486 for context.
--

wpt-commits: efec8204e84d434d80407bb7cf8df37d33cabaa1
wpt-pr: 23256
xeonchen pushed a commit to xeonchen/gecko that referenced this issue May 1, 2020
…a=testonly

Automatic update from web-platform-tests
URL: percent encode NULLs in fragments

See whatwg/url#440 and whatwg/url#486 for context.
--

wpt-commits: efec8204e84d434d80407bb7cf8df37d33cabaa1
wpt-pr: 23256
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 1, 2020
…a=testonly

Automatic update from web-platform-tests
URL: percent encode NULLs in fragments

See whatwg/url#440 and whatwg/url#486 for context.
--

wpt-commits: efec8204e84d434d80407bb7cf8df37d33cabaa1
wpt-pr: 23256
xeonchen pushed a commit to xeonchen/gecko that referenced this issue May 1, 2020
…a=testonly

Automatic update from web-platform-tests
URL: percent encode NULLs in fragments

See whatwg/url#440 and whatwg/url#486 for context.
--

wpt-commits: efec8204e84d434d80407bb7cf8df37d33cabaa1
wpt-pr: 23256
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue May 3, 2020
…a=testonly

Automatic update from web-platform-tests
URL: percent encode NULLs in fragments

See whatwg/url#440 and whatwg/url#486 for context.
--

wpt-commits: efec8204e84d434d80407bb7cf8df37d33cabaa1
wpt-pr: 23256

UltraBlame original commit: 650ec6f46c2e17f43b3448bd91f3616e1622e3d9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue May 3, 2020
…a=testonly

Automatic update from web-platform-tests
URL: percent encode NULLs in fragments

See whatwg/url#440 and whatwg/url#486 for context.
--

wpt-commits: efec8204e84d434d80407bb7cf8df37d33cabaa1
wpt-pr: 23256

UltraBlame original commit: daaf57708f9447f0f68e4caadb80eac9ce03c477
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue May 3, 2020
…a=testonly

Automatic update from web-platform-tests
URL: percent encode NULLs in fragments

See whatwg/url#440 and whatwg/url#486 for context.
--

wpt-commits: efec8204e84d434d80407bb7cf8df37d33cabaa1
wpt-pr: 23256

UltraBlame original commit: 650ec6f46c2e17f43b3448bd91f3616e1622e3d9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue May 3, 2020
…a=testonly

Automatic update from web-platform-tests
URL: percent encode NULLs in fragments

See whatwg/url#440 and whatwg/url#486 for context.
--

wpt-commits: efec8204e84d434d80407bb7cf8df37d33cabaa1
wpt-pr: 23256

UltraBlame original commit: daaf57708f9447f0f68e4caadb80eac9ce03c477
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue May 3, 2020
…a=testonly

Automatic update from web-platform-tests
URL: percent encode NULLs in fragments

See whatwg/url#440 and whatwg/url#486 for context.
--

wpt-commits: efec8204e84d434d80407bb7cf8df37d33cabaa1
wpt-pr: 23256

UltraBlame original commit: 650ec6f46c2e17f43b3448bd91f3616e1622e3d9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue May 3, 2020
…a=testonly

Automatic update from web-platform-tests
URL: percent encode NULLs in fragments

See whatwg/url#440 and whatwg/url#486 for context.
--

wpt-commits: efec8204e84d434d80407bb7cf8df37d33cabaa1
wpt-pr: 23256

UltraBlame original commit: daaf57708f9447f0f68e4caadb80eac9ce03c477
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

6 participants