url: prioritize toString when stringifying

TimothyGu · evanlucas · commit d6fe91648a43 · 2017-05-01T11:16:05.000-05:00
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 <jasnell@gmail.com>
diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -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);
@@ -215,7 +215,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);
@@ -326,7 +326,7 @@ Object.defineProperties(URL.prototype,{
 },
 set(input){
 // toUSVString is not needed.
- input = '' + input;
+ input = `${input}`;
 parse(this, input);
 }
 },
@@ -345,7 +345,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],
@@ -360,7 +360,7 @@ Object.defineProperties(URL.prototype,{
 },
 set(username){
 // toUSVString is not needed.
- username = '' + username;
+ username = `${username}`;
 if (!this.hostname)
 return;
 const ctx = this[context];
@@ -381,7 +381,7 @@ Object.defineProperties(URL.prototype,{
 },
 set(password){
 // toUSVString is not needed.
- password = '' + password;
+ password = `${password}`;
 if (!this.hostname)
 return;
 const ctx = this[context];
@@ -407,7 +407,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
@@ -432,7 +432,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
@@ -457,7 +457,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:')
@@ -481,7 +481,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],
@@ -530,7 +530,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){
@@ -1122,12 +1122,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
diff --git a/test/parallel/test-whatwg-url-searchparams-append.js b/test/parallel/test-whatwg-url-searchparams-append.js
@@ -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$/);
diff --git a/test/parallel/test-whatwg-url-searchparams-constructor.js b/test/parallel/test-whatwg-url-searchparams-constructor.js
@@ -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$/);
diff --git a/test/parallel/test-whatwg-url-searchparams-delete.js b/test/parallel/test-whatwg-url-searchparams-delete.js
@@ -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),
diff --git a/test/parallel/test-whatwg-url-searchparams-get.js b/test/parallel/test-whatwg-url-searchparams-get.js
@@ -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),
diff --git a/test/parallel/test-whatwg-url-searchparams-getall.js b/test/parallel/test-whatwg-url-searchparams-getall.js
@@ -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),
diff --git a/test/parallel/test-whatwg-url-searchparams-has.js b/test/parallel/test-whatwg-url-searchparams-has.js
@@ -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),
diff --git a/test/parallel/test-whatwg-url-searchparams-set.js b/test/parallel/test-whatwg-url-searchparams-set.js
@@ -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$/);
diff --git a/test/parallel/test-whatwg-url-setters.js b/test/parallel/test-whatwg-url-setters.js
@@ -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)){

-Original file line number
+Diff line change
 constunpairedSurrogateRe=
 /([^\uD800-\uDBFF]|^)[\uDC00-\uDFFF]|[\uD800-\uDBFF](?![\uDC00-\uDFFF])/;
 functiontoUSVString(val){
 -conststr=''+val;
 +conststr=`${val}`;
 // As of V8 5.5, `str.search()` (and `unpairedSurrogateRe[@@search]()`) are
 // slower than `unpairedSurrogateRe.exec()`.
 constmatch=unpairedSurrogateRe.exec(str);
 classURL{
 constructor(input,base){
 // toUSVString is not needed.
 -input=''+input;
 +input=`${input}`;
 if(base!==undefined&&!(baseinstanceofURL))
 base=newURL(base);
 parse(this,input,base);
 },
 set(input){
 // toUSVString is not needed.
 -input=''+input;
 +input=`${input}`;
 parse(this,input);
+}
 },
 },
 set(scheme){
 // toUSVString is not needed.
 -scheme=''+scheme;
 +scheme=`${scheme}`;
 if(scheme.length===0)
 return;
 binding.parse(scheme,binding.kSchemeStart,null,this[context],
 },
 set(username){
 // toUSVString is not needed.
 -username=''+username;
 +username=`${username}`;
 if(!this.hostname)
 return;
 constctx=this[context];
 },
 set(password){
 // toUSVString is not needed.
 -password=''+password;
 +password=`${password}`;
 if(!this.hostname)
 return;
 constctx=this[context];
 set(host){
 constctx=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
 set(host){
 constctx=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
 },
 set(port){
 // toUSVString is not needed.
 -port=''+port;
 +port=`${port}`;
 constctx=this[context];
 if(!ctx.host||this[cannotBeBase]||
 this.protocol==='file:')
 },
 set(path){
 // toUSVString is not needed.
 -path=''+path;
 +path=`${path}`;
 if(this[cannotBeBase])
 return;
 binding.parse(path,binding.kPathStart,null,this[context],
 set(hash){
 constctx=this[context];
 // toUSVString is not needed.
 -hash=''+hash;
 +hash=`${hash}`;
 if(this.protocol==='javascript:')
 return;
 if(!hash){
 functiondomainToASCII(domain){
 // toUSVString is not needed.
 -returnbinding.domainToASCII(''+domain);
 +returnbinding.domainToASCII(`${domain}`);
+}
 functiondomainToUnicode(domain){
 // toUSVString is not needed.
 -returnbinding.domainToUnicode(''+domain);
 +returnbinding.domainToUnicode(`${domain}`);
+}
 // Utility function that converts a URL object into an ordinary
-Original file line number
+Diff line change
 params.set('a');
 },/^TypeError:"name"and"value"argumentsmustbespecified$/);
 -constobj={toString(){thrownewError('toString');}};
 +constobj={
 +toString(){thrownewError('toString');},
 +valueOf(){thrownewError('valueOf');}
 +};
 constsym=Symbol();
 assert.throws(()=>params.set(obj,'b'),/^Error:toString$/);
 assert.throws(()=>params.set('a',obj),/^Error:toString$/);
Original file line number	Diff line number	Diff line change
`@@ -209,7 +209,10 @@ test(() =>{`
`209`	`209`	`}`
`210`	`210`
`211`	`211`	`{`
`212`		`-constobj={toString(){thrownewError('toString');}};`
	`212`	`+constobj={`
	`213`	`+toString(){thrownewError('toString');},`
	`214`	`+valueOf(){thrownewError('valueOf');}`
	`215`	`+};`
`213`	`216`	`constsym=Symbol();`
`214`	`217`
`215`	`218`	`assert.throws(()=>newURLSearchParams({a: obj}),/^Error:toString$/);`