Repository: nodejs/node
Pull request: url: more sophisticated brand check for URLSearchParams

Use private properties, following the lead of the `URL` class.

<!--
Before submitting a pull request, please read
https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md.

Commit message formatting guidelines:
https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/internal/url.js
patch: @@ -13,10 +13,7 @@ const {
   IteratorPrototype,
   Number,
   ObjectDefineProperties,
-  ObjectDefineProperty,
-  ObjectGetOwnPropertySymbols,
-  ObjectGetPrototypeOf,
-  ObjectKeys,
+  ObjectSetPrototypeOf,
   ReflectGetOwnPropertyDescriptor,
   ReflectOwnKeys,
   RegExpPrototypeSymbolReplace,
@@ -90,8 +87,7 @@ const bindingUrl = internalBinding('url');
 
 const FORWARD_SLASH = /\//g;
 
-const context = Symbol('context');
-const searchParams = Symbol('query');
+const contextForInspect = Symbol('context');
 
 const updateActions = {
   kProtocol: 0,
@@ -172,15 +168,124 @@ class URLContext {
   }
 }
 
-function isURLSearchParams(self) {
-  return self && self[searchParams] && !self[searchParams][searchParams];
+let setURLSearchParamsContext;
+let getURLSearchParamsList;
+let setURLSearchParams;
+
+class URLSearchParamsIterator {
+  #target;
+  #kind;
+  #index;
+
+  // https://heycam.github.io/webidl/#dfn-default-iterator-object
+  constructor(target, kind) {
+    this.#target = target;
+    this.#kind = kind;
+    this.#index = 0;
+  }
+
+  next() {
+    if (typeof this !== 'object' || this === null || !(#target in this))
+      throw new ERR_INVALID_THIS('URLSearchParamsIterator');
+
+    const index = this.#index;
+    const values = getURLSearchParamsList(this.#target);
+    const len = values.length;
+    if (index >= len) {
+      return {
+        value: undefined,
+        done: true,
+      };
+    }
+
+    const name = values[index];
+    const value = values[index + 1];
+    this.#index = index + 2;
+
+    let result;
+    if (this.#kind === 'key') {
+      result = name;
+    } else if (this.#kind === 'value') {
+      result = value;
+    } else {
+      result = [name, value];
+    }
+
+    return {
+      value: result,
+      done: false,
+    };
+  }
+
+  [inspect.custom](recurseTimes, ctx) {
+    if (!this || typeof this !== 'object' || !(#target in this))
+      throw new ERR_INVALID_THIS('URLSearchParamsIterator');
+
+    if (typeof recurseTimes === 'number' && recurseTimes < 0)
+      return ctx.stylize('[Object]', 'special');
+
+    const innerOpts = { ...ctx };
+    if (recurseTimes !== null) {
+      innerOpts.depth = recurseTimes - 1;
+    }
+    const index = this.#index;
+    const values = getURLSearchParamsList(this.#target);
+    const output = ArrayPrototypeReduce(
+      ArrayPrototypeSlice(values, index),
+      (prev, cur, i) => {
+        const key = i % 2 === 0;
+        if (this.#kind === 'key' && key) {
+          ArrayPrototypePush(prev, cur);
+        } else if (this.#kind === 'value' && !key) {
+          ArrayPrototypePush(prev, cur);
+        } else if (this.#kind === 'key+value' && !key) {
+          ArrayPrototypePush(prev, [values[index + i - 1], cur]);
+        }
+        return prev;
+      },
+      [],
+    );
+    const breakLn = StringPrototypeIncludes(inspect(output, innerOpts), '\n');
+    const outputStrs = ArrayPrototypeMap(output, (p) => inspect(p, innerOpts));
+    let outputStr;
+    if (breakLn) {
+      outputStr = `\n  ${ArrayPrototypeJoin(outputStrs, ',\n  ')}`;
+    } else {
+      outputStr = ` ${ArrayPrototypeJoin(outputStrs, ', ')}`;
+    }
+    return `${this[SymbolToStringTag]} {${outputStr} }`;
+  }
 }
 
+// https://heycam.github.io/webidl/#dfn-iterator-prototype-object
+delete URLSearchParamsIterator.prototype.constructor;
+ObjectSetPrototypeOf(URLSearchParamsIterator.prototype, IteratorPrototype);
+
+ObjectDefineProperties(URLSearchParamsIterator.prototype, {
+  [SymbolToStringTag]: { __proto__: null, configurable: true, value: 'URLSearchParams Iterator' },
+  next: kEnumerableProperty,
+});
+
+
 class URLSearchParams {
-  [searchParams] = [];
+  #searchParams = [];
 
   // "associated url object"
-  [context] = null;
+  #context;
+
+  static {
+    setURLSearchParamsContext = (obj, ctx) => {
+      obj.#context = ctx;
+    };
+    getURLSearchParamsList = (obj) => obj.#searchParams;
+    setURLSearchParams = (obj, query) => {
+      if (query === undefined) {
+        obj.#searchParams = [];
+      } else {
+        obj.#searchParams = parseParams(query);
+      }
+    };
+  }
 
   // URL Standard says the default value is '', but as undefined and '' have
   // the same result, undefined is used to prevent unnecessary parsing.
@@ -191,11 +296,11 @@ class URLSearchParams {
       // Do nothing
     } else if (typeof init === 'object' || typeof init === 'function') {
       const method = init[SymbolIterator];
-      if (method === this[SymbolIterator]) {
+      if (method === this[SymbolIterator] && #searchParams in init) {
         // While the spec does not have this branch, we can use it as a
         // shortcut to avoid having to go through the costly generic iterator.
-        const childParams = init[searchParams];
-        this[searchParams] = childParams.slice();
+        const childParams = init.#searchParams;
+        this.#searchParams = childParams.slice();
       } else if (method != null) {
         // Sequence<sequence<USVString>>
         if (typeof method !== 'function') {
@@ -221,7 +326,7 @@ class URLSearchParams {
               throw new ERR_INVALID_TUPLE('Each query pair', '[name, value]');
             }
             // Append (innerSequence[0], innerSequence[1]) to querys list.
-            ArrayPrototypePush(this[searchParams], toUSVString(pair[0]), toUSVString(pair[1]));
+            ArrayPrototypePush(this.#searchParams, toUSVString(pair[0]), toUSVString(pair[1]));
           } else {
             if (((typeof pair !== 'object' && typeof pair !== 'function') ||
                 typeof pair[SymbolIterator] !== 'function')) {
@@ -232,7 +337,7 @@ class URLSearchParams {
 
             for (const element of pair) {
               length++;
-              ArrayPrototypePush(this[searchParams], toUSVString(element));
+              ArrayPrototypePush(this.#searchParams, toUSVString(element));
             }
 
             // If innerSequence's size is not 2, then throw a TypeError.
@@ -257,9 +362,9 @@ class URLSearchParams {
             // In that case, we retain the later one. Refer to WPT.
             const keyIdx = visited.get(typedKey);
             if (keyIdx !== undefined) {
-              this[searchParams][keyIdx] = typedValue;
+              this.#searchParams[keyIdx] = typedValue;
             } else {
-              visited.set(typedKey, ArrayPrototypePush(this[searchParams],
+              visited.set(typedKey, ArrayPrototypePush(this.#searchParams,
                                                        typedKey,
                                                        typedValue) - 1);
             }
@@ -269,12 +374,12 @@ class URLSearchParams {
     } else {
       // https://url.spec.whatwg.org/#dom-urlsearchparams-urlsearchparams
       init = toUSVString(init);
-      this[searchParams] = init ? parseParams(init) : [];
+      this.#searchParams = init ? parseParams(init) : [];
     }
   }
 
   [inspect.custom](recurseTimes, ctx) {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
     if (typeof recurseTimes === 'number' && recurseTimes < 0)
@@ -287,7 +392,7 @@ class URLSearchParams {
     }
     const innerInspect = (v) => inspect(v, innerOpts);
 
-    const list = this[searchParams];
+    const list = this.#searchParams;
     const output = [];
     for (let i = 0; i < list.length; i += 2)
       ArrayPrototypePush(
@@ -310,13 +415,13 @@ class URLSearchParams {
   }
 
   get size() {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
-    return this[searchParams].length / 2;
+    return this.#searchParams.length / 2;
   }
 
   append(name, value) {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
     if (arguments.length < 2) {
@@ -325,21 +430,21 @@ class URLSearchParams {
 
     name = toUSVString(name);
     value = toUSVString(value);
-    ArrayPrototypePush(this[searchParams], name, value);
-    if (this[context]) {
-      this[context].search = this.toString();
+    ArrayPrototypePush(this.#searchParams, name, value);
+    if (this.#context) {
+      this.#context.search = this.toString();
     }
   }
 
   delete(name) {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
     if (arguments.length < 1) {
       throw new ERR_MISSING_ARGS('name');
     }
 
-    const list = this[searchParams];
+    const list = this.#searchParams;
     name = toUSVString(name);
     for (let i = 0; i < list.length;) {
       const cur = list[i];
@@ -349,20 +454,20 @@ class URLSearchParams {
         i += 2;
       }
     }
-    if (this[context]) {
-      this[context].search = this.toString();
+    if (this.#context) {
+      this.#context.search = this.toString();
     }
   }
 
   get(name) {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
     if (arguments.length < 1) {
       throw new ERR_MISSING_ARGS('name');
     }
 
-    const list = this[searchParams];
+    const list = this.#searchParams;
     name = toUSVString(name);
     for (let i = 0; i < list.length; i += 2) {
       if (list[i] === name) {
@@ -373,14 +478,14 @@ class URLSearchParams {
   }
 
   getAll(name) {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
     if (arguments.length < 1) {
       throw new ERR_MISSING_ARGS('name');
     }
 
-    const list = this[searchParams];
+    const list = this.#searchParams;
     const values = [];
     name = toUSVString(name);
     for (let i = 0; i < list.length; i += 2) {
@@ -392,14 +497,14 @@ class URLSearchParams {
   }
 
   has(name) {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
     if (arguments.length < 1) {
       throw new ERR_MISSING_ARGS('name');
     }
 
-    const list = this[searchParams];
+    const list = this.#searchParams;
     name = toUSVString(name);
     for (let i = 0; i < list.length; i += 2) {
       if (list[i] === name) {
@@ -410,14 +515,14 @@ class URLSearchParams {
   }
 
   set(name, value) {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
     if (arguments.length < 2) {
       throw new ERR_MISSING_ARGS('name', 'value');
     }
 
-    const list = this[searchParams];
+    const list = this.#searchParams;
     name = toUSVString(name);
     value = toUSVString(value);
 
@@ -446,13 +551,16 @@ class URLSearchParams {
       ArrayPrototypePush(list, name, value);
     }
 
-    if (this[context]) {
-      this[context].search = this.toString();
+    if (this.#context) {
+      this.#context.search = this.toString();
     }
   }
 
   sort() {
-    const a = this[searchParams];
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
+      throw new ERR_INVALID_THIS('URLSearchParams');
+
+    const a = this.#searchParams;
     const len = a.length;
 
     if (len <= 2) {
@@ -492,62 +600,62 @@ class URLSearchParams {
       }
     }
 
-    if (this[context]) {
-      this[context].search = this.toString();
+    if (this.#context) {
+      this.#context.search = this.toString();
     }
   }
 
   // https://heycam.github.io/webidl/#es-iterators
   // Define entries here rather than [Symbol.iterator] as the function name
   // must be set to `entries`.
   entries() {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
-    return createSearchParamsIterator(this, 'key+value');
+    return new URLSearchParamsIterator(this, 'key+value');
   }
 
   forEach(callback, thisArg = undefined) {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
     validateFunction(callback, 'callback');
 
-    let list = this[searchParams];
+    let list = this.#searchParams;
 
     let i = 0;
     while (i < list.length) {
       const key = list[i];
       const value = list[i + 1];
       callback.call(thisArg, value, key, this);
       // In case the URL object's `search` is updated
-      list = this[searchParams];
+      list = this.#searchParams;
       i += 2;
     }
   }
 
   // https://heycam.github.io/webidl/#es-iterable
   keys() {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
-    return createSearchParamsIterator(this, 'key');
+    return new URLSearchParamsIterator(this, 'key');
   }
 
   values() {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
-    return createSearchParamsIterator(this, 'value');
+    return new URLSearchParamsIterator(this, 'value');
   }
 
   // https://heycam.github.io/webidl/#es-stringifier
   // https://url.spec.whatwg.org/#urlsearchparams-stringification-behavior
   toString() {
-    if (!isURLSearchParams(this))
+    if (typeof this !== 'object' || this === null || !(#searchParams in this))
       throw new ERR_INVALID_THIS('URLSearchParams');
 
-    return serializeParams(this[searchParams]);
+    return serializeParams(this.#searchParams);
   }
 }
 
@@ -635,7 +743,7 @@ class URL {
     obj.hash = this.hash;
 
     if (opts.showHidden) {
-      obj[context] = this.#context;
+      obj[contextForInspect] = this.#context;
     }
 
     return `${constructor.name} ${inspect(obj, opts)}`;
@@ -668,9 +776,9 @@ class URL {
 
     if (this.#searchParams) {
       if (this.#context.hasSearch) {
-        this.#searchParams[searchParams] = parseParams(this.search);
+        setURLSearchParams(this.#searchParams, this.search);
       } else {
-        this.#searchParams[searchParams] = [];
+        setURLSearchParams(this.#searchParams, undefined);
       }
     }
   }
@@ -846,7 +954,7 @@ class URL {
     // Create URLSearchParams on demand to greatly improve the URL performance.
     if (this.#searchParams == null) {
       this.#searchParams = new URLSearchParams(this.search);
-      this.#searchParams[context] = this;
+      setURLSearchParamsContext(this.#searchParams, this);
     }
     return this.#searchParams;
   }
@@ -1099,38 +1207,6 @@ function serializeParams(array) {
   return output;
 }
 
-// Mainly to mitigate func-name-matching ESLint rule
-function defineIDLClass(proto, classStr, obj) {
-  // https://heycam.github.io/webidl/#dfn-class-string
-  ObjectDefineProperty(proto, SymbolToStringTag, {
-    __proto__: null,
-    writable: false,
-    enumerable: false,
-    configurable: true,
-    value: classStr,
-  });
-
-  // https://heycam.github.io/webidl/#es-operations
-  for (const key of ObjectKeys(obj)) {
-    ObjectDefineProperty(proto, key, {
-      __proto__: null,
-      writable: true,
-      enumerable: true,
-      configurable: true,
-      value: obj[key],
-    });
-  }
-  for (const key of ObjectGetOwnPropertySymbols(obj)) {
-    ObjectDefineProperty(proto, key, {
-      __proto__: null,
-      writable: true,
-      enumerable: false,
-      configurable: true,
-      value: obj[key],
-    });
-  }
-}
-
 // for merge sort
 function merge(out, start, mid, end, lBuffer, rBuffer) {
   const sizeLeft = mid - start;
@@ -1160,102 +1236,6 @@ function merge(out, start, mid, end, lBuffer, rBuffer) {
     out[o++] = rBuffer[r++];
 }
 
-// https://heycam.github.io/webidl/#dfn-default-iterator-object
-function createSearchParamsIterator(target, kind) {
-  const iterator = { __proto__: URLSearchParamsIteratorPrototype };
-  iterator[context] = {
-    target,
-    kind,
-    index: 0,
-  };
-  return iterator;
-}
-
-// https://heycam.github.io/webidl/#dfn-iterator-prototype-object
-const URLSearchParamsIteratorPrototype = { __proto__: IteratorPrototype };
-
-defineIDLClass(URLSearchParamsIteratorPrototype, 'URLSearchParams Iterator', {
-  next() {
-    if (!this ||
-        ObjectGetPrototypeOf(this) !== URLSearchParamsIteratorPrototype) {
-      throw new ERR_INVALID_THIS('URLSearchParamsIterator');
-    }
-
-    const {
-      target,
-      kind,
-      index,
-    } = this[context];
-    const values = target[searchParams];
-    const len = values.length;
-    if (index >= len) {
-      return {
-        value: undefined,
-        done: true,
-      };
-    }
-
-    const name = values[index];
-    const value = values[index + 1];
-    this[context].index = index + 2;
-
-    let result;
-    if (kind === 'key') {
-      result = name;
-    } else if (kind === 'value') {
-      result = value;
-    } else {
-      result = [name, value];
-    }
-
-    return {
-      value: result,
-      done: false,
-    };
-  },
-  [inspect.custom](recurseTimes, ctx) {
-    if (this == null || this[context] == null || this[context].target == null)
-      throw new ERR_INVALID_THIS('URLSearchParamsIterator');
-
-    if (typeof recurseTimes === 'number' && recurseTimes < 0)
-      return ctx.stylize('[Object]', 'special');
-
-    const innerOpts = { ...ctx };
-    if (recurseTimes !== null) {
-      innerOpts.depth = recurseTimes - 1;
-    }
-    const {
-      target,
-      kind,
-      index,
-    } = this[context];
-    const output = ArrayPrototypeReduce(
-      ArrayPrototypeSlice(target[searchParams], index),
-      (prev, cur, i) => {
-        const key = i % 2 === 0;
-        if (kind === 'key' && key) {
-          ArrayPrototypePush(prev, cur);
-        } else if (kind === 'value' && !key) {
-          ArrayPrototypePush(prev, cur);
-        } else if (kind === 'key+value' && !key) {
-          ArrayPrototypePush(prev, [target[searchParams][index + i - 1], cur]);
-        }
-        return prev;
-      },
-      [],
-    );
-    const breakLn = StringPrototypeIncludes(inspect(output, innerOpts), '\n');
-    const outputStrs = ArrayPrototypeMap(output, (p) => inspect(p, innerOpts));
-    let outputStr;
-    if (breakLn) {
-      outputStr = `\n  ${ArrayPrototypeJoin(outputStrs, ',\n  ')}`;
-    } else {
-      outputStr = ` ${ArrayPrototypeJoin(outputStrs, ', ')}`;
-    }
-    return `${this[SymbolToStringTag]} {${outputStr} }`;
-  },
-});
-
 function domainToASCII(domain) {
   if (arguments.length < 1)
     throw new ERR_MISSING_ARGS('domain');