Merge pull request #5195 from cloudflare/jasnell/fixup-jsg-ser

Fixup handling of ab/sab in jsg serialization

Author: jasnell

src/workerd/api/tests/global-scope-test.js

@@ -777,3 +777,35 @@ export const reuseCtx = {
     ctx.reused = true;
   },
 };
+
+export const structuredCloneError = {
+  test() {
+    // If it doesn't crash, we're good.
+    var globalProp = {
+      get p1() {
+        return this;
+      },
+      get trigger() {
+        function createSab() {
+          var sab = new SharedArrayBuffer(4096);
+          return sab;
+        }
+        var prop = {};
+        Object.defineProperty(prop, 'constructor', {
+          writable: true,
+          value: createSab,
+        });
+        return prop.constructor();
+      },
+      get p1() {
+        var gTCtor = globalThis.Intl.constructor;
+        var returnVal;
+        try {
+          returnVal = gTCtor.entries(this);
+        } catch (e) {}
+        return returnVal;
+      },
+    };
+    globalThis.structuredClone(globalProp);
+  },
+};

src/workerd/jsg/ser.c++

@@ -159,15 +159,16 @@ Serializer::Serializer(Lock& js, Options options)
 
 v8::Maybe<uint32_t> Serializer::GetSharedArrayBufferId(
     v8::Isolate* isolate, v8::Local<v8::SharedArrayBuffer> sab) {
+  auto& js = jsg::Lock::from(isolate);
   uint32_t n;
   auto value = JsValue(sab);
   for (n = 0; n < sharedArrayBuffers.size(); n++) {
     // If the SharedArrayBuffer has already been added, return the existing ID for it.
-    if (sharedArrayBuffers[n] == value) {
+    if (sharedArrayBuffers[n].getHandle(js) == value) {
       return v8::Just(n);
     }
   }
-  sharedArrayBuffers.add(value);
+  sharedArrayBuffers.add(jsg::JsRef(js, value));
   sharedBackingStores.add(sab->GetBackingStore());
   return v8::Just(n);
 }
@@ -352,11 +353,11 @@ void Serializer::transfer(Lock& js, const JsValue& value) {
   uint32_t n;
   for (n = 0; n < arrayBuffers.size(); n++) {
     // If the ArrayBuffer has already been added, we do not want to try adding it again.
-    if (arrayBuffers[n] == value) {
+    if (arrayBuffers[n].getHandle(js) == value) {
       return;
     }
   }
-  arrayBuffers.add(value);
+  arrayBuffers.add(jsg::JsRef(js, value));
 
   backingStores.add(arrayBuffer->GetBackingStore());
   check(arrayBuffer->Detach(v8::Local<v8::Value>()));

src/workerd/jsg/ser.h

@@ -197,8 +197,8 @@ class Serializer final: v8::ValueSerializer::Delegate {
 
   kj::Maybe<ExternalHandler&> externalHandler;
 
-  kj::Vector<JsValue> sharedArrayBuffers;
-  kj::Vector<JsValue> arrayBuffers;
+  kj::Vector<jsg::JsRef<JsValue>> sharedArrayBuffers;
+  kj::Vector<jsg::JsRef<JsValue>> arrayBuffers;
   kj::Vector<std::shared_ptr<v8::BackingStore>> sharedBackingStores;
   kj::Vector<std::shared_ptr<v8::BackingStore>> backingStores;
   bool released = false;