From 0beb5ab93d03dc7add66667c0c087110fbddb2b3 Mon Sep 17 00:00:00 2001
From: Antoine du Hamel <duhamelantoine1995@gmail.com>
Date: Mon, 24 Jul 2023 09:47:47 +0200
Subject: [PATCH] url: ensure getter access do not mutate observable symbols

PR-URL: https://github.com/nodejs/node/pull/48897
Backport-PR-URL: https://github.com/nodejs/node/pull/48891
Refs: https://github.com/nodejs/node/pull/48891
Refs: https://github.com/nodejs/node/issues/48886
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
---
 lib/internal/url.js                           | 30 +++++++++++++------
 .../test-whatwg-url-custom-searchparams.js    |  2 ++
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 51688b8403076a..beadc12cbaa913 100644
--- 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
index 0b2087ac246313..75fa1779bdeb45 100644
--- 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(), '');