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: https://github.com/nodejs/node/issues/24345 Refs: https://github.com/nodejs/node/pull/24218#issuecomment-438476591 PR-URL: https://github.com/nodejs/node/pull/24495 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
parent
639f6411a7
commit
f2be20b735
@ -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) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
// Reused by URL constructor and URL#href setter.
|
||||
function parse(url, input, base) {
|
||||
const base_context = base ? base[context] : undefined;
|
||||
url[context] = new URLContext();
|
||||
_parse(input.trim(), -1, base_context, undefined,
|
||||
onParseComplete.bind(url), onParseError);
|
||||
}
|
||||
|
||||
function onParseProtocolComplete(flags, protocol, username, password,
|
||||
host, port, path, query, fragment) {
|
||||
const ctx = this[context];
|
||||
@ -319,10 +311,13 @@ class URL {
|
||||
constructor(input, base) {
|
||||
// toUSVString is not needed.
|
||||
input = `${input}`;
|
||||
let base_context;
|
||||
if (base !== undefined) {
|
||||
base = new URL(base);
|
||||
base_context = new URL(base)[context];
|
||||
}
|
||||
parse(this, input, base);
|
||||
this[context] = new URLContext();
|
||||
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: {
|
||||
|
@ -1376,6 +1376,7 @@ void URL::Parse(const char* input,
|
||||
else
|
||||
break;
|
||||
}
|
||||
input = p;
|
||||
len = end - p;
|
||||
}
|
||||
|
||||
|
15
test/parallel/test-whatwg-url-custom-href-side-effect.js
Normal file
15
test/parallel/test-whatwg-url-custom-href-side-effect.js
Normal file
@ -0,0 +1,15 @@
|
||||
'use strict';
|
||||
|
||||
// Tests below are not from WPT.
|
||||
const common = require('../common');
|
||||
const assert = require('assert');
|
||||
|
||||
const ref = new URL('http://example.com/path');
|
||||
const url = new URL('http://example.com/path');
|
||||
common.expectsError(() => {
|
||||
url.href = '';
|
||||
}, {
|
||||
type: TypeError
|
||||
});
|
||||
|
||||
assert.deepStrictEqual(url, ref);
|
Loading…
x
Reference in New Issue
Block a user