Skip to content
Permalink
Browse files

url: prioritize toString when stringifying

The ES addition operator calls the ToPrimitive() abstract operation
without hint String, leading a subsequent OrdinaryToPrimitive() to call
valueOf() first on an object rather than the desired toString().

Instead, use template literals which directly call ToString() abstract
operation, per Web IDL spec.

PR-URL: #11737
Fixes: b610a4d "url: enforce valid UTF-8 in WHATWG parser"
Refs: b610a4d#commitcomment-21200056
Refs: https://tc39.github.io/ecma262/#sec-addition-operator-plus-runtime-semantics-evaluation
Refs: https://tc39.github.io/ecma262/#sec-template-literals-runtime-semantics-evaluation
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information...
TimothyGu committed Mar 8, 2017
1 parent df97727 commit 99b27ce99a8bdd6bc733cde36a144a82cfea0b98
@@ -26,7 +26,7 @@ const IteratorPrototype = Object.getPrototypeOf(
const unpairedSurrogateRe =
/([^\uD800-\uDBFF]|^)[\uDC00-\uDFFF]|[\uD800-\uDBFF](?![\uDC00-\uDFFF])/;
function toUSVString(val) {
const str = '' + val;
const str = `${val}`;
// As of V8 5.5, `str.search()` (and `unpairedSurrogateRe[@@search]()`) are
// slower than `unpairedSurrogateRe.exec()`.
const match = unpairedSurrogateRe.exec(str);
@@ -218,7 +218,7 @@ function onParseHashComplete(flags, protocol, username, password,
class URL {
constructor(input, base) {
// toUSVString is not needed.
input = '' + input;
input = `${input}`;
if (base !== undefined && !(base instanceof URL))
base = new URL(base);
parse(this, input, base);
@@ -329,7 +329,7 @@ Object.defineProperties(URL.prototype, {
},
set(input) {
// toUSVString is not needed.
input = '' + input;
input = `${input}`;
parse(this, input);
}
},
@@ -348,7 +348,7 @@ Object.defineProperties(URL.prototype, {
},
set(scheme) {
// toUSVString is not needed.
scheme = '' + scheme;
scheme = `${scheme}`;
if (scheme.length === 0)
return;
binding.parse(scheme, binding.kSchemeStart, null, this[context],
@@ -363,7 +363,7 @@ Object.defineProperties(URL.prototype, {
},
set(username) {
// toUSVString is not needed.
username = '' + username;
username = `${username}`;
if (!this.hostname)
return;
const ctx = this[context];
@@ -384,7 +384,7 @@ Object.defineProperties(URL.prototype, {
},
set(password) {
// toUSVString is not needed.
password = '' + password;
password = `${password}`;
if (!this.hostname)
return;
const ctx = this[context];
@@ -410,7 +410,7 @@ Object.defineProperties(URL.prototype, {
set(host) {
const ctx = this[context];
// toUSVString is not needed.
host = '' + host;
host = `${host}`;
if (this[cannotBeBase] ||
(this[special] && host.length === 0)) {
// Cannot set the host if cannot-be-base is set or
@@ -435,7 +435,7 @@ Object.defineProperties(URL.prototype, {
set(host) {
const ctx = this[context];
// toUSVString is not needed.
host = '' + host;
host = `${host}`;
if (this[cannotBeBase] ||
(this[special] && host.length === 0)) {
// Cannot set the host if cannot-be-base is set or
@@ -460,7 +460,7 @@ Object.defineProperties(URL.prototype, {
},
set(port) {
// toUSVString is not needed.
port = '' + port;
port = `${port}`;
const ctx = this[context];
if (!ctx.host || this[cannotBeBase] ||
this.protocol === 'file:')
@@ -484,7 +484,7 @@ Object.defineProperties(URL.prototype, {
},
set(path) {
// toUSVString is not needed.
path = '' + path;
path = `${path}`;
if (this[cannotBeBase])
return;
binding.parse(path, binding.kPathStart, null, this[context],
@@ -533,7 +533,7 @@ Object.defineProperties(URL.prototype, {
set(hash) {
const ctx = this[context];
// toUSVString is not needed.
hash = '' + hash;
hash = `${hash}`;
if (this.protocol === 'javascript:')
return;
if (!hash) {
@@ -1125,12 +1125,12 @@ function originFor(url, base) {

function domainToASCII(domain) {
// toUSVString is not needed.
return binding.domainToASCII('' + domain);
return binding.domainToASCII(`${domain}`);
}

function domainToUnicode(domain) {
// toUSVString is not needed.
return binding.domainToUnicode('' + domain);
return binding.domainToUnicode(`${domain}`);
}

// Utility function that converts a URL object into an ordinary
@@ -58,7 +58,10 @@ test(function() {
params.set('a');
}, /^TypeError: "name" and "value" arguments must be specified$/);

const obj = { toString() { throw new Error('toString'); } };
const obj = {
toString() { throw new Error('toString'); },
valueOf() { throw new Error('valueOf'); }
};
const sym = Symbol();
assert.throws(() => params.set(obj, 'b'), /^Error: toString$/);
assert.throws(() => params.set('a', obj), /^Error: toString$/);
@@ -209,7 +209,10 @@ test(() => {
}

{
const obj = { toString() { throw new Error('toString'); } };
const obj = {
toString() { throw new Error('toString'); },
valueOf() { throw new Error('valueOf'); }
};
const sym = Symbol();

assert.throws(() => new URLSearchParams({ a: obj }), /^Error: toString$/);
@@ -52,7 +52,10 @@ test(function() {
params.delete();
}, /^TypeError: "name" argument must be specified$/);

const obj = { toString() { throw new Error('toString'); } };
const obj = {
toString() { throw new Error('toString'); },
valueOf() { throw new Error('valueOf'); }
};
const sym = Symbol();
assert.throws(() => params.delete(obj), /^Error: toString$/);
assert.throws(() => params.delete(sym),
@@ -43,7 +43,10 @@ test(function() {
params.get();
}, /^TypeError: "name" argument must be specified$/);

const obj = { toString() { throw new Error('toString'); } };
const obj = {
toString() { throw new Error('toString'); },
valueOf() { throw new Error('valueOf'); }
};
const sym = Symbol();
assert.throws(() => params.get(obj), /^Error: toString$/);
assert.throws(() => params.get(sym),
@@ -47,7 +47,10 @@ test(function() {
params.getAll();
}, /^TypeError: "name" argument must be specified$/);

const obj = { toString() { throw new Error('toString'); } };
const obj = {
toString() { throw new Error('toString'); },
valueOf() { throw new Error('valueOf'); }
};
const sym = Symbol();
assert.throws(() => params.getAll(obj), /^Error: toString$/);
assert.throws(() => params.getAll(sym),
@@ -46,7 +46,10 @@ test(function() {
params.has();
}, /^TypeError: "name" argument must be specified$/);

const obj = { toString() { throw new Error('toString'); } };
const obj = {
toString() { throw new Error('toString'); },
valueOf() { throw new Error('valueOf'); }
};
const sym = Symbol();
assert.throws(() => params.has(obj), /^Error: toString$/);
assert.throws(() => params.has(sym),
@@ -44,7 +44,10 @@ test(function() {
params.set('a');
}, /^TypeError: "name" and "value" arguments must be specified$/);

const obj = { toString() { throw new Error('toString'); } };
const obj = {
toString() { throw new Error('toString'); },
valueOf() { throw new Error('valueOf'); }
};
const sym = Symbol();
assert.throws(() => params.append(obj, 'b'), /^Error: toString$/);
assert.throws(() => params.append('a', obj), /^Error: toString$/);
@@ -107,7 +107,10 @@ startURLSettersTests()

{
const url = new URL('http://example.com/');
const obj = { toString() { throw new Error('toString'); } };
const obj = {
toString() { throw new Error('toString'); },
valueOf() { throw new Error('valueOf'); }
};
const sym = Symbol();
const props = Object.getOwnPropertyDescriptors(Object.getPrototypeOf(url));
for (const [name, { set }] of Object.entries(props)) {

0 comments on commit 99b27ce

Please sign in to comment.
You can’t perform that action at this time.