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

Verify domain is not empty after "domain to ASCII" #497

Merged
merged 2 commits into from
May 7, 2020

Conversation

rmisev
Copy link
Member

@rmisev rmisev commented May 6, 2020

The empty host is not allowed in special non-"file" URLs, but
domain to ASCII result can be the empty string, even if the
input is not empty, for example:

  • the input consists entirely of IDNA ignored code points;
  • the input is xn--.

The domain to ASCII do not fail when it is called from host parser,
because VerifyDnsLength is false.
So we need additional check after it.

An interesting case is with file URLS, as empty host is allowed. But most
browsers report failure as well. I tested URLs with the U+00AD
percent encoded in host:

URL Chrome Firefox Safari
file://%C2%AD/p TypeError file:///p TypeError
http://%C2%AD/p TypeError TypeError TypeError

Note. To find more IDNA ignored code points refer to IDNA Mapping Table:
https://unicode.org/reports/tr46/#IDNA_Mapping_Table

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

The empty host is not allowed in special non-"file" URLs, but
`domain to ASCII` result can be the empty string, even if the
input is not empty, for example:

* the input consists entirely of IDNA ignored code points;
* the input is `xn--`.

The `domain to ASCII` do not fail when it is called from `host parser`,
because `VerifyDnsLength` is false.
So we need additional check after it.
rmisev added a commit to rmisev/web-platform-tests that referenced this pull request May 6, 2020
@annevk
Copy link
Member

annevk commented May 6, 2020

See also #438.

I think we should add this check in https://url.spec.whatwg.org/#concept-domain-to-ascii so that a domain also cannot be valid if it ends up empty.

@rmisev
Copy link
Member Author

rmisev commented May 6, 2020

Hmm, in the valid domain check https://whatpr.org/url/497.html#valid-domain the domain to ASCII is called with beStrict = true, i. e. VerifyDnsLength = true, so will fail anyway on empty result.

@annevk
Copy link
Member

annevk commented May 6, 2020

Ah fair, I guess I'd still like to include this check there, but I'd be okay with making it conditional upon beStrict. At some point we have to revise the general ToASCII operation and it would be good if all the weirdness was consolidated to make that easier.

@rmisev
Copy link
Member Author

rmisev commented May 6, 2020

OK, changed.

annevk pushed a commit to web-platform-tests/wpt that referenced this pull request May 7, 2020
@annevk annevk merged commit cceb435 into whatwg:master May 7, 2020
@rmisev rmisev deleted the toascii-empty branch May 7, 2020 10:20
TRowbotham added a commit to TRowbotham/URL-Parser that referenced this pull request May 15, 2020
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 15, 2020
…st, a=testonly

Automatic update from web-platform-tests
URL: test IDNA ignored code points in host

See whatwg/url#497 for context.
--

wpt-commits: e9a106175a02a192a7239f42a94235fbe0f0c7a3
wpt-pr: 23432
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request May 15, 2020
…st, a=testonly

Automatic update from web-platform-tests
URL: test IDNA ignored code points in host

See whatwg/url#497 for context.
--

wpt-commits: e9a106175a02a192a7239f42a94235fbe0f0c7a3
wpt-pr: 23432
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 20, 2020
…st, a=testonly

Automatic update from web-platform-tests
URL: test IDNA ignored code points in host

See whatwg/url#497 for context.
--

wpt-commits: e9a106175a02a192a7239f42a94235fbe0f0c7a3
wpt-pr: 23432
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request May 20, 2020
…st, a=testonly

Automatic update from web-platform-tests
URL: test IDNA ignored code points in host

See whatwg/url#497 for context.
--

wpt-commits: e9a106175a02a192a7239f42a94235fbe0f0c7a3
wpt-pr: 23432
rmisev added a commit to upa-url/upa that referenced this pull request May 24, 2020
djc added a commit to servo/rust-url that referenced this pull request Aug 18, 2020
targos added a commit to targos/node that referenced this pull request Sep 8, 2020
nodejs-github-bot pushed a commit to nodejs/node that referenced this pull request Sep 16, 2020
Port of whatwg/url#497

PR-URL: #33770
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
Port of whatwg/url#497

PR-URL: nodejs#33770
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants