fix invalid urlpattern test(s) #49782
Conversation
|
There are no reviewers for this pull request. Please reach out on the chat room to get help with this. Thank you! |
|
cc @sisidovski |
|
Aside: are there no tests where port is assigned to something that is clearly not a port? |
anonrig
force-pushed
the
anonrig/fix-wpt-for-urlpattern
branch
3 times, most recently
from
d66cd3c to
1e055be
Compare
@annevk I'm not sure. By the way, port is not the only wrong test here. As I progress with Ada's implementation, it seems |
anonrig
force-pushed
the
anonrig/fix-wpt-for-urlpattern
branch
from
1e055be to
68ffecf
Compare
anonrig
force-pushed
the
anonrig/fix-wpt-for-urlpattern
branch
from
68ffecf to
153c3e9
Compare
|
@annevk I've added a new commit that includes a test for a failing port field. |
|
See whatwg/urlpattern#239 (comment) - I am not entirely sure your analysis for port is correct. |
|
It seems that concern was resolved. @sisidovski and @jeremyroman can you please review this? |
| }, | ||
| { | ||
| "pattern": [{ "protocol": "http", "port": "100000" }], | ||
| "inputs": [{ "protocol": "http", "port": "100000" }], |
There was a problem hiding this comment.
I don't quite follow what the intention of this test case is. Could you help me to understand?
There was a problem hiding this comment.
Definitely. It tests the maximum valid port number. If it is greater than 65k, it should throw (according to url spec)
There was a problem hiding this comment.
Thanks. This is for the step 2.1.2 in the port state in the basic-url-parser. That makes sense.
| }, | ||
| { | ||
| "pattern": [{ "protocol": "http", "port": "100000" }], | ||
| "inputs": [{ "protocol": "http", "port": "100000" }], |
There was a problem hiding this comment.
Thanks. This is for the step 2.1.2 in the port state in the basic-url-parser. That makes sense.
| @@ -2419,15 +2436,24 @@ | |||
| }, | |||
| { | |||
| "pattern": [{ "hostname": "bad\nhostname" }], | |||
There was a problem hiding this comment.
Why are \n, \r, and \t just stripped?
| @@ -2367,15 +2375,24 @@ | |||
| }, | |||
| { | |||
| "pattern": [{ "hostname": "bad#hostname" }], | |||
There was a problem hiding this comment.
Could you clarify why only #%/ don't throw errors and other hostname patterns like "bad\:hostname" or "bad>hostname" continue throw errors?
btw I noticed "bad\:hostname" updates hostname in URL.
u = new URL('http:/dummy.site')
u.hostname = "bad\\:hostname"
u.hostname // bad
There was a problem hiding this comment.
Host parser (specifically domain to ASCII with domain and false) strip all trailing values whenever it sees # https://url.spec.whatwg.org/#concept-host-parser
There was a problem hiding this comment.
The host parser does not do that. Do you mean the host setter or some such?
Fixes whatwg/urlpattern#239
Port calls canonicalizePort which just calls url parser setter, which removes the leading trailing c0 whitespace characters from the input.
cc @annevk @lucacasonato @sisidovski