url: ensure getter access do not mutate observable symbols

PR-URL: #48897 Refs: #48891 Refs: #48886 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
nodejs · Jul 24, 2023 · eb29d9a · eb29d9a
1 parent a05f72f
commit eb29d9a
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 9 deletions.
diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -22,6 +22,7 @@ const {
   ReflectOwnKeys,
   RegExpPrototypeSymbolReplace,
   SafeMap,
+  SafeWeakMap,
   StringPrototypeCharAt,
   StringPrototypeCharCodeAt,
   StringPrototypeCodePointAt,
@@ -99,6 +100,12 @@ const FORWARD_SLASH = /\//g;
 const context = Symbol('context');
 const searchParams = Symbol('query');
 
+/**
+ * We cannot use private fields without a breaking change, so a WeakMap is the alternative.
+ * @type {WeakMap<URL,URLSearchParams>}
+ */
+const internalSearchParams = new SafeWeakMap();
+
 const updateActions = {
   kProtocol: 0,
   kHost: 1,
@@ -179,7 +186,7 @@ class URLContext {
 }
 
 function isURLSearchParams(self) {
-  return self && self[searchParams] && !self[searchParams][searchParams];
+  return self?.[searchParams];
 }
 
 class URLSearchParams {
@@ -672,11 +679,12 @@ class URL {
     ctx.hash_start = hash_start;
     ctx.scheme_type = scheme_type;
 
-    if (this[searchParams]) {
+    const alreadyInstantiatedSearchParams = internalSearchParams.get(this);
+    if (alreadyInstantiatedSearchParams) {
       if (ctx.hasSearch) {
-        this[searchParams][searchParams] = parseParams(this.search);
+        alreadyInstantiatedSearchParams[searchParams] = parseParams(this.search);
       } else {
-        this[searchParams][searchParams] = [];
+        alreadyInstantiatedSearchParams[searchParams] = [];
       }
     }
   }
@@ -898,11 +906,15 @@ class URL {
   get searchParams() {
     if (!isURL(this))
       throw new ERR_INVALID_THIS('URL');
-    if (this[searchParams] == null) {
-      this[searchParams] = new URLSearchParams(this.search);
-      this[searchParams][context] = this;
-    }
-    return this[searchParams];
+
+    const cachedValue = internalSearchParams.get(this);
+    if (cachedValue != null)
+      return cachedValue;
+
+    const value = new URLSearchParams(this.search);
+    value[context] = this;
+    internalSearchParams.set(this, value);
+    return value;
   }
 
   get hash() {

diff --git a/test/parallel/test-whatwg-url-custom-searchparams.js b/test/parallel/test-whatwg-url-custom-searchparams.js
@@ -16,7 +16,9 @@ const normalizedValues = ['a', '1', 'true', 'undefined', 'null', '\uFFFD',
                           '[object Object]'];
 
 const m = new URL('http://example.org');
+const ownSymbolsBeforeGetterAccess = Object.getOwnPropertySymbols(m);
 const sp = m.searchParams;
+assert.deepStrictEqual(Object.getOwnPropertySymbols(m), ownSymbolsBeforeGetterAccess);
 
 assert(sp);
 assert.strictEqual(sp.toString(), '');