Skip to content

Commit 99b27ce

Browse files
TimothyGuTimothyGu
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: #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>
1 parent df97727 commit 99b27ce

9 files changed

+45
-21
lines changed

‎lib/internal/url.js

+13-13
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);
@@ -218,7 +218,7 @@ function onParseHashComplete(flags, protocol, username, password,
218218
class URL {
219219
constructor(input, base) {
220220
// toUSVString is not needed.
221-
input = '' + input;
221+
input = `${input}`;
222222
if (base !== undefined && !(base instanceof URL))
223223
base = new URL(base);
224224
parse(this, input, base);
@@ -329,7 +329,7 @@ Object.defineProperties(URL.prototype, {
329329
},
330330
set(input) {
331331
// toUSVString is not needed.
332-
input = '' + input;
332+
input = `${input}`;
333333
parse(this, input);
334334
}
335335
},
@@ -348,7 +348,7 @@ Object.defineProperties(URL.prototype, {
348348
},
349349
set(scheme) {
350350
// toUSVString is not needed.
351-
scheme = '' + scheme;
351+
scheme = `${scheme}`;
352352
if (scheme.length === 0)
353353
return;
354354
binding.parse(scheme, binding.kSchemeStart, null, this[context],
@@ -363,7 +363,7 @@ Object.defineProperties(URL.prototype, {
363363
},
364364
set(username) {
365365
// toUSVString is not needed.
366-
username = '' + username;
366+
username = `${username}`;
367367
if (!this.hostname)
368368
return;
369369
const ctx = this[context];
@@ -384,7 +384,7 @@ Object.defineProperties(URL.prototype, {
384384
},
385385
set(password) {
386386
// toUSVString is not needed.
387-
password = '' + password;
387+
password = `${password}`;
388388
if (!this.hostname)
389389
return;
390390
const ctx = this[context];
@@ -410,7 +410,7 @@ Object.defineProperties(URL.prototype, {
410410
set(host) {
411411
const ctx = this[context];
412412
// toUSVString is not needed.
413-
host = '' + host;
413+
host = `${host}`;
414414
if (this[cannotBeBase] ||
415415
(this[special] && host.length === 0)) {
416416
// Cannot set the host if cannot-be-base is set or
@@ -435,7 +435,7 @@ Object.defineProperties(URL.prototype, {
435435
set(host) {
436436
const ctx = this[context];
437437
// toUSVString is not needed.
438-
host = '' + host;
438+
host = `${host}`;
439439
if (this[cannotBeBase] ||
440440
(this[special] && host.length === 0)) {
441441
// Cannot set the host if cannot-be-base is set or
@@ -460,7 +460,7 @@ Object.defineProperties(URL.prototype, {
460460
},
461461
set(port) {
462462
// toUSVString is not needed.
463-
port = '' + port;
463+
port = `${port}`;
464464
const ctx = this[context];
465465
if (!ctx.host || this[cannotBeBase] ||
466466
this.protocol === 'file:')
@@ -484,7 +484,7 @@ Object.defineProperties(URL.prototype, {
484484
},
485485
set(path) {
486486
// toUSVString is not needed.
487-
path = '' + path;
487+
path = `${path}`;
488488
if (this[cannotBeBase])
489489
return;
490490
binding.parse(path, binding.kPathStart, null, this[context],
@@ -533,7 +533,7 @@ Object.defineProperties(URL.prototype, {
533533
set(hash) {
534534
const ctx = this[context];
535535
// toUSVString is not needed.
536-
hash = '' + hash;
536+
hash = `${hash}`;
537537
if (this.protocol === 'javascript:')
538538
return;
539539
if (!hash) {
@@ -1125,12 +1125,12 @@ function originFor(url, base) {
11251125

11261126
function domainToASCII(domain) {
11271127
// toUSVString is not needed.
1128-
return binding.domainToASCII('' + domain);
1128+
return binding.domainToASCII(`${domain}`);
11291129
}
11301130

11311131
function domainToUnicode(domain) {
11321132
// toUSVString is not needed.
1133-
return binding.domainToUnicode('' + domain);
1133+
return binding.domainToUnicode(`${domain}`);
11341134
}
11351135

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

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

+4-1
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

+4-1
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

+4-1
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

+4-1
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

+4-1
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

+4-1
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

+4-1
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

+4-1
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)