Skip to content

Commit

Permalink
url: use private properties for brand check
Browse files Browse the repository at this point in the history
PR-URL: #46904
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
anonrig authored Mar 3, 2023
1 parent 5a7491b commit 027841c
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 117 deletions.
4 changes: 2 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const { BuiltinModule } = require('internal/bootstrap/loaders');
const {
maybeCacheSourceMap,
} = require('internal/source_map/source_map_cache');
const { pathToFileURL, fileURLToPath, isURLInstance } = require('internal/url');
const { pathToFileURL, fileURLToPath, isURL } = require('internal/url');
const {
deprecate,
emitExperimentalWarning,
Expand Down Expand Up @@ -1396,7 +1396,7 @@ const createRequireError = 'must be a file URL object, file URL string, or ' +
function createRequire(filename) {
let filepath;

if (isURLInstance(filename) ||
if (isURL(filename) ||
(typeof filename === 'string' && !path.isAbsolute(filename))) {
try {
filepath = fileURLToPath(filename);
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const {
ERR_INVALID_RETURN_PROPERTY_VALUE,
ERR_INVALID_RETURN_VALUE,
} = require('internal/errors').codes;
const { isURLInstance, URL } = require('internal/url');
const { isURL, URL } = require('internal/url');
const {
isAnyArrayBuffer,
isArrayBufferView,
Expand Down Expand Up @@ -263,7 +263,7 @@ class Hooks {
if (
!isMain &&
typeof parentURL !== 'string' &&
!isURLInstance(parentURL)
!isURL(parentURL)
) {
throw new ERR_INVALID_ARG_TYPE(
'parentURL',
Expand Down
163 changes: 58 additions & 105 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const {
ArrayPrototypePush,
ArrayPrototypeReduce,
ArrayPrototypeSlice,
Boolean,
Int8Array,
IteratorPrototype,
Number,
Expand All @@ -15,7 +16,6 @@ const {
ObjectGetOwnPropertySymbols,
ObjectGetPrototypeOf,
ObjectKeys,
ObjectPrototypeHasOwnProperty,
ReflectGetOwnPropertyDescriptor,
ReflectOwnKeys,
RegExpPrototypeSymbolReplace,
Expand Down Expand Up @@ -534,15 +534,27 @@ ObjectDefineProperties(URLSearchParams.prototype, {
},
});

/**
* Checks if a value has the shape of a WHATWG URL object.
*
* Using a symbol or instanceof would not be able to recognize URL objects
* coming from other implementations (e.g. in Electron), so instead we are
* checking some well known properties for a lack of a better test.
*
* @param {*} self
* @returns {self is URL}
*/
function isURL(self) {
return self != null && ObjectPrototypeHasOwnProperty(self, context);
return Boolean(self?.href && self.origin);
}

class URL {
#context = new URLContext();
#searchParams;

constructor(input, base = undefined) {
// toUSVString is not needed.
input = `${input}`;
this[context] = new URLContext();

if (base !== undefined) {
base = `${base}`;
Expand All @@ -558,11 +570,6 @@ class URL {
}

[inspect.custom](depth, opts) {
if (this == null ||
ObjectGetPrototypeOf(this[context]) !== URLContext.prototype) {
throw new ERR_INVALID_THIS('URL');
}

if (typeof depth === 'number' && depth < 0)
return this;

Expand All @@ -583,182 +590,133 @@ class URL {
obj.hash = this.hash;

if (opts.showHidden) {
obj[context] = this[context];
obj[context] = this.#context;
}

return `${constructor.name} ${inspect(obj, opts)}`;
}

#onParseComplete = (href, origin, protocol, hostname, pathname,
search, username, password, port, hash) => {
const ctx = this[context];
ctx.href = href;
ctx.origin = origin;
ctx.protocol = protocol;
ctx.hostname = hostname;
ctx.pathname = pathname;
ctx.search = search;
ctx.username = username;
ctx.password = password;
ctx.port = port;
ctx.hash = hash;
if (this[searchParams]) {
this[searchParams][searchParams] = parseParams(search);
this.#context.href = href;
this.#context.origin = origin;
this.#context.protocol = protocol;
this.#context.hostname = hostname;
this.#context.pathname = pathname;
this.#context.search = search;
this.#context.username = username;
this.#context.password = password;
this.#context.port = port;
this.#context.hash = hash;
if (this.#searchParams) {
this.#searchParams[searchParams] = parseParams(search);
}
};

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

// readonly
get searchParams() {
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;
if (this.#searchParams == null) {
this.#searchParams = new URLSearchParams(this.#context.search);
this.#searchParams[context] = this;
}
return this[searchParams];
return this.#searchParams;
}

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

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

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

static createObjectURL(obj) {
Expand Down Expand Up @@ -1206,7 +1164,7 @@ function getPathFromURLPosix(url) {
function fileURLToPath(path) {
if (typeof path === 'string')
path = new URL(path);
else if (!isURLInstance(path))
else if (!isURL(path))
throw new ERR_INVALID_ARG_TYPE('path', ['string', 'URL'], path);
if (path.protocol !== 'file:')
throw new ERR_INVALID_URL_SCHEME('file');
Expand Down Expand Up @@ -1282,12 +1240,8 @@ function pathToFileURL(filepath) {
return outURL;
}

function isURLInstance(fileURLOrPath) {
return fileURLOrPath != null && fileURLOrPath.href && fileURLOrPath.origin;
}

function toPathIfFileURL(fileURLOrPath) {
if (!isURLInstance(fileURLOrPath))
if (!isURL(fileURLOrPath))
return fileURLOrPath;
return fileURLToPath(fileURLOrPath);
}
Expand All @@ -1297,7 +1251,6 @@ module.exports = {
fileURLToPath,
pathToFileURL,
toPathIfFileURL,
isURLInstance,
URL,
URLSearchParams,
domainToASCII,
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const {
WritableWorkerStdio,
} = workerIo;
const { deserializeError } = require('internal/error_serdes');
const { fileURLToPath, isURLInstance, pathToFileURL } = require('internal/url');
const { fileURLToPath, isURL, pathToFileURL } = require('internal/url');
const { kEmptyObject } = require('internal/util');
const { validateArray } = require('internal/validators');

Expand Down Expand Up @@ -148,13 +148,13 @@ class Worker extends EventEmitter {
}
url = null;
doEval = 'classic';
} else if (isURLInstance(filename) && filename.protocol === 'data:') {
} else if (isURL(filename) && filename.protocol === 'data:') {
url = null;
doEval = 'module';
filename = `import ${JSONStringify(`${filename}`)}`;
} else {
doEval = false;
if (isURLInstance(filename)) {
if (isURL(filename)) {
url = filename;
filename = fileURLToPath(filename);
} else if (typeof filename !== 'string') {
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-whatwg-url-custom-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ assert.strictEqual(

assert.strictEqual(
util.inspect({ a: url }, { depth: 0 }),
'{ a: [URL] }');
'{ a: URL {} }');

class MyURL extends URL {}
assert(util.inspect(new MyURL(url.href)).startsWith('MyURL {'));
Loading

0 comments on commit 027841c

Please sign in to comment.