From 0dfc0f570295ca8809a9023b7ab64b73aa765ff6 Mon Sep 17 00:00:00 2001 From: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> Date: Tue, 5 Jan 2021 13:20:00 +0100 Subject: [PATCH] =?UTF-8?q?url:=20fix=C2=A0definitions=20of=C2=A0`URL`/`Se?= =?UTF-8?q?archParams`=C2=A0methods=20and=C2=A0accessors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes getter and setter names for the WHATWG URL classes, and fixes a few other inconsistencies with browsers implementations. Co-authored-by: Gerhard Stöbich PR-URL: https://github.com/nodejs/node/pull/36799 Reviewed-By: Antoine du Hamel Reviewed-By: Tiancheng "Timothy" Gu --- lib/internal/url.js | 620 ++++++++++---------- test/parallel/test-whatwg-url-properties.js | 100 ++++ 2 files changed, 404 insertions(+), 316 deletions(-) create mode 100644 test/parallel/test-whatwg-url-properties.js diff --git a/lib/internal/url.js b/lib/internal/url.js index 0584fd83983c62..4fd7702367e8a6 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -257,9 +257,7 @@ class URLSearchParams { } return `${this.constructor.name} {}`; } -} -defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { append(name, value) { if (!this || !this[searchParams] || this[searchParams][searchParams]) { throw new ERR_INVALID_THIS('URLSearchParams'); @@ -272,7 +270,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { value = toUSVString(value); ArrayPrototypePush(this[searchParams], name, value); update(this[context], this); - }, + } delete(name) { if (!this || !this[searchParams] || this[searchParams][searchParams]) { @@ -293,7 +291,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { } } update(this[context], this); - }, + } get(name) { if (!this || !this[searchParams] || this[searchParams][searchParams]) { @@ -311,7 +309,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { } } return null; - }, + } getAll(name) { if (!this || !this[searchParams] || this[searchParams][searchParams]) { @@ -330,7 +328,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { } } return values; - }, + } has(name) { if (!this || !this[searchParams] || this[searchParams][searchParams]) { @@ -348,7 +346,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { } } return false; - }, + } set(name, value) { if (!this || !this[searchParams] || this[searchParams][searchParams]) { @@ -388,7 +386,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { } update(this[context], this); - }, + } sort() { const a = this[searchParams]; @@ -432,7 +430,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { } update(this[context], this); - }, + } // https://heycam.github.io/webidl/#es-iterators // Define entries here rather than [Symbol.iterator] as the function name @@ -443,7 +441,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { } return createSearchParamsIterator(this, 'key+value'); - }, + } forEach(callback, thisArg = undefined) { if (!this || !this[searchParams] || this[searchParams][searchParams]) { @@ -462,7 +460,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { list = this[searchParams]; i += 2; } - }, + } // https://heycam.github.io/webidl/#es-iterable keys() { @@ -471,7 +469,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { } return createSearchParamsIterator(this, 'key'); - }, + } values() { if (!this || !this[searchParams] || this[searchParams][searchParams]) { @@ -479,7 +477,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { } return createSearchParamsIterator(this, 'value'); - }, + } // https://heycam.github.io/webidl/#es-stringifier // https://url.spec.whatwg.org/#urlsearchparams-stringification-behavior @@ -490,13 +488,29 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', { return serializeParams(this[searchParams]); } -}); +} -// https://heycam.github.io/webidl/#es-iterable-entries -ObjectDefineProperty(URLSearchParams.prototype, SymbolIterator, { - writable: true, - configurable: true, - value: URLSearchParams.prototype.entries +ObjectDefineProperties(URLSearchParams.prototype, { + append: { enumerable: true }, + delete: { enumerable: true }, + get: { enumerable: true }, + getAll: { enumerable: true }, + has: { enumerable: true }, + set: { enumerable: true }, + sort: { enumerable: true }, + entries: { enumerable: true }, + forEach: { enumerable: true }, + keys: { enumerable: true }, + values: { enumerable: true }, + toString: { enumerable: true }, + [SymbolToStringTag]: { configurable: true, value: 'URLSearchParams' }, + + // https://heycam.github.io/webidl/#es-iterable-entries + [SymbolIterator]: { + configurable: true, + writable: true, + value: URLSearchParams.prototype.entries, + }, }); function onParseComplete(flags, protocol, username, password, @@ -648,319 +662,293 @@ class URL { return `${constructor.name} ${inspect(obj, opts)}`; } -} -ObjectDefineProperties(URL.prototype, { - [kFormat]: { - enumerable: false, - configurable: false, - // eslint-disable-next-line func-name-matching - value: function format(options) { - if (options) - validateObject(options, 'options'); - - options = { - fragment: true, - unicode: false, - search: true, - auth: true, - ...options - }; - const ctx = this[context]; - // https://url.spec.whatwg.org/#url-serializing - let ret = ctx.scheme; - if (ctx.host !== null) { - ret += '//'; - const has_username = ctx.username !== ''; - const has_password = ctx.password !== ''; - if (options.auth && (has_username || has_password)) { - if (has_username) - ret += ctx.username; - if (has_password) - ret += `:${ctx.password}`; - ret += '@'; - } - ret += options.unicode ? - domainToUnicode(ctx.host) : ctx.host; - if (ctx.port !== null) - ret += `:${ctx.port}`; - } - if (this[cannotBeBase]) { - ret += ctx.path[0]; - } else { - if (ctx.host === null && ctx.path.length > 1 && ctx.path[0] === '') { - ret += '/.'; - } - if (ctx.path.length) { - ret += '/' + ArrayPrototypeJoin(ctx.path, '/'); - } - } - if (options.search && ctx.query !== null) - ret += `?${ctx.query}`; - if (options.fragment && ctx.fragment !== null) - ret += `#${ctx.fragment}`; - return ret; - } - }, - [SymbolToStringTag]: { - configurable: true, - value: 'URL' - }, - toString: { - // https://heycam.github.io/webidl/#es-stringifier - writable: true, - enumerable: true, - configurable: true, - // eslint-disable-next-line func-name-matching - value: function toString() { - return this[kFormat]({}); - } - }, - href: { - enumerable: true, - configurable: true, - get() { - return this[kFormat]({}); - }, - set(input) { - // toUSVString is not needed. - input = `${input}`; - parse(input, -1, undefined, undefined, - FunctionPrototypeBind(onParseComplete, this), onParseError); - } - }, - origin: { // readonly - enumerable: true, - configurable: true, - get() { - // Refs: https://url.spec.whatwg.org/#concept-url-origin - const ctx = this[context]; - switch (ctx.scheme) { - case 'blob:': - if (ctx.path.length > 0) { - try { - return (new URL(ctx.path[0])).origin; - } catch { - // Fall through... do nothing - } - } - return kOpaqueOrigin; - case 'ftp:': - case 'http:': - case 'https:': - case 'ws:': - case 'wss:': - return serializeTupleOrigin(ctx.scheme, ctx.host, ctx.port); + [kFormat](options) { + if (options) + validateObject(options, 'options'); + + options = { + fragment: true, + unicode: false, + search: true, + auth: true, + ...options + }; + const ctx = this[context]; + // https://url.spec.whatwg.org/#url-serializing + let ret = ctx.scheme; + if (ctx.host !== null) { + ret += '//'; + const has_username = ctx.username !== ''; + const has_password = ctx.password !== ''; + if (options.auth && (has_username || has_password)) { + if (has_username) + ret += ctx.username; + if (has_password) + ret += `:${ctx.password}`; + ret += '@'; } - return kOpaqueOrigin; - } - }, - protocol: { - enumerable: true, - configurable: true, - get() { - return this[context].scheme; - }, - set(scheme) { - // toUSVString is not needed. - scheme = `${scheme}`; - if (scheme.length === 0) - return; - const ctx = this[context]; - parse(scheme, kSchemeStart, null, ctx, - FunctionPrototypeBind(onParseProtocolComplete, this)); + ret += options.unicode ? + domainToUnicode(ctx.host) : ctx.host; + if (ctx.port !== null) + ret += `:${ctx.port}`; } - }, - username: { - enumerable: true, - configurable: true, - get() { - return this[context].username; - }, - set(username) { - // toUSVString is not needed. - username = `${username}`; - if (this[cannotHaveUsernamePasswordPort]) - return; - const ctx = this[context]; - if (username === '') { - ctx.username = ''; - ctx.flags &= ~URL_FLAGS_HAS_USERNAME; - return; + if (this[cannotBeBase]) { + ret += ctx.path[0]; + } else { + if (ctx.host === null && ctx.path.length > 1 && ctx.path[0] === '') { + ret += '/.'; } - ctx.username = encodeAuth(username); - ctx.flags |= URL_FLAGS_HAS_USERNAME; - } - }, - password: { - enumerable: true, - configurable: true, - get() { - return this[context].password; - }, - set(password) { - // toUSVString is not needed. - password = `${password}`; - if (this[cannotHaveUsernamePasswordPort]) - return; - const ctx = this[context]; - if (password === '') { - ctx.password = ''; - ctx.flags &= ~URL_FLAGS_HAS_PASSWORD; - return; + if (ctx.path.length) { + ret += '/' + ArrayPrototypeJoin(ctx.path, '/'); } - ctx.password = encodeAuth(password); - ctx.flags |= URL_FLAGS_HAS_PASSWORD; } - }, - host: { - enumerable: true, - configurable: true, - get() { - const ctx = this[context]; - let ret = ctx.host || ''; - if (ctx.port !== null) - ret += `:${ctx.port}`; - return ret; - }, - set(host) { - const ctx = this[context]; - // toUSVString is not needed. - host = `${host}`; - if (this[cannotBeBase]) { - // Cannot set the host if cannot-be-base is set - return; - } - parse(host, kHost, null, ctx, - FunctionPrototypeBind(onParseHostComplete, this)); + if (options.search && ctx.query !== null) + ret += `?${ctx.query}`; + if (options.fragment && ctx.fragment !== null) + ret += `#${ctx.fragment}`; + return ret; + } + + // https://heycam.github.io/webidl/#es-stringifier + toString() { + return this[kFormat]({}); + } + + get href() { + return this[kFormat]({}); + } + + set href(input) { + // toUSVString is not needed. + input = `${input}`; + parse(input, -1, undefined, undefined, + FunctionPrototypeBind(onParseComplete, this), onParseError); + } + + // readonly + get origin() { + // Refs: https://url.spec.whatwg.org/#concept-url-origin + const ctx = this[context]; + switch (ctx.scheme) { + case 'blob:': + if (ctx.path.length > 0) { + try { + return (new URL(ctx.path[0])).origin; + } catch { + // Fall through... do nothing + } + } + return kOpaqueOrigin; + case 'ftp:': + case 'http:': + case 'https:': + case 'ws:': + case 'wss:': + return serializeTupleOrigin(ctx.scheme, ctx.host, ctx.port); } - }, - hostname: { - enumerable: true, - configurable: true, - get() { - return this[context].host || ''; - }, - set(host) { - const ctx = this[context]; - // toUSVString is not needed. - host = `${host}`; - if (this[cannotBeBase]) { - // Cannot set the host if cannot-be-base is set - return; - } - parse(host, kHostname, null, ctx, onParseHostnameComplete.bind(this)); + return kOpaqueOrigin; + } + + get protocol() { + return this[context].scheme; + } + + set protocol(scheme) { + // toUSVString is not needed. + scheme = `${scheme}`; + if (scheme.length === 0) + return; + const ctx = this[context]; + parse(scheme, kSchemeStart, null, ctx, + FunctionPrototypeBind(onParseProtocolComplete, this)); + } + + get username() { + return this[context].username; + } + + set username(username) { + // toUSVString is not needed. + username = `${username}`; + if (this[cannotHaveUsernamePasswordPort]) + return; + const ctx = this[context]; + if (username === '') { + ctx.username = ''; + ctx.flags &= ~URL_FLAGS_HAS_USERNAME; + return; } - }, - port: { - enumerable: true, - configurable: true, - get() { - const port = this[context].port; - return port === null ? '' : String(port); - }, - set(port) { - // toUSVString is not needed. - port = `${port}`; - if (this[cannotHaveUsernamePasswordPort]) - return; - const ctx = this[context]; - if (port === '') { - ctx.port = null; - return; - } - parse(port, kPort, null, ctx, - FunctionPrototypeBind(onParsePortComplete, this)); + ctx.username = encodeAuth(username); + ctx.flags |= URL_FLAGS_HAS_USERNAME; + } + + get password() { + return this[context].password; + } + + set password(password) { + // toUSVString is not needed. + password = `${password}`; + if (this[cannotHaveUsernamePasswordPort]) + return; + const ctx = this[context]; + if (password === '') { + ctx.password = ''; + ctx.flags &= ~URL_FLAGS_HAS_PASSWORD; + return; } - }, - pathname: { - enumerable: true, - configurable: true, - get() { - const ctx = this[context]; - if (this[cannotBeBase]) - return ctx.path[0]; - if (ctx.path.length === 0) - return ''; - return `/${ArrayPrototypeJoin(ctx.path, '/')}`; - }, - set(path) { - // toUSVString is not needed. - path = `${path}`; - if (this[cannotBeBase]) - return; - parse(path, kPathStart, null, this[context], - onParsePathComplete.bind(this)); + ctx.password = encodeAuth(password); + ctx.flags |= URL_FLAGS_HAS_PASSWORD; + } + + get host() { + const ctx = this[context]; + let ret = ctx.host || ''; + if (ctx.port !== null) + ret += `:${ctx.port}`; + return ret; + } + + set host(host) { + const ctx = this[context]; + // toUSVString is not needed. + host = `${host}`; + if (this[cannotBeBase]) { + // Cannot set the host if cannot-be-base is set + return; } - }, - search: { - enumerable: true, - configurable: true, - get() { - const { query } = this[context]; - if (query === null || query === '') - return ''; - return `?${query}`; - }, - set(search) { - const ctx = this[context]; - search = toUSVString(search); - if (search === '') { - ctx.query = null; - ctx.flags &= ~URL_FLAGS_HAS_QUERY; - } else { - if (search[0] === '?') search = StringPrototypeSlice(search, 1); - ctx.query = ''; - ctx.flags |= URL_FLAGS_HAS_QUERY; - if (search) { - parse(search, kQuery, null, ctx, - FunctionPrototypeBind(onParseSearchComplete, this)); - } - } - initSearchParams(this[searchParams], search); + parse(host, kHost, null, ctx, + FunctionPrototypeBind(onParseHostComplete, this)); + } + + get hostname() { + return this[context].host || ''; + } + + set hostname(host) { + const ctx = this[context]; + // toUSVString is not needed. + host = `${host}`; + if (this[cannotBeBase]) { + // Cannot set the host if cannot-be-base is set + return; } - }, - searchParams: { // readonly - enumerable: true, - configurable: true, - get() { - return this[searchParams]; + parse(host, kHostname, null, ctx, onParseHostnameComplete.bind(this)); + } + + get port() { + const port = this[context].port; + return port === null ? '' : String(port); + } + + set port(port) { + // toUSVString is not needed. + port = `${port}`; + if (this[cannotHaveUsernamePasswordPort]) + return; + const ctx = this[context]; + if (port === '') { + ctx.port = null; + return; } - }, - hash: { - enumerable: true, - configurable: true, - get() { - const { fragment } = this[context]; - if (fragment === null || fragment === '') - return ''; - return `#${fragment}`; - }, - set(hash) { - const ctx = this[context]; - // toUSVString is not needed. - hash = `${hash}`; - if (!hash) { - ctx.fragment = null; - ctx.flags &= ~URL_FLAGS_HAS_FRAGMENT; - return; + parse(port, kPort, null, ctx, + FunctionPrototypeBind(onParsePortComplete, this)); + } + + get pathname() { + const ctx = this[context]; + if (this[cannotBeBase]) + return ctx.path[0]; + if (ctx.path.length === 0) + return ''; + return `/${ArrayPrototypeJoin(ctx.path, '/')}`; + } + + set pathname(path) { + // toUSVString is not needed. + path = `${path}`; + if (this[cannotBeBase]) + return; + parse(path, kPathStart, null, this[context], + onParsePathComplete.bind(this)); + } + + get search() { + const { query } = this[context]; + if (query === null || query === '') + return ''; + return `?${query}`; + } + + set search(search) { + const ctx = this[context]; + search = toUSVString(search); + if (search === '') { + ctx.query = null; + ctx.flags &= ~URL_FLAGS_HAS_QUERY; + } else { + if (search[0] === '?') search = StringPrototypeSlice(search, 1); + ctx.query = ''; + ctx.flags |= URL_FLAGS_HAS_QUERY; + if (search) { + parse(search, kQuery, null, ctx, + FunctionPrototypeBind(onParseSearchComplete, this)); } - if (hash[0] === '#') hash = StringPrototypeSlice(hash, 1); - ctx.fragment = ''; - ctx.flags |= URL_FLAGS_HAS_FRAGMENT; - parse(hash, kFragment, null, ctx, - FunctionPrototypeBind(onParseHashComplete, this)); } - }, - toJSON: { - writable: true, - enumerable: true, - configurable: true, - // eslint-disable-next-line func-name-matching - value: function toJSON() { - return this[kFormat]({}); + initSearchParams(this[searchParams], search); + } + + // readonly + get searchParams() { + return this[searchParams]; + } + + get hash() { + const { fragment } = this[context]; + if (fragment === null || fragment === '') + return ''; + return `#${fragment}`; + } + + set hash(hash) { + const ctx = this[context]; + // toUSVString is not needed. + hash = `${hash}`; + if (!hash) { + ctx.fragment = null; + ctx.flags &= ~URL_FLAGS_HAS_FRAGMENT; + return; } + if (hash[0] === '#') hash = StringPrototypeSlice(hash, 1); + ctx.fragment = ''; + ctx.flags |= URL_FLAGS_HAS_FRAGMENT; + parse(hash, kFragment, null, ctx, + FunctionPrototypeBind(onParseHashComplete, this)); } + + toJSON() { + return this[kFormat]({}); + } +} + +ObjectDefineProperties(URL.prototype, { + [kFormat]: { configurable: false, writable: false }, + [SymbolToStringTag]: { configurable: true, value: 'URL' }, + toString: { enumerable: true }, + href: { enumerable: true }, + origin: { enumerable: true }, + protocol: { enumerable: true }, + username: { enumerable: true }, + password: { enumerable: true }, + host: { enumerable: true }, + hostname: { enumerable: true }, + port: { enumerable: true }, + pathname: { enumerable: true }, + search: { enumerable: true }, + searchParams: { enumerable: true }, + hash: { enumerable: true }, + toJSON: { enumerable: true }, }); function update(url, params) { diff --git a/test/parallel/test-whatwg-url-properties.js b/test/parallel/test-whatwg-url-properties.js new file mode 100644 index 00000000000000..a387b0eb753e1a --- /dev/null +++ b/test/parallel/test-whatwg-url-properties.js @@ -0,0 +1,100 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const { URL, URLSearchParams } = require('url'); + +[ + { name: 'toString' }, + { name: 'toJSON' }, + { name: Symbol.for('nodejs.util.inspect.custom') }, +].forEach(({ name }) => { + testMethod(URL.prototype, name); +}); + +[ + { name: 'href' }, + { name: 'protocol' }, + { name: 'username' }, + { name: 'password' }, + { name: 'host' }, + { name: 'hostname' }, + { name: 'port' }, + { name: 'pathname' }, + { name: 'search' }, + { name: 'hash' }, + { name: 'origin', readonly: true }, + { name: 'searchParams', readonly: true }, +].forEach(({ name, readonly = false }) => { + testAccessor(URL.prototype, name, readonly); +}); + +[ + { name: 'append' }, + { name: 'delete' }, + { name: 'get' }, + { name: 'getAll' }, + { name: 'has' }, + { name: 'set' }, + { name: 'sort' }, + { name: 'entries' }, + { name: 'forEach' }, + { name: 'keys' }, + { name: 'values' }, + { name: 'toString' }, + { name: Symbol.iterator, methodName: 'entries' }, + { name: Symbol.for('nodejs.util.inspect.custom') }, +].forEach(({ name, methodName }) => { + testMethod(URLSearchParams.prototype, name, methodName); +}); + +function stringifyName(name) { + if (typeof name === 'symbol') { + const { description } = name; + if (description === undefined) { + return ''; + } + return `[${description}]`; + } + + return name; +} + +function testMethod(target, name, methodName = stringifyName(name)) { + const desc = Object.getOwnPropertyDescriptor(target, name); + assert.notStrictEqual(desc, undefined); + assert.strictEqual(desc.enumerable, typeof name === 'string'); + + const { value } = desc; + assert.strictEqual(typeof value, 'function'); + assert.strictEqual(value.name, methodName); + assert.strictEqual( + Object.prototype.hasOwnProperty.call(value, 'prototype'), + false, + ); +} + +function testAccessor(target, name, readonly = false) { + const desc = Object.getOwnPropertyDescriptor(target, name); + assert.notStrictEqual(desc, undefined); + assert.strictEqual(desc.enumerable, typeof name === 'string'); + + const methodName = stringifyName(name); + const { get, set } = desc; + assert.strictEqual(typeof get, 'function'); + assert.strictEqual(get.name, `get ${methodName}`); + assert.strictEqual( + Object.prototype.hasOwnProperty.call(get, 'prototype'), + false, + ); + + if (readonly) { + assert.strictEqual(set, undefined); + } else { + assert.strictEqual(typeof set, 'function'); + assert.strictEqual(set.name, `set ${methodName}`); + assert.strictEqual( + Object.prototype.hasOwnProperty.call(set, 'prototype'), + false, + ); + } +}