Skip to content

Commit

Permalink
Bug 1489623 - Throw SyntaxError on empty urls in iceServers and switc…
Browse files Browse the repository at this point in the history 
…h to real SyntaxError over DOMException, + update WPT. r=ng

Differential Revision: https://phabricator.services.mozilla.com/D5452

--HG--
extra : moz-landing-system : lando
  • Loading branch information
@jan-ivar
jan-ivar committed Sep 10, 2018
1 parent ab81400 commit c65e5bb
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 114 deletions.
27 changes: 14 additions & 13 deletions dom/media/PeerConnection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -719,8 +719,7 @@ class RTCPeerConnection {
return Services.io.newURI(uriStr);
} catch (e) {
if (e.result == Cr.NS_ERROR_MALFORMED_URI) {
throw new this._win.DOMException(msg + " - malformed URI: " + uriStr,
"SyntaxError");
throw new this._win.SyntaxError(`${msg} - malformed URI: ${uriStr}`);
}
throw e;
}
Expand All @@ -730,27 +729,30 @@ class RTCPeerConnection {

iceServers.forEach(({ urls, username, credential, credentialType }) => {
if (!urls) {
throw new this._win.DOMException(msg + " - missing urls", "InvalidAccessError");
// TODO: Remove once url is deprecated (Bug 1369563)
throw new this._win.TypeError("Missing required 'urls' member of RTCIceServer");
}
if (urls.length == 0) {
throw new this._win.SyntaxError(`${msg} - urls is empty`);
}
urls.map(url => nicerNewURI(url)).forEach(({ scheme, spec }) => {
if (scheme in { turn: 1, turns: 1 }) {
if (username == undefined) {
throw new this._win.DOMException(msg + " - missing username: " + spec,
throw new this._win.DOMException(`${msg} - missing username: ${spec}`,
"InvalidAccessError");
}
if (username.length > 512) {
throw new this._win.DOMException(msg +
" - username longer then 512 bytes: "
+ username, "InvalidAccessError");
throw new this._win.DOMException(
`${msg} - username longer then 512 bytes: ${username}`,
"InvalidAccessError");
}
if (credential == undefined) {
throw new this._win.DOMException(msg + " - missing credential: " + spec,
throw new this._win.DOMException(`${msg} - missing credential: ${spec}`,
"InvalidAccessError");
}
if (credentialType != "password") {
this.logWarning("RTCConfiguration TURN credentialType \"" +
credentialType +
"\" is not yet implemented. Treating as password." +
this.logWarning(`RTCConfiguration TURN credentialType \"${credentialType}\"` +
" is not yet implemented. Treating as password." +
" https://bugzil.la/1247616");
}
this._hasTurnServer = true;
Expand All @@ -759,8 +761,7 @@ class RTCPeerConnection {
this._hasStunServer = true;
stunServers += 1;
} else {
throw new this._win.DOMException(msg + " - improper scheme: " + scheme,
"SyntaxError");
throw new this._win.SyntaxError(`${msg} - improper scheme: ${scheme}`);
}
if (scheme in { stuns: 1 }) {
this.logWarning(scheme.toUpperCase() + " is not yet supported.");
Expand Down
16 changes: 1 addition & 15 deletions dom/media/tests/mochitest/test_peerConnection_bug825703.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,7 @@
ok(!exception, "RTCPeerConnection() succeeds");
exception = null;

makePC();

makePC(1, "TypeError");

makePC({});

makePC({ iceServers: [] });

makePC({ iceServers: [{ urls:"" }] }, "SyntaxError");
// Some overlap still with WPT RTCConfiguration-iceServers.html

makePC({ iceServers: [
{ urls:"stun:127.0.0.1" },
Expand All @@ -75,12 +67,6 @@
{ url:"turn:localhost", username:"p", credential:"p" }
]});

makePC({ iceServers: [{ urls: ["stun:127.0.0.1", ""] }] }, "SyntaxError");

makePC({ iceServers: [{ urls:"turns:localhost:3478", username:"p" }] }, "InvalidAccessError");

makePC({ iceServers: [{ url:"turns:localhost:3478", credential:"p" }] }, "InvalidAccessError");

makePC({ iceServers: [{ urls:"http:0.0.0.0" }] }, "SyntaxError");

try {
Expand Down
46 changes: 12 additions & 34 deletions testing/web-platform/meta/webrtc/RTCConfiguration-iceServers.html.ini
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
[RTCConfiguration-iceServers.html]
[setConfiguration(config) - {} should succeed]
expected: FAIL

[setConfiguration(config) - { iceServers: null } should throw TypeError]
expected: FAIL

Expand All @@ -8,21 +11,12 @@
[setConfiguration(config) - { iceServers: [\] } should succeed]
expected: FAIL

[new RTCPeerConnection(config) - { iceServers: [null\] } should throw TypeError]
expected: FAIL

[setConfiguration(config) - { iceServers: [null\] } should throw TypeError]
expected: FAIL

[new RTCPeerConnection(config) - { iceServers: [undefined\] } should throw TypeError]
expected: FAIL

[setConfiguration(config) - { iceServers: [undefined\] } should throw TypeError]
expected: FAIL

[new RTCPeerConnection(config) - { iceServers: [{}\] } should throw TypeError]
expected: FAIL

[setConfiguration(config) - { iceServers: [{}\] } should throw TypeError]
expected: FAIL

Expand All @@ -38,6 +32,9 @@
[setConfiguration(config) - with 2 stun servers should succeed]
expected: FAIL

[setConfiguration(config) - with turn server, username, credential should succeed]
expected: FAIL

[setConfiguration(config) - with turns server and empty string username, credential should succeed]
expected: FAIL

Expand Down Expand Up @@ -68,6 +65,12 @@
[setConfiguration(config) - with turns server and only credential should throw InvalidAccessError]
expected: FAIL

[setConfiguration(config) - with "" url should throw SyntaxError]
expected: FAIL

[setConfiguration(config) - with ["stun:stun1.example.net", ""\] url should throw SyntaxError]
expected: FAIL

[setConfiguration(config) - with relative url should throw SyntaxError]
expected: FAIL

Expand All @@ -86,15 +89,6 @@
[setConfiguration(config) - with invalid stun url should throw SyntaxError]
expected: FAIL

[setConfiguration(config) - with empty urls and credentialType password should succeed]
expected: FAIL

[new RTCPeerConnection(config) - with empty urls and credentialType oauth should succeed]
expected: FAIL

[setConfiguration(config) - with empty urls and credentialType oauth should succeed]
expected: FAIL

[setConfiguration(config) - with invalid credentialType should throw TypeError]
expected: FAIL

Expand Down Expand Up @@ -146,21 +140,5 @@
[setConfiguration(config) - with stun server, credentialType password, and RTCOAuthCredential credential should succeed]
expected: FAIL

[new RTCPeerConnection(config) - with empty urls list, credentialType oauth, and string credential should succeed]
expected: FAIL

[setConfiguration(config) - with empty urls list, credentialType oauth, and string credential should succeed]
expected: FAIL

[new RTCPeerConnection(config) - with empty urls list, credentialType password, and RTCOAuthCredential credential should succeed]
expected: FAIL

[setConfiguration(config) - with empty urls list, credentialType password, and RTCOAuthCredential credential should succeed]
expected: FAIL

[setConfiguration(config) - with empty urls should throw SyntaxError]
expected: FAIL

[new RTCPeerConnection(config) - with empty urls should throw SyntaxError]
expected: FAIL

76 changes: 24 additions & 52 deletions testing/web-platform/tests/webrtc/RTCConfiguration-iceServers.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@
assert_equals(pc.getConfiguration().iceServers, undefined);
}, 'new RTCPeerConnection() should have default configuration.iceServers of undefined');

config_test(makePc => {
makePc({});
}, '{} should succeed');

config_test(makePc => {
assert_throws(new TypeError(), () =>
makePc({ iceServers: null }));
Expand Down Expand Up @@ -126,7 +130,7 @@
}, `with 2 stun servers should succeed`);

config_test(makePc => {
const pc = new RTCPeerConnection({ iceServers: [{
const pc = makePc({ iceServers: [{
urls: 'turn:turn.example.org',
username: 'user',
credential: 'cred'
Expand Down Expand Up @@ -281,35 +285,49 @@
transport-ext = 1*unreserved
*/
config_test(makePc => {
assert_throws('SyntaxError', () =>
assert_throws(new SyntaxError(), () =>
makePc({ iceServers: [{
urls: ''
}] }));
}, 'with "" url should throw SyntaxError');

config_test(makePc => {
assert_throws(new SyntaxError(), () =>
makePc({ iceServers: [{
urls: ['stun:stun1.example.net', '']
}] }));
}, 'with ["stun:stun1.example.net", ""] url should throw SyntaxError');

config_test(makePc => {
assert_throws(new SyntaxError(), () =>
makePc({ iceServers: [{
urls: 'relative-url'
}] }));
}, 'with relative url should throw SyntaxError');

config_test(makePc => {
assert_throws('SyntaxError', () =>
assert_throws(new SyntaxError(), () =>
makePc({ iceServers: [{
urls: 'http://example.com'
}] }));
}, 'with http url should throw SyntaxError');

config_test(makePc => {
assert_throws('SyntaxError', () =>
assert_throws(new SyntaxError(), () =>
makePc({ iceServers: [{
urls: 'turn://example.org/foo?x=y'
}] }));
}, 'with invalid turn url should throw SyntaxError');

config_test(makePc => {
assert_throws('SyntaxError', () =>
assert_throws(new SyntaxError(), () =>
makePc({ iceServers: [{
urls: 'stun://example.org/foo?x=y'
}] }));
}, 'with invalid stun url should throw SyntaxError');

config_test(makePc => {
assert_throws('SyntaxError', () =>
assert_throws(new SyntaxError(), () =>
makePc({ iceServers: [{
urls: []
}] }));
Expand Down Expand Up @@ -471,52 +489,6 @@

}, 'with stun server, credentialType password, and RTCOAuthCredential credential should succeed');

// credential type validation is ignored when urls is empty and there is no scheme name
config_test(makePc => {
const pc = makePc({ iceServers: [{
urls: [],
credentialType: 'oauth',
username: 'user',
credential: 'cred'
}] });

const { iceServers } = pc.getConfiguration();
assert_equals(iceServers.length, 1);
const server = iceServers[0];

assert_array_equals(server.urls, []);
assert_equals(server.credentialType, 'oauth');
assert_equals(server.username, 'user');
assert_equals(server.credential, 'cred');

}, 'with empty urls list, credentialType oauth, and string credential should succeed');

// credential type validation is ignored when urls is empty and there is no scheme name
config_test(makePc => {
const pc = makePc({ iceServers: [{
urls: [],
credentialType: 'password',
username: 'user',
credential: {
macKey: '',
accessToken: ''
}
}] });

const { iceServers } = pc.getConfiguration();
assert_equals(iceServers.length, 1);

const server = iceServers[0];
assert_array_equals(server.urls, []);
assert_equals(server.credentialType, 'password');
assert_equals(server.username, 'user');

const { credential } = server;
assert_equals(credential.macKey, '');
assert_equals(credential.accessToken, '');

}, 'with empty urls list, credentialType password, and RTCOAuthCredential credential should succeed');

/*
Tested
4.3.2. To set a configuration
Expand Down

0 comments on commit c65e5bb

Please sign in to comment.