Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

url: offload URLSearchParams initialization #46867

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const {
const Agent = require('_http_agent');
const { Buffer } = require('buffer');
const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
const { URL, urlToHttpOptions, searchParamsSymbol } = require('internal/url');
const { URL, urlToHttpOptions, isURL } = require('internal/url');
const {
kOutHeaders,
kNeedDrain,
Expand Down Expand Up @@ -133,8 +133,7 @@ function ClientRequest(input, options, cb) {
if (typeof input === 'string') {
const urlStr = input;
input = urlToHttpOptions(new URL(urlStr));
} else if (input && input[searchParamsSymbol] &&
input[searchParamsSymbol][searchParamsSymbol]) {
} else if (isURL(input)) {
// url.URL instance
input = urlToHttpOptions(input);
} else {
Expand Down
6 changes: 2 additions & 4 deletions lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const { ClientRequest } = require('_http_client');
let debug = require('internal/util/debuglog').debuglog('https', (fn) => {
debug = fn;
});
const { URL, urlToHttpOptions, searchParamsSymbol } = require('internal/url');
const { URL, urlToHttpOptions, isURL } = require('internal/url');
const { validateObject } = require('internal/validators');

function Server(opts, requestListener) {
Expand Down Expand Up @@ -350,9 +350,7 @@ function request(...args) {
if (typeof args[0] === 'string') {
const urlStr = ArrayPrototypeShift(args);
options = urlToHttpOptions(new URL(urlStr));
} else if (args[0] && args[0][searchParamsSymbol] &&
args[0][searchParamsSymbol][searchParamsSymbol]) {
// url.URL instance
} else if (isURL(args[0])) {
options = urlToHttpOptions(ArrayPrototypeShift(args));
}

Expand Down
84 changes: 37 additions & 47 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ const FORWARD_SLASH = /\//g;

const context = Symbol('context');
const searchParams = Symbol('query');
const kFormat = Symbol('format');

const updateActions = {
kProtocol: 0,
Expand Down Expand Up @@ -226,7 +225,7 @@ class URLSearchParams {
} else {
// USVString
init = toUSVString(init);
initSearchParams(this, init);
this[searchParams] = init ? parseParams(init) : [];
}

// "associated url object"
Expand Down Expand Up @@ -536,7 +535,7 @@ ObjectDefineProperties(URLSearchParams.prototype, {
},
});

function isURLThis(self) {
function isURL(self) {
return self != null && ObjectPrototypeHasOwnProperty(self, context);
}

Expand Down Expand Up @@ -605,160 +604,161 @@ class URL {
ctx.password = password;
ctx.port = port;
ctx.hash = hash;
if (!this[searchParams]) { // Invoked from URL constructor
this[searchParams] = new URLSearchParams();
this[searchParams][context] = this;
if (this[searchParams]) {
this[searchParams][searchParams] = parseParams(search);
}
initSearchParams(this[searchParams], ctx.search);
};

toString() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].href;
}

get href() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].href;
}

set href(value) {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
const valid = updateUrl(this[context].href, updateActions.kHref, `${value}`, this.#onParseComplete);
if (!valid) { throw ERR_INVALID_URL(`${value}`); }
}

// readonly
get origin() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].origin;
}

get protocol() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].protocol;
}

set protocol(value) {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
updateUrl(this[context].href, updateActions.kProtocol, `${value}`, this.#onParseComplete);
}

get username() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].username;
}

set username(value) {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
updateUrl(this[context].href, updateActions.kUsername, `${value}`, this.#onParseComplete);
}

get password() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].password;
}

set password(value) {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
updateUrl(this[context].href, updateActions.kPassword, `${value}`, this.#onParseComplete);
}

get host() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
const port = this[context].port;
const suffix = port.length > 0 ? `:${port}` : '';
return this[context].hostname + suffix;
}

set host(value) {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
updateUrl(this[context].href, updateActions.kHost, `${value}`, this.#onParseComplete);
}

get hostname() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].hostname;
}

set hostname(value) {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
updateUrl(this[context].href, updateActions.kHostname, `${value}`, this.#onParseComplete);
}

get port() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].port;
}

set port(value) {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
updateUrl(this[context].href, updateActions.kPort, `${value}`, this.#onParseComplete);
}

get pathname() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].pathname;
}

set pathname(value) {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
updateUrl(this[context].href, updateActions.kPathname, `${value}`, this.#onParseComplete);
}

get search() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].search;
}

set search(search) {
if (!isURLThis(this))
set search(value) {
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
search = toUSVString(search);
updateUrl(this[context].href, updateActions.kSearch, search, this.#onParseComplete);
initSearchParams(this[searchParams], this[context].search);
updateUrl(this[context].href, updateActions.kSearch, toUSVString(value), this.#onParseComplete);
}

// readonly
get searchParams() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
// Create URLSearchParams on demand to greatly improve the URL performance.
if (this[searchParams] == null) {
this[searchParams] = new URLSearchParams(this[context].search);
this[searchParams][context] = this;
}
return this[searchParams];
}

get hash() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].hash;
}

set hash(value) {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
updateUrl(this[context].href, updateActions.kHash, `${value}`, this.#onParseComplete);
}

toJSON() {
if (!isURLThis(this))
if (!isURL(this))
throw new ERR_INVALID_THIS('URL');
return this[context].href;
}
Expand Down Expand Up @@ -794,7 +794,6 @@ class URL {
}

ObjectDefineProperties(URL.prototype, {
[kFormat]: { __proto__: null, configurable: false, writable: false },
[SymbolToStringTag]: { __proto__: null, configurable: true, value: 'URL' },
toString: kEnumerableProperty,
href: kEnumerableProperty,
Expand All @@ -817,14 +816,6 @@ ObjectDefineProperties(URL, {
revokeObjectURL: kEnumerableProperty,
});

function initSearchParams(url, init) {
if (!init) {
url[searchParams] = [];
return;
}
url[searchParams] = parseParams(init);
}

// application/x-www-form-urlencoded parser
// Ref: https://url.spec.whatwg.org/#concept-urlencoded-parser
function parseParams(qs) {
Expand Down Expand Up @@ -1143,8 +1134,7 @@ function domainToUnicode(domain) {
function urlToHttpOptions(url) {
const options = {
protocol: url.protocol,
hostname: typeof url.hostname === 'string' &&
StringPrototypeStartsWith(url.hostname, '[') ?
hostname: url.hostname && StringPrototypeStartsWith(url.hostname, '[') ?
StringPrototypeSlice(url.hostname, 1, -1) :
url.hostname,
anonrig marked this conversation as resolved.
Show resolved Hide resolved
hash: url.hash,
Expand Down Expand Up @@ -1315,6 +1305,6 @@ module.exports = {
domainToASCII,
domainToUnicode,
urlToHttpOptions,
searchParamsSymbol: searchParams,
encodeStr,
isURL,
};
7 changes: 7 additions & 0 deletions test/parallel/test-whatwg-url-properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ const { URL, URLSearchParams, format } = require('url');
assert.strictEqual(params.size, 3);
}

{
const u = new URL('https://abc.com/?q=old');
const s = u.searchParams;
u.href = 'http://abc.com/?q=new';
assert.strictEqual(s.get('q'), 'new');
}

function stringifyName(name) {
if (typeof name === 'symbol') {
const { description } = name;
Expand Down