Add test for URL parsing of ICE servers in RTCConfiguration #47959
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
see w3c/webrtc-pc#2853 and related discussions in w3c/webrtc-pc#2997 (comment)
wpt-pr-bot
requested review from
alvestrand,
guidou,
henbos,
jan-ivar,
rwaldron,
snuggs and
youennf
dontcallmedom
added a commit
to w3c/webrtc-pc
that referenced
this pull request
Sep 4, 2024
alvestrand
reviewed
Sep 4, 2024
| @@ -301,4 +301,33 @@ | |||
| }] })); | |||
| }, `with empty urls should throw SyntaxError`); | |||
|
|
|||
| // Inspired from https://github.com/web-platform-tests/wpt/blob/master/url/resources/urltestdata.json | |||
| const validUrls = ['stun:example\t.\norg', 'stun:ex%61mple.net', 'stun:[2001::1]', 'stun:example(&!$)*+-.;_=~☺org', 'stun:example.net:', 'stun:example.net:0000000000000000', 'stun:example.net:65535', 'stun:example.net:\n', 'stun:GOO\u200b\u2060\ufeffgoo.com', '\u0000\u001b\u0004\u0012 stun:example.com\u001f \u000d ', 'stun:www.foo。bar.com', 'stun:Ｇｏ.com', 'stun:你好你好', 'stun:faß.ExAmPlE', 'stun:%30%78%63%30%2e%30%32%35%30.01', 'stun:%30%78%63%30%2e%30%32%35%30.01%2e', 'stun:０Ｘｃ０．０２５０．０１', 'stun:.', 'stun:ho\u0009st', "stun:!\"$&'()*+,-.;=_`{}~", "stun:\u0001\u0002\u0003\u0004\u0005\u0006\u0007\u0008\u000B\u000C\u000E\u000F\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001A\u001B\u001C\u001D\u001E\u001F\u007F!\"$%&'()*+,-.;=_`{}~", "stun:h\to\ns\rt:9\t0\n0\r0", 'stun:1.2.3.4.', 'stun:192.168.257', '192.168.257.com', 'stun:256', 'stun:256.com', 'stun:999999999', 'stun:0xffffffff', | |||
There was a problem hiding this comment.
can you format those blocks with one URL per line (s/,/,\n/) for easier reading and commenting?
Note that 192.168.257 is a perfectly valid IP address, but it means the same as 192.168.1.1 (I think).
There was a problem hiding this comment.
reformatted (and indeed, it is in the list of valid addresses)
| // Inspired from https://github.com/web-platform-tests/wpt/blob/master/url/resources/urltestdata.json | ||
| const validUrls = ['stun:example\t.\norg', 'stun:ex%61mple.net', 'stun:[2001::1]', 'stun:example(&!$)*+-.;_=~☺org', 'stun:example.net:', 'stun:example.net:0000000000000000', 'stun:example.net:65535', 'stun:example.net:\n', 'stun:GOO\u200b\u2060\ufeffgoo.com', '\u0000\u001b\u0004\u0012 stun:example.com\u001f \u000d ', 'stun:www.foo。bar.com', 'stun:Ｇｏ.com', 'stun:你好你好', 'stun:faß.ExAmPlE', 'stun:%30%78%63%30%2e%30%32%35%30.01', 'stun:%30%78%63%30%2e%30%32%35%30.01%2e', 'stun:０Ｘｃ０．０２５０．０１', 'stun:.', 'stun:ho\u0009st', "stun:!\"$&'()*+,-.;=_`{}~", "stun:\u0001\u0002\u0003\u0004\u0005\u0006\u0007\u0008\u000B\u000C\u000E\u000F\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001A\u001B\u001C\u001D\u001E\u001F\u007F!\"$%&'()*+,-.;=_`{}~", "stun:h\to\ns\rt:9\t0\n0\r0", 'stun:1.2.3.4.', 'stun:192.168.257', '192.168.257.com', 'stun:256', 'stun:256.com', 'stun:999999999', 'stun:0xffffffff', | ||
| // Spec say the following are valid, | ||
| // but that sounds like a bug |
There was a problem hiding this comment.
RFC 7064 section 3.1 says that they are not valid (no hierarchical part in the ABNF following "host/port" - please disambiguate "spec" here.
There was a problem hiding this comment.
I meant the WebRTC API - clarified
|
test results can be previewed at https://wpt.fyi/results/webrtc/RTCConfiguration-iceServers.html?sha=c515f732fc261e22d64b2d49de2a4b6f5cdc6502 |
|
It's possible that we need to specify the steps as:
But that's for the webrtc-pc spec to disambiguate. |
alvestrand
approved these changes
Sep 4, 2024
|
I'll refrain from merging for now since I expect we will want to fix the remaining inconsistencies with RFC parsing |
dontcallmedom
added a commit
to w3c/webrtc-pc
that referenced
this pull request
Sep 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
see w3c/webrtc-pc#2853 and related discussions in w3c/webrtc-pc#2997 (comment)