url: reuse existing context in href setter

Correctness-wise, this removes side effects in the href setter if parsing fails. Style-wise, this allows removing the parse() wrapper function around C++ _parse(). Also fix an existing bug with whitespace trimming in C++ that was previously circumvented by additionally trimming the input in JavaScript. Fixes: #24345 Refs: #24218 (comment) PR-URL: #24495 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>

Author: TimothyGu

lib/internal/url.js

@@ -43,7 +43,7 @@ const{
 domainToUnicode: _domainToUnicode,
  encodeAuth,
 toUSVString: _toUSVString,
-parse: _parse,
+ parse,
  setURLConstructor,
 URL_FLAGS_CANNOT_BE_BASE,
 URL_FLAGS_HAS_FRAGMENT,
@@ -243,14 +243,6 @@ function onParseError(flags, input){
 throwerror;
 }
 
-// Reused by URL constructor and URL#href setter.
-functionparse(url,input,base){
-constbase_context=base ? base[context] : undefined;
-url[context]=newURLContext();
-_parse(input.trim(),-1,base_context,undefined,
-onParseComplete.bind(url),onParseError);
-}
-
 functiononParseProtocolComplete(flags,protocol,username,password,
 host,port,path,query,fragment){
 constctx=this[context];
@@ -319,10 +311,13 @@ class URL{
 constructor(input,base){
 // toUSVString is not needed.
 input=`${input}`;
+letbase_context;
 if(base!==undefined){
-base=newURL(base);
+base_context=newURL(base)[context];
 }
-parse(this,input,base);
+this[context]=newURLContext();
+parse(input,-1,base_context,undefined,onParseComplete.bind(this),
+onParseError);
 }
 
 get[special](){
@@ -445,7 +440,8 @@ Object.defineProperties(URL.prototype,{
 set(input){
 // toUSVString is not needed.
 input=`${input}`;
-parse(this,input);
+parse(input,-1,undefined,undefined,onParseComplete.bind(this),
+onParseError);
 }
 },
 origin: {// readonly
@@ -491,8 +487,8 @@ Object.defineProperties(URL.prototype,{
 (ctx.host===''||ctx.host===null)){
 return;
 }
-_parse(scheme,kSchemeStart,null,ctx,
-onParseProtocolComplete.bind(this));
+parse(scheme,kSchemeStart,null,ctx,
+onParseProtocolComplete.bind(this));
 }
 },
 username: {
@@ -555,7 +551,7 @@ Object.defineProperties(URL.prototype,{
 // Cannot set the host if cannot-be-base is set
 return;
 }
-_parse(host,kHost,null,ctx,onParseHostComplete.bind(this));
+parse(host,kHost,null,ctx,onParseHostComplete.bind(this));
 }
 },
 hostname: {
@@ -572,7 +568,7 @@ Object.defineProperties(URL.prototype,{
 // Cannot set the host if cannot-be-base is set
 return;
 }
-_parse(host,kHostname,null,ctx,onParseHostnameComplete.bind(this));
+parse(host,kHostname,null,ctx,onParseHostnameComplete.bind(this));
 }
 },
 port: {
@@ -592,7 +588,7 @@ Object.defineProperties(URL.prototype,{
 ctx.port=null;
 return;
 }
-_parse(port,kPort,null,ctx,onParsePortComplete.bind(this));
+parse(port,kPort,null,ctx,onParsePortComplete.bind(this));
 }
 },
 pathname: {
@@ -611,8 +607,8 @@ Object.defineProperties(URL.prototype,{
 path=`${path}`;
 if(this[cannotBeBase])
 return;
-_parse(path,kPathStart,null,this[context],
-onParsePathComplete.bind(this));
+parse(path,kPathStart,null,this[context],
+onParsePathComplete.bind(this));
 }
 },
 search: {
@@ -635,7 +631,7 @@ Object.defineProperties(URL.prototype,{
 ctx.query='';
 ctx.flags|=URL_FLAGS_HAS_QUERY;
 if(search){
-_parse(search,kQuery,null,ctx,onParseSearchComplete.bind(this));
+parse(search,kQuery,null,ctx,onParseSearchComplete.bind(this));
 }
 }
 initSearchParams(this[searchParams],search);
@@ -669,7 +665,7 @@ Object.defineProperties(URL.prototype,{
 if(hash[0]==='#')hash=hash.slice(1);
 ctx.fragment='';
 ctx.flags|=URL_FLAGS_HAS_FRAGMENT;
-_parse(hash,kFragment,null,ctx,onParseHashComplete.bind(this));
+parse(hash,kFragment,null,ctx,onParseHashComplete.bind(this));
 }
 },
 toJSON: {

src/node_url.cc

@@ -1376,6 +1376,7 @@ void URL::Parse(const char* input,
 else
 break;
  }
+ input = p;
  len = end - p;
  }
 

test/parallel/test-whatwg-url-custom-href-side-effect.js

@@ -0,0 +1,15 @@
+'use strict';
+
+// Tests below are not from WPT.
+constcommon=require('../common');
+constassert=require('assert');
+
+constref=newURL('http://example.com/path');
+consturl=newURL('http://example.com/path');
+common.expectsError(()=>{
+url.href='';
+},{
+type: TypeError
+});
+
+assert.deepStrictEqual(url,ref);