Skip to content

Commit d6fe916

Browse files
TimothyGuevanlucas
TimothyGu
authored and
evanlucas
committed
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: #12507 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: James M Snell <[email protected]>
1 parent 7c9ca0f commit d6fe916

9 files changed

+45
-21
lines changed

‎lib/internal/url.js

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const IteratorPrototype = Object.getPrototypeOf(
2626
const unpairedSurrogateRe =
2727
/([^\uD800-\uDBFF]|^)[\uDC00-\uDFFF]|[\uD800-\uDBFF](?![\uDC00-\uDFFF])/;
2828
function toUSVString(val) {
29-
const str = '' + val;
29+
const str = `${val}`;
3030
// As of V8 5.5, `str.search()` (and `unpairedSurrogateRe[@@search]()`) are
3131
// slower than `unpairedSurrogateRe.exec()`.
3232
const match = unpairedSurrogateRe.exec(str);
@@ -215,7 +215,7 @@ function onParseHashComplete(flags, protocol, username, password,
215215
class URL {
216216
constructor(input, base) {
217217
// toUSVString is not needed.
218-
input = '' + input;
218+
input = `${input}`;
219219
if (base !== undefined && !(base instanceof URL))
220220
base = new URL(base);
221221
parse(this, input, base);
@@ -326,7 +326,7 @@ Object.defineProperties(URL.prototype, {
326326
},
327327
set(input) {
328328
// toUSVString is not needed.
329-
input = '' + input;
329+
input = `${input}`;
330330
parse(this, input);
331331
}
332332
},
@@ -345,7 +345,7 @@ Object.defineProperties(URL.prototype, {
345345
},
346346
set(scheme) {
347347
// toUSVString is not needed.
348-
scheme = '' + scheme;
348+
scheme = `${scheme}`;
349349
if (scheme.length === 0)
350350
return;
351351
binding.parse(scheme, binding.kSchemeStart, null, this[context],
@@ -360,7 +360,7 @@ Object.defineProperties(URL.prototype, {
360360
},
361361
set(username) {
362362
// toUSVString is not needed.
363-
username = '' + username;
363+
username = `${username}`;
364364
if (!this.hostname)
365365
return;
366366
const ctx = this[context];
@@ -381,7 +381,7 @@ Object.defineProperties(URL.prototype, {
381381
},
382382
set(password) {
383383
// toUSVString is not needed.
384-
password = '' + password;
384+
password = `${password}`;
385385
if (!this.hostname)
386386
return;
387387
const ctx = this[context];
@@ -407,7 +407,7 @@ Object.defineProperties(URL.prototype, {
407407
set(host) {
408408
const ctx = this[context];
409409
// toUSVString is not needed.
410-
host = '' + host;
410+
host = `${host}`;
411411
if (this[cannotBeBase] ||
412412
(this[special] && host.length === 0)) {
413413
// Cannot set the host if cannot-be-base is set or
@@ -432,7 +432,7 @@ Object.defineProperties(URL.prototype, {
432432
set(host) {
433433
const ctx = this[context];
434434
// toUSVString is not needed.
435-
host = '' + host;
435+
host = `${host}`;
436436
if (this[cannotBeBase] ||
437437
(this[special] && host.length === 0)) {
438438
// Cannot set the host if cannot-be-base is set or
@@ -457,7 +457,7 @@ Object.defineProperties(URL.prototype, {
457457
},
458458
set(port) {
459459
// toUSVString is not needed.
460-
port = '' + port;
460+
port = `${port}`;
461461
const ctx = this[context];
462462
if (!ctx.host || this[cannotBeBase] ||
463463
this.protocol === 'file:')
@@ -481,7 +481,7 @@ Object.defineProperties(URL.prototype, {
481481
},
482482
set(path) {
483483
// toUSVString is not needed.
484-
path = '' + path;
484+
path = `${path}`;
485485
if (this[cannotBeBase])
486486
return;
487487
binding.parse(path, binding.kPathStart, null, this[context],
@@ -530,7 +530,7 @@ Object.defineProperties(URL.prototype, {
530530
set(hash) {
531531
const ctx = this[context];
532532
// toUSVString is not needed.
533-
hash = '' + hash;
533+
hash = `${hash}`;
534534
if (this.protocol === 'javascript:')
535535
return;
536536
if (!hash) {
@@ -1122,12 +1122,12 @@ function originFor(url, base) {
11221122

11231123
function domainToASCII(domain) {
11241124
// toUSVString is not needed.
1125-
return binding.domainToASCII('' + domain);
1125+
return binding.domainToASCII(`${domain}`);
11261126
}
11271127

11281128
function domainToUnicode(domain) {
11291129
// toUSVString is not needed.
1130-
return binding.domainToUnicode('' + domain);
1130+
return binding.domainToUnicode(`${domain}`);
11311131
}
11321132

11331133
// Utility function that converts a URL object into an ordinary

‎test/parallel/test-whatwg-url-searchparams-append.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@ test(function() {
5858
params.set('a');
5959
}, /^TypeError: "name" and "value" arguments must be specified$/);
6060

61-
const obj = { toString() { throw new Error('toString'); } };
61+
const obj = {
62+
toString() { throw new Error('toString'); },
63+
valueOf() { throw new Error('valueOf'); }
64+
};
6265
const sym = Symbol();
6366
assert.throws(() => params.set(obj, 'b'), /^Error: toString$/);
6467
assert.throws(() => params.set('a', obj), /^Error: toString$/);

‎test/parallel/test-whatwg-url-searchparams-constructor.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,10 @@ test(() => {
209209
}
210210

211211
{
212-
const obj = { toString() { throw new Error('toString'); } };
212+
const obj = {
213+
toString() { throw new Error('toString'); },
214+
valueOf() { throw new Error('valueOf'); }
215+
};
213216
const sym = Symbol();
214217

215218
assert.throws(() => new URLSearchParams({ a: obj }), /^Error: toString$/);

‎test/parallel/test-whatwg-url-searchparams-delete.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ test(function() {
5252
params.delete();
5353
}, /^TypeError: "name" argument must be specified$/);
5454

55-
const obj = { toString() { throw new Error('toString'); } };
55+
const obj = {
56+
toString() { throw new Error('toString'); },
57+
valueOf() { throw new Error('valueOf'); }
58+
};
5659
const sym = Symbol();
5760
assert.throws(() => params.delete(obj), /^Error: toString$/);
5861
assert.throws(() => params.delete(sym),

‎test/parallel/test-whatwg-url-searchparams-get.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@ test(function() {
4343
params.get();
4444
}, /^TypeError: "name" argument must be specified$/);
4545

46-
const obj = { toString() { throw new Error('toString'); } };
46+
const obj = {
47+
toString() { throw new Error('toString'); },
48+
valueOf() { throw new Error('valueOf'); }
49+
};
4750
const sym = Symbol();
4851
assert.throws(() => params.get(obj), /^Error: toString$/);
4952
assert.throws(() => params.get(sym),

‎test/parallel/test-whatwg-url-searchparams-getall.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ test(function() {
4747
params.getAll();
4848
}, /^TypeError: "name" argument must be specified$/);
4949

50-
const obj = { toString() { throw new Error('toString'); } };
50+
const obj = {
51+
toString() { throw new Error('toString'); },
52+
valueOf() { throw new Error('valueOf'); }
53+
};
5154
const sym = Symbol();
5255
assert.throws(() => params.getAll(obj), /^Error: toString$/);
5356
assert.throws(() => params.getAll(sym),

‎test/parallel/test-whatwg-url-searchparams-has.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ test(function() {
4646
params.has();
4747
}, /^TypeError: "name" argument must be specified$/);
4848

49-
const obj = { toString() { throw new Error('toString'); } };
49+
const obj = {
50+
toString() { throw new Error('toString'); },
51+
valueOf() { throw new Error('valueOf'); }
52+
};
5053
const sym = Symbol();
5154
assert.throws(() => params.has(obj), /^Error: toString$/);
5255
assert.throws(() => params.has(sym),

‎test/parallel/test-whatwg-url-searchparams-set.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ test(function() {
4444
params.set('a');
4545
}, /^TypeError: "name" and "value" arguments must be specified$/);
4646

47-
const obj = { toString() { throw new Error('toString'); } };
47+
const obj = {
48+
toString() { throw new Error('toString'); },
49+
valueOf() { throw new Error('valueOf'); }
50+
};
4851
const sym = Symbol();
4952
assert.throws(() => params.append(obj, 'b'), /^Error: toString$/);
5053
assert.throws(() => params.append('a', obj), /^Error: toString$/);

‎test/parallel/test-whatwg-url-setters.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,10 @@ startURLSettersTests()
107107

108108
{
109109
const url = new URL('http://example.com/');
110-
const obj = { toString() { throw new Error('toString'); } };
110+
const obj = {
111+
toString() { throw new Error('toString'); },
112+
valueOf() { throw new Error('valueOf'); }
113+
};
111114
const sym = Symbol();
112115
const props = Object.getOwnPropertyDescriptors(Object.getPrototypeOf(url));
113116
for (const [name, { set }] of Object.entries(props)) {

0 commit comments

Comments
 (0)