url: use private properties for brand check

anonrig · anonrig · commit 90cd20de7130 · 2023-06-23T17:03:11.000-04:00
PR-URL: nodejs#46904 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -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,
@@ -1363,7 +1363,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);
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -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,
@@ -792,7 +792,7 @@ class ESMLoader {
     if (
       !isMain &&
       typeof parentURL !== 'string' &&
-      !isURLInstance(parentURL)
+      !isURL(parentURL)
     ) {
       throw new ERR_INVALID_ARG_TYPE(
         'parentURL',
diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -7,7 +7,7 @@ const {
   ArrayPrototypePush,
   ArrayPrototypeReduce,
   ArrayPrototypeSlice,
-  FunctionPrototypeBind,
+  Boolean,
   Int8Array,
   IteratorPrototype,
   Number,
@@ -17,7 +17,6 @@ const {
   ObjectGetOwnPropertySymbols,
   ObjectGetPrototypeOf,
   ObjectKeys,
-  ObjectPrototypeHasOwnProperty,
   ReflectGetOwnPropertyDescriptor,
   ReflectOwnKeys,
   RegExpPrototypeSymbolReplace,
@@ -536,16 +535,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();
-    this.#onParseComplete = FunctionPrototypeBind(this.#onParseComplete, this);
 
     if (base !== undefined) {
       base = `${base}`;
@@ -561,11 +571,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;
 
@@ -586,182 +591,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) {
@@ -1209,7 +1165,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');
@@ -1285,12 +1241,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);
 }
@@ -1300,7 +1252,6 @@ module.exports = {
   fileURLToPath,
   pathToFileURL,
   toPathIfFileURL,
-  isURLInstance,
   URL,
   URLSearchParams,
   domainToASCII,
diff --git a/lib/internal/worker.js b/lib/internal/worker.js
@@ -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');
 
@@ -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') {
diff --git a/test/parallel/test-whatwg-url-custom-inspect.js b/test/parallel/test-whatwg-url-custom-inspect.js
@@ -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 {'));
diff --git a/test/parallel/test-whatwg-url-invalidthis.js b/test/parallel/test-whatwg-url-invalidthis.js
