src: distinguish HTML transferable and cloneable

The HTML structured serialize algorithm treats transferable and serializable as two different bits. A web platform interface can be both transferable and serializable. Splits BaseObject::TransferMode to be able to compose the two bits and distinguishes the transferable and cloneable. PR-URL: #47956 Refs: v8/v8@cf13b9b Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
nodejs · Jul 7, 2023 · 38dee8a · 38dee8a
1 parent 3205b19
commit 38dee8a
Show file tree

Hide file tree

Showing 37 changed files with 547 additions and 320 deletions.
diff --git a/common.gypi b/common.gypi
@@ -36,7 +36,7 @@
 
     # Reset this number to 0 on major V8 upgrades.
     # Increment by one for each non-official patch applied to deps/v8.
-    'v8_embedder_string': '-node.10',
+    'v8_embedder_string': '-node.11',
 
     ##### V8 defaults for Node.js #####
 

diff --git a/deps/v8/include/v8-value-serializer.h b/deps/v8/include/v8-value-serializer.h
@@ -75,6 +75,20 @@ class V8_EXPORT ValueSerializer {
      */
     virtual void ThrowDataCloneError(Local<String> message) = 0;
 
+    /**
+     * The embedder overrides this method to enable custom host object filter
+     * with Delegate::IsHostObject.
+     *
+     * This method is called at most once per serializer.
+     */
+    virtual bool HasCustomHostObject(Isolate* isolate);
+
+    /**
+     * The embedder overrides this method to determine if an JS object is a
+     * host object and needs to be serialized by the host.
+     */
+    virtual Maybe<bool> IsHostObject(Isolate* isolate, Local<Object> object);
+
     /**
      * The embedder overrides this method to write some kind of host object, if
      * possible. If not, a suitable exception should be thrown and

diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc
@@ -3569,6 +3569,19 @@ Maybe<bool> ValueSerializer::Delegate::WriteHostObject(Isolate* v8_isolate,
   return Nothing<bool>();
 }
 
+bool ValueSerializer::Delegate::HasCustomHostObject(Isolate* v8_isolate) {
+  return false;
+}
+
+Maybe<bool> ValueSerializer::Delegate::IsHostObject(Isolate* v8_isolate,
+                                                    Local<Object> object) {
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
+  i::Handle<i::JSObject> js_object =
+      i::Handle<i::JSObject>::cast(Utils::OpenHandle(*object));
+  return Just<bool>(
+      i::JSObject::GetEmbedderFieldCount(js_object->map(i_isolate)));
+}
+
 Maybe<uint32_t> ValueSerializer::Delegate::GetSharedArrayBufferId(
     Isolate* v8_isolate, Local<SharedArrayBuffer> shared_array_buffer) {
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(v8_isolate);

diff --git a/deps/v8/src/objects/value-serializer.cc b/deps/v8/src/objects/value-serializer.cc
@@ -268,7 +268,12 @@ ValueSerializer::ValueSerializer(Isolate* isolate,
       zone_(isolate->allocator(), ZONE_NAME),
       id_map_(isolate->heap(), ZoneAllocationPolicy(&zone_)),
       array_buffer_transfer_map_(isolate->heap(),
-                                 ZoneAllocationPolicy(&zone_)) {}
+                                 ZoneAllocationPolicy(&zone_)) {
+  if (delegate_) {
+    v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate_);
+    has_custom_host_objects_ = delegate_->HasCustomHostObject(v8_isolate);
+  }
+}
 
 ValueSerializer::~ValueSerializer() {
   if (buffer_) {
@@ -582,7 +587,11 @@ Maybe<bool> ValueSerializer::WriteJSReceiver(Handle<JSReceiver> receiver) {
     case JS_TYPED_ARRAY_PROTOTYPE_TYPE:
     case JS_API_OBJECT_TYPE: {
       Handle<JSObject> js_object = Handle<JSObject>::cast(receiver);
-      if (JSObject::GetEmbedderFieldCount(js_object->map(isolate_))) {
+      Maybe<bool> is_host_object = IsHostObject(js_object);
+      if (is_host_object.IsNothing()) {
+        return is_host_object;
+      }
+      if (is_host_object.FromJust()) {
         return WriteHostObject(js_object);
       } else {
         return WriteJSObject(js_object);
@@ -1190,6 +1199,23 @@ Maybe<uint32_t> ValueSerializer::WriteJSObjectPropertiesSlow(
   return Just(properties_written);
 }
 
+Maybe<bool> ValueSerializer::IsHostObject(Handle<JSObject> js_object) {
+  if (!has_custom_host_objects_) {
+    return Just<bool>(
+        JSObject::GetEmbedderFieldCount(js_object->map(isolate_)));
+  }
+  DCHECK_NOT_NULL(delegate_);
+
+  v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate_);
+  Maybe<bool> result =
+      delegate_->IsHostObject(v8_isolate, Utils::ToLocal(js_object));
+  RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate_, Nothing<bool>());
+  DCHECK(!result.IsNothing());
+
+  if (V8_UNLIKELY(out_of_memory_)) return ThrowIfOutOfMemory();
+  return result;
+}
+
 Maybe<bool> ValueSerializer::ThrowIfOutOfMemory() {
   if (out_of_memory_) {
     return ThrowDataCloneError(MessageTemplate::kDataCloneErrorOutOfMemory);

diff --git a/deps/v8/src/objects/value-serializer.h b/deps/v8/src/objects/value-serializer.h
@@ -155,6 +155,8 @@ class ValueSerializer {
   Maybe<uint32_t> WriteJSObjectPropertiesSlow(
       Handle<JSObject> object, Handle<FixedArray> keys) V8_WARN_UNUSED_RESULT;
 
+  Maybe<bool> IsHostObject(Handle<JSObject> object);
+
   /*
    * Asks the delegate to handle an error that occurred during data cloning, by
    * throwing an exception appropriate for the host.
@@ -172,6 +174,7 @@ class ValueSerializer {
   uint8_t* buffer_ = nullptr;
   size_t buffer_size_ = 0;
   size_t buffer_capacity_ = 0;
+  bool has_custom_host_objects_ = false;
   bool treat_array_buffer_views_as_host_objects_ = false;
   bool out_of_memory_ = false;
   Zone zone_;

diff --git a/deps/v8/test/unittests/objects/value-serializer-unittest.cc b/deps/v8/test/unittests/objects/value-serializer-unittest.cc
@@ -2808,7 +2808,18 @@ TEST_F(ValueSerializerTest, UnsupportedHostObject) {
 
 class ValueSerializerTestWithHostObject : public ValueSerializerTest {
  protected:
-  ValueSerializerTestWithHostObject() : serializer_delegate_(this) {}
+  ValueSerializerTestWithHostObject() : serializer_delegate_(this) {
+    ON_CALL(serializer_delegate_, HasCustomHostObject)
+        .WillByDefault([this](Isolate* isolate) {
+          return serializer_delegate_
+              .ValueSerializer::Delegate::HasCustomHostObject(isolate);
+        });
+    ON_CALL(serializer_delegate_, IsHostObject)
+        .WillByDefault([this](Isolate* isolate, Local<Object> object) {
+          return serializer_delegate_.ValueSerializer::Delegate::IsHostObject(
+              isolate, object);
+        });
+  }
 
   static const uint8_t kExampleHostObjectTag;
 
@@ -2832,6 +2843,9 @@ class ValueSerializerTestWithHostObject : public ValueSerializerTest {
    public:
     explicit SerializerDelegate(ValueSerializerTestWithHostObject* test)
         : test_(test) {}
+    MOCK_METHOD(bool, HasCustomHostObject, (Isolate*), (override));
+    MOCK_METHOD(Maybe<bool>, IsHostObject, (Isolate*, Local<Object> object),
+                (override));
     MOCK_METHOD(Maybe<bool>, WriteHostObject, (Isolate*, Local<Object> object),
                 (override));
     void ThrowDataCloneError(Local<String> message) override {
@@ -3049,6 +3063,43 @@ TEST_F(ValueSerializerTestWithHostObject, DecodeSimpleHostObject) {
       });
 }
 
+TEST_F(ValueSerializerTestWithHostObject,
+       RoundTripHostJSObjectWithoutCustomHostObject) {
+  EXPECT_CALL(serializer_delegate_, HasCustomHostObject(isolate()))
+      .WillOnce(Invoke([](Isolate* isolate) { return false; }));
+  RoundTripTest("({ a: { my_host_object: true }, get b() { return this.a; }})");
+}
+
+TEST_F(ValueSerializerTestWithHostObject, RoundTripHostJSObject) {
+  EXPECT_CALL(serializer_delegate_, HasCustomHostObject(isolate()))
+      .WillOnce(Invoke([](Isolate* isolate) { return true; }));
+  EXPECT_CALL(serializer_delegate_, IsHostObject(isolate(), _))
+      .WillRepeatedly(Invoke([this](Isolate* isolate, Local<Object> object) {
+        EXPECT_TRUE(object->IsObject());
+        Local<Context> context = isolate->GetCurrentContext();
+        return object->Has(context, StringFromUtf8("my_host_object"));
+      }));
+  EXPECT_CALL(serializer_delegate_, WriteHostObject(isolate(), _))
+      .WillOnce(Invoke([this](Isolate*, Local<Object> object) {
+        EXPECT_TRUE(object->IsObject());
+        WriteExampleHostObjectTag();
+        return Just(true);
+      }));
+  EXPECT_CALL(deserializer_delegate_, ReadHostObject(isolate()))
+      .WillOnce(Invoke([this](Isolate* isolate) {
+        EXPECT_TRUE(ReadExampleHostObjectTag());
+        Local<Context> context = isolate->GetCurrentContext();
+        Local<Object> obj = Object::New(isolate);
+        obj->Set(context, StringFromUtf8("my_host_object"), v8::True(isolate))
+            .Check();
+        return obj;
+      }));
+  RoundTripTest("({ a: { my_host_object: true }, get b() { return this.a; }})");
+  ExpectScriptTrue("!('my_host_object' in result)");
+  ExpectScriptTrue("result.a.my_host_object");
+  ExpectScriptTrue("result.a === result.b");
+}
+
 class ValueSerializerTestWithHostArrayBufferView
     : public ValueSerializerTestWithHostObject {
  protected:

diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js
@@ -58,15 +58,15 @@ const {
 const assert = require('internal/assert');
 
 const {
-  messaging_deserialize_symbol: kDeserialize,
-  messaging_transfer_symbol: kTransfer,
-  messaging_transfer_list_symbol: kTransferList,
-} = internalBinding('symbols');
+  kDeserialize,
+  kTransfer,
+  kTransferList,
+} = require('internal/worker/js_transferable');
 
 let _MessageChannel;
-let makeTransferable;
+let markTransferMode;
 
-// Loading the MessageChannel and makeTransferable have to be done lazily
+// Loading the MessageChannel and markTransferable have to be done lazily
 // because otherwise we'll end up with a require cycle that ends up with
 // an incomplete initialization of abort_controller.
 
@@ -75,10 +75,10 @@ function lazyMessageChannel() {
   return new _MessageChannel();
 }
 
-function lazyMakeTransferable(obj) {
-  makeTransferable ??=
-    require('internal/worker/js_transferable').makeTransferable;
-  return makeTransferable(obj);
+function lazyMarkTransferMode(obj, cloneable, transferable) {
+  markTransferMode ??=
+    require('internal/worker/js_transferable').markTransferMode;
+  markTransferMode(obj, cloneable, transferable);
 }
 
 const clearTimeoutRegistry = new SafeFinalizationRegistry(clearTimeout);
@@ -355,7 +355,10 @@ function createAbortSignal(init = kEmptyObject) {
   signal[kAborted] = aborted;
   signal[kReason] = reason;
   signal[kComposite] = composite;
-  return transferable ? lazyMakeTransferable(signal) : signal;
+  if (transferable) {
+    lazyMarkTransferMode(signal, false, true);
+  }
+  return signal;
 }
 
 function abortSignal(signal, reason) {
@@ -411,7 +414,8 @@ class AbortController {
 function transferableAbortSignal(signal) {
   if (signal?.[kAborted] === undefined)
     throw new ERR_INVALID_ARG_TYPE('signal', 'AbortSignal', signal);
-  return lazyMakeTransferable(signal);
+  lazyMarkTransferMode(signal, false, true);
+  return signal;
 }
 
 /**

diff --git a/lib/internal/blob.js b/lib/internal/blob.js
@@ -32,7 +32,7 @@ const {
 const { URL } = require('internal/url');
 
 const {
-  makeTransferable,
+  markTransferMode,
   kClone,
   kDeserialize,
 } = require('internal/worker/js_transferable');
@@ -136,6 +136,8 @@ class Blob {
    * @constructs {Blob}
    */
   constructor(sources = [], options) {
+    markTransferMode(this, true, false);
+
     if (sources === null ||
         typeof sources[SymbolIterator] !== 'function' ||
         typeof sources === 'string') {
@@ -167,9 +169,6 @@ class Blob {
     type = `${type}`;
     this[kType] = RegExpPrototypeExec(disallowedTypeCharacters, type) !== null ?
       '' : StringPrototypeToLowerCase(type);
-
-    // eslint-disable-next-line no-constructor-return
-    return makeTransferable(this);
   }
 
   [kInspect](depth, options) {
@@ -385,16 +384,19 @@ class Blob {
 }
 
 function ClonedBlob() {
-  return makeTransferable(ReflectConstruct(function() {}, [], Blob));
+  return ReflectConstruct(function() {
+    markTransferMode(this, true, false);
+  }, [], Blob);
 }
 ClonedBlob.prototype[kDeserialize] = () => {};
 
 function createBlob(handle, length, type = '') {
-  return makeTransferable(ReflectConstruct(function() {
+  return ReflectConstruct(function() {
+    markTransferMode(this, true, false);
     this[kHandle] = handle;
     this[kType] = type;
     this[kLength] = length;
-  }, [], Blob));
+  }, [], Blob);
 }
 
 ObjectDefineProperty(Blob.prototype, SymbolToStringTag, {

diff --git a/lib/internal/blocklist.js b/lib/internal/blocklist.js
@@ -20,7 +20,7 @@ const {
 } = require('internal/socketaddress');
 
 const {
-  JSTransferable,
+  markTransferMode,
   kClone,
   kDeserialize,
 } = require('internal/worker/js_transferable');
@@ -36,9 +36,9 @@ const {
 
 const { validateInt32, validateString } = require('internal/validators');
 
-class BlockList extends JSTransferable {
+class BlockList {
   constructor() {
-    super();
+    markTransferMode(this, true, false);
     this[kHandle] = new BlockListHandle();
     this[kHandle][owner_symbol] = this;
   }
@@ -148,9 +148,9 @@ class BlockList extends JSTransferable {
   }
 }
 
-class InternalBlockList extends JSTransferable {
+class InternalBlockList {
   constructor(handle) {
-    super();
+    markTransferMode(this, true, false);
     this[kHandle] = handle;
     if (handle !== undefined)
       handle[owner_symbol] = this;

diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js
@@ -57,7 +57,7 @@ const {
 } = require('internal/util/types');
 
 const {
-  makeTransferable,
+  markTransferMode,
   kClone,
   kDeserialize,
 } = require('internal/worker/js_transferable');
@@ -706,24 +706,22 @@ ObjectDefineProperties(CryptoKey.prototype, {
 // All internal code must use new InternalCryptoKey to create
 // CryptoKey instances. The CryptoKey class is exposed to end
 // user code but is not permitted to be constructed directly.
-// Using makeTransferable also allows the CryptoKey to be
+// Using markTransferMode also allows the CryptoKey to be
 // cloned to Workers.
 class InternalCryptoKey {
   constructor(
     keyObject,
     algorithm,
     keyUsages,
     extractable) {
+    markTransferMode(this, true, false);
     // Using symbol properties here currently instead of private
     // properties because (for now) the performance penalty of
     // private fields is still too high.
     this[kKeyObject] = keyObject;
     this[kAlgorithm] = algorithm;
     this[kExtractable] = extractable;
     this[kKeyUsages] = keyUsages;
-
-    // eslint-disable-next-line no-constructor-return
-    return makeTransferable(this);
   }
 
   [kClone]() {