-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Tests for forbidden host code-points and resolution of non-special URLs #29246
Tests for forbidden host code-points and resolution of non-special URLs #29246
Conversation
url/resources/urltestdata.json
Outdated
@@ -7794,5 +7794,204 @@ | |||
"protocol": "wss:", | |||
"search": "", | |||
"username": "" | |||
}, | |||
"Forbidden host codepoints", | |||
{ "input": "foo://ho\u0000st/", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to add these tests near the tests added in #23572?
Also, the prevailing style in the file is to have {
not be on the same line as the first property.
url/resources/urltestdata.json
Outdated
"base": "about:blank", | ||
"failure": true | ||
}, | ||
"Ensure that non-special URLs use strict resolution", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"strict resolution" isn't a term used in the URL Standard, though I know it is in RFC 3986. Let's just say
"Ensure that non-special URLs use strict resolution", | |
"Ensure that schemes are not dropped when resolving non-special URLs", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normative content of tests LGTM, but it'd be great to address the style/naming issues pointed out by @TimothyGu.
I understand and agree, no need to point it out twice. I moved the entire block of forbidden host codepoint tests up and removed the three duplicates. |
This adds additional tests for:
e.g.
abc:rootless
againstabc://host/path
should result inabc:rootless
.Related issues:
/cc @TimothyGu
Edit It seems some of the forbidden host code points were already tested in #23572.
Let me know if I should remove the (few) duplicates.