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: nodejs#46904
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
  • Loading branch information
anonrig committed Jun 17, 2023
1 parent 0dbacb1 commit c7d0951
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 122 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 @@ -1361,7 +1361,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/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const {
ERR_INVALID_RETURN_VALUE,
ERR_UNKNOWN_MODULE_FORMAT,
} = require('internal/errors').codes;
const { pathToFileURL, isURLInstance, URL } = require('internal/url');
const { pathToFileURL, isURL, URL } = require('internal/url');
const { emitExperimentalWarning } = require('internal/util');
const {
isAnyArrayBuffer,
Expand Down Expand Up @@ -792,7 +792,7 @@ class ESMLoader {
if (
!isMain &&
typeof parentURL !== 'string' &&
!isURLInstance(parentURL)
!isURL(parentURL)
) {
throw new ERR_INVALID_ARG_TYPE(
'parentURL',
Expand Down
173 changes: 63 additions & 110 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const {
ArrayPrototypePush,
ArrayPrototypeReduce,
ArrayPrototypeSlice,
FunctionPrototypeBind,
Boolean,
Int8Array,
IteratorPrototype,
Number,
Expand All @@ -17,7 +17,6 @@ const {
ObjectGetOwnPropertySymbols,
ObjectGetPrototypeOf,
ObjectKeys,
ObjectPrototypeHasOwnProperty,
ReflectGetOwnPropertyDescriptor,
ReflectOwnKeys,
RegExpPrototypeSymbolReplace,
Expand Down Expand Up @@ -537,16 +536,27 @@ ObjectDefineProperties(URLSearchParams.prototype, {
},
});

function isURLThis(self) {
return self != null && ObjectPrototypeHasOwnProperty(self, context);
/**
* 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 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();
this.#onParseComplete = FunctionPrototypeBind(this.#onParseComplete, this);

if (base !== undefined) {
base = `${base}`;
Expand All @@ -562,11 +572,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 @@ -587,181 +592,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]) { // Invoked from URL constructor
this[searchParams] = new URLSearchParams();
this[searchParams][context] = this;
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);
}
initSearchParams(this[searchParams], ctx.search);
};

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

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

set href(value) {
if (!isURLThis(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 (!isURLThis(this))
throw new ERR_INVALID_THIS('URL');
return this[context].origin;
return this.#context.origin;
}

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

set protocol(value) {
if (!isURLThis(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 (!isURLThis(this))
throw new ERR_INVALID_THIS('URL');
return this[context].username;
return this.#context.username;
}

set username(value) {
if (!isURLThis(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 (!isURLThis(this))
throw new ERR_INVALID_THIS('URL');
return this[context].password;
return this.#context.password;
}

set password(value) {
if (!isURLThis(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 (!isURLThis(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 (!isURLThis(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 (!isURLThis(this))
throw new ERR_INVALID_THIS('URL');
return this[context].hostname;
return this.#context.hostname;
}

set hostname(value) {
if (!isURLThis(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 (!isURLThis(this))
throw new ERR_INVALID_THIS('URL');
return this[context].port;
return this.#context.port;
}

set port(value) {
if (!isURLThis(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 (!isURLThis(this))
throw new ERR_INVALID_THIS('URL');
return this[context].pathname;
return this.#context.pathname;
}

set pathname(value) {
if (!isURLThis(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 (!isURLThis(this))
throw new ERR_INVALID_THIS('URL');
return this[context].search;
return this.#context.search;
}

set search(search) {
if (!isURLThis(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);
set search(value) {
updateUrl(this.#context.href, updateActions.kSearch, toUSVString(value), this.#onParseComplete);
}

// readonly
get searchParams() {
if (!isURLThis(this))
throw new ERR_INVALID_THIS('URL');
return this[searchParams];
// 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))
throw new ERR_INVALID_THIS('URL');
return this[context].hash;
return this.#context.hash;
}

set hash(value) {
if (!isURLThis(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 (!isURLThis(this))
throw new ERR_INVALID_THIS('URL');
return this[context].href;
return this.#context.href;
}

static createObjectURL(obj) {
Expand Down Expand Up @@ -1219,7 +1176,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 @@ -1295,12 +1252,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 @@ -1310,12 +1263,12 @@ module.exports = {
fileURLToPath,
pathToFileURL,
toPathIfFileURL,
isURLInstance,
URL,
URLSearchParams,
domainToASCII,
domainToUnicode,
urlToHttpOptions,
searchParamsSymbol: searchParams,
encodeStr,
isURL,
};
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, validateString } = require('internal/validators');

Expand Down Expand Up @@ -145,13 +145,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 c7d0951

Please sign in to comment.