From 886652156e0e548a44fd41b293916bf37594eaa3 Mon Sep 17 00:00:00 2001
From: Kenta Moriuchi <moriken@kimamass.com>
Date: Tue, 14 Nov 2023 15:01:15 +0900
Subject: [PATCH] fix(ext/web): webstorage has trap for symbol (#21090)

---
 cli/tests/unit/webstorage_test.ts | 14 ++++++--
 ext/webstorage/01_webstorage.js   | 53 ++++++++++++++++---------------
 2 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/cli/tests/unit/webstorage_test.ts b/cli/tests/unit/webstorage_test.ts
index e6ca5bb8801b8f..9c71b632053015 100644
--- a/cli/tests/unit/webstorage_test.ts
+++ b/cli/tests/unit/webstorage_test.ts
@@ -1,7 +1,7 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 // deno-lint-ignore-file no-explicit-any
 
-import { assert, assertThrows } from "./test_util.ts";
+import { assert, assertEquals, assertThrows } from "./test_util.ts";
 
 Deno.test({ permissions: "none" }, function webStoragesReassignable() {
   // Can reassign to web storages
@@ -21,7 +21,7 @@ Deno.test(function webstorageSizeLimit() {
     Error,
     "Exceeded maximum storage size",
   );
-  assert(localStorage.getItem("k") === null);
+  assertEquals(localStorage.getItem("k"), null);
   assertThrows(
     () => {
       localStorage.setItem("k".repeat(15 * 1024 * 1024), "v");
@@ -40,3 +40,13 @@ Deno.test(function webstorageSizeLimit() {
     "Exceeded maximum storage size",
   );
 });
+
+Deno.test(function webstorageProxy() {
+  localStorage.clear();
+  localStorage.foo = "foo";
+  assertEquals(localStorage.foo, "foo");
+  const symbol = Symbol("bar");
+  localStorage[symbol as any] = "bar";
+  assertEquals(localStorage[symbol as any], "bar");
+  assertEquals(symbol in localStorage, true);
+});
diff --git a/ext/webstorage/01_webstorage.js b/ext/webstorage/01_webstorage.js
index 58c68c832d959a..59abab1bf6f64e 100644
--- a/ext/webstorage/01_webstorage.js
+++ b/ext/webstorage/01_webstorage.js
@@ -7,12 +7,12 @@ const ops = core.ops;
 import * as webidl from "ext:deno_webidl/00_webidl.js";
 const primordials = globalThis.__bootstrap.primordials;
 const {
-  SafeArrayIterator,
   Symbol,
   SymbolFor,
-  ObjectDefineProperty,
   ObjectFromEntries,
   ObjectEntries,
+  ReflectDefineProperty,
+  ReflectDeleteProperty,
   ReflectGet,
   ReflectHas,
   Proxy,
@@ -83,51 +83,54 @@ function createStorage(persistent) {
 
   const proxy = new Proxy(storage, {
     deleteProperty(target, key) {
-      if (typeof key == "symbol") {
-        delete target[key];
-      } else {
-        target.removeItem(key);
+      if (typeof key === "symbol") {
+        return ReflectDeleteProperty(target, key);
       }
+      target.removeItem(key);
       return true;
     },
+
     defineProperty(target, key, descriptor) {
-      if (typeof key == "symbol") {
-        ObjectDefineProperty(target, key, descriptor);
-      } else {
-        target.setItem(key, descriptor.value);
+      if (typeof key === "symbol") {
+        return ReflectDefineProperty(target, key, descriptor);
       }
+      target.setItem(key, descriptor.value);
       return true;
     },
-    get(target, key) {
-      if (typeof key == "symbol") return target[key];
+
+    get(target, key, receiver) {
+      if (typeof key === "symbol") {
+        return target[key];
+      }
       if (ReflectHas(target, key)) {
-        return ReflectGet(...new SafeArrayIterator(arguments));
-      } else {
-        return target.getItem(key) ?? undefined;
+        return ReflectGet(target, key, receiver);
       }
+      return target.getItem(key) ?? undefined;
     },
+
     set(target, key, value) {
-      if (typeof key == "symbol") {
-        ObjectDefineProperty(target, key, {
+      if (typeof key === "symbol") {
+        return ReflectDefineProperty(target, key, {
           value,
           configurable: true,
         });
-      } else {
-        target.setItem(key, value);
       }
+      target.setItem(key, value);
       return true;
     },
-    has(target, p) {
-      return p === SymbolFor("Deno.customInspect") ||
-        (typeof target.getItem(p)) === "string";
+
+    has(target, key) {
+      if (ReflectHas(target, key)) {
+        return true;
+      }
+      return typeof key === "string" && typeof target.getItem(key) === "string";
     },
+
     ownKeys() {
       return ops.op_webstorage_iterate_keys(persistent);
     },
+
     getOwnPropertyDescriptor(target, key) {
-      if (arguments.length === 1) {
-        return undefined;
-      }
       if (ReflectHas(target, key)) {
         return undefined;
       }