From 38dee8a1c04237bd231a01410f42e9d172f4c162 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Sat, 8 Jul 2023 01:00:00 +0800 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/47956 Refs: https://github.com/v8/v8/commit/cf13b9b46572a9824d2d632abdd48c56161ace02 Reviewed-By: Matteo Collina Reviewed-By: Yagiz Nizipli Reviewed-By: Daeyeon Jeong --- common.gypi | 2 +- deps/v8/include/v8-value-serializer.h | 14 + deps/v8/src/api/api.cc | 13 + deps/v8/src/objects/value-serializer.cc | 30 +- deps/v8/src/objects/value-serializer.h | 3 + .../objects/value-serializer-unittest.cc | 53 ++- lib/internal/abort_controller.js | 28 +- lib/internal/blob.js | 16 +- lib/internal/blocklist.js | 10 +- lib/internal/crypto/keys.js | 8 +- lib/internal/crypto/x509.js | 10 +- lib/internal/event_target.js | 25 -- lib/internal/fs/promises.js | 7 +- lib/internal/histogram.js | 12 +- lib/internal/perf/event_loop_delay.js | 7 +- lib/internal/socketaddress.js | 12 +- lib/internal/test/transfer.js | 5 +- lib/internal/webstreams/readablestream.js | 13 +- lib/internal/webstreams/transfer.js | 14 +- lib/internal/webstreams/transformstream.js | 11 +- lib/internal/webstreams/writablestream.js | 11 +- lib/internal/worker/js_transferable.js | 80 ++-- src/base_object.cc | 27 ++ src/base_object.h | 27 +- src/crypto/crypto_x509.cc | 1 - src/env_properties.h | 3 + src/histogram.h | 4 +- src/node_blob.cc | 2 +- src/node_file.cc | 9 +- src/node_file.h | 2 +- src/node_messaging.cc | 343 ++++++++++-------- src/node_messaging.h | 25 +- src/node_sockaddr.h | 4 +- src/node_util.cc | 14 + ....js => test-messaging-marktransfermode.js} | 4 - .../test-whatwg-webstreams-transfer.js | 12 +- ...rt-jstransferable-nested-untransferable.js | 6 +- 37 files changed, 547 insertions(+), 320 deletions(-) rename test/parallel/{test-messaging-maketransferable.js => test-messaging-marktransfermode.js} (82%) diff --git a/common.gypi b/common.gypi index 4e645cf50671d4..65720283c54109 100644 --- 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 index 729730c608dbe4..0cb3e045bc46ec 100644 --- 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 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 IsHostObject(Isolate* isolate, Local 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 index e286ccd254497a..10e342d496a703 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -3569,6 +3569,19 @@ Maybe ValueSerializer::Delegate::WriteHostObject(Isolate* v8_isolate, return Nothing(); } +bool ValueSerializer::Delegate::HasCustomHostObject(Isolate* v8_isolate) { + return false; +} + +Maybe ValueSerializer::Delegate::IsHostObject(Isolate* v8_isolate, + Local object) { + i::Isolate* i_isolate = reinterpret_cast(v8_isolate); + i::Handle js_object = + i::Handle::cast(Utils::OpenHandle(*object)); + return Just( + i::JSObject::GetEmbedderFieldCount(js_object->map(i_isolate))); +} + Maybe ValueSerializer::Delegate::GetSharedArrayBufferId( Isolate* v8_isolate, Local shared_array_buffer) { i::Isolate* i_isolate = reinterpret_cast(v8_isolate); diff --git a/deps/v8/src/objects/value-serializer.cc b/deps/v8/src/objects/value-serializer.cc index 63c32f4ba18284..2efca82aaaa00d 100644 --- 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(isolate_); + has_custom_host_objects_ = delegate_->HasCustomHostObject(v8_isolate); + } +} ValueSerializer::~ValueSerializer() { if (buffer_) { @@ -582,7 +587,11 @@ Maybe ValueSerializer::WriteJSReceiver(Handle receiver) { case JS_TYPED_ARRAY_PROTOTYPE_TYPE: case JS_API_OBJECT_TYPE: { Handle js_object = Handle::cast(receiver); - if (JSObject::GetEmbedderFieldCount(js_object->map(isolate_))) { + Maybe 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 ValueSerializer::WriteJSObjectPropertiesSlow( return Just(properties_written); } +Maybe ValueSerializer::IsHostObject(Handle js_object) { + if (!has_custom_host_objects_) { + return Just( + JSObject::GetEmbedderFieldCount(js_object->map(isolate_))); + } + DCHECK_NOT_NULL(delegate_); + + v8::Isolate* v8_isolate = reinterpret_cast(isolate_); + Maybe result = + delegate_->IsHostObject(v8_isolate, Utils::ToLocal(js_object)); + RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate_, Nothing()); + DCHECK(!result.IsNothing()); + + if (V8_UNLIKELY(out_of_memory_)) return ThrowIfOutOfMemory(); + return result; +} + Maybe 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 index 13156b99dd259c..f5ccdcbf0a5f0c 100644 --- a/deps/v8/src/objects/value-serializer.h +++ b/deps/v8/src/objects/value-serializer.h @@ -155,6 +155,8 @@ class ValueSerializer { Maybe WriteJSObjectPropertiesSlow( Handle object, Handle keys) V8_WARN_UNUSED_RESULT; + Maybe IsHostObject(Handle 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 index a2f6461ab28f25..bf81b745dea261 100644 --- 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) { + 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, IsHostObject, (Isolate*, Local object), + (override)); MOCK_METHOD(Maybe, WriteHostObject, (Isolate*, Local object), (override)); void ThrowDataCloneError(Local 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) { + EXPECT_TRUE(object->IsObject()); + Local context = isolate->GetCurrentContext(); + return object->Has(context, StringFromUtf8("my_host_object")); + })); + EXPECT_CALL(serializer_delegate_, WriteHostObject(isolate(), _)) + .WillOnce(Invoke([this](Isolate*, Local 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 = isolate->GetCurrentContext(); + Local 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 index e31738b98288ca..3c5c2388b217bd 100644 --- 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 index 0d0e9906dbdb31..655023e07780f2 100644 --- 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 index c0bce00321fc8b..d4eb35c9a70cb8 100644 --- 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 index 4b06810a8729c4..1e49bd62473a1b 100644 --- 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,7 +706,7 @@ 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( @@ -714,6 +714,7 @@ class InternalCryptoKey { 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. @@ -721,9 +722,6 @@ class InternalCryptoKey { this[kAlgorithm] = algorithm; this[kExtractable] = extractable; this[kKeyUsages] = keyUsages; - - // eslint-disable-next-line no-constructor-return - return makeTransferable(this); } [kClone]() { diff --git a/lib/internal/crypto/x509.js b/lib/internal/crypto/x509.js index 0e9d4e87506329..30005390a4571e 100644 --- a/lib/internal/crypto/x509.js +++ b/lib/internal/crypto/x509.js @@ -48,7 +48,7 @@ const { } = require('internal/errors'); const { - JSTransferable, + markTransferMode, kClone, kDeserialize, } = require('internal/worker/js_transferable'); @@ -94,16 +94,16 @@ function getFlags(options = kEmptyObject) { return flags; } -class InternalX509Certificate extends JSTransferable { +class InternalX509Certificate { [kInternalState] = new SafeMap(); constructor(handle) { - super(); + markTransferMode(this, true, false); this[kHandle] = handle; } } -class X509Certificate extends JSTransferable { +class X509Certificate { [kInternalState] = new SafeMap(); constructor(buffer) { @@ -115,7 +115,7 @@ class X509Certificate extends JSTransferable { ['string', 'Buffer', 'TypedArray', 'DataView'], buffer); } - super(); + markTransferMode(this, true, false); this[kHandle] = parseX509(buffer); } diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index 2da787d6388603..c6fbc81a27b4f2 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -10,11 +10,7 @@ const { ObjectDefineProperties, ObjectDefineProperty, ObjectGetOwnPropertyDescriptor, - ObjectGetOwnPropertyDescriptors, - ObjectSetPrototypeOf, - ObjectValues, ReflectApply, - SafeArrayIterator, SafeFinalizationRegistry, SafeMap, SafeWeakMap, @@ -1114,30 +1110,9 @@ function defineEventHandler(emitter, name, event = name) { }); } -const EventEmitterMixin = (Superclass) => { - class MixedEventEmitter extends Superclass { - constructor(...args) { - args = new SafeArrayIterator(args); - super(...args); - FunctionPrototypeCall(EventEmitter, this); - } - } - const protoProps = ObjectGetOwnPropertyDescriptors(EventEmitter.prototype); - delete protoProps.constructor; - const propertiesValues = ObjectValues(protoProps); - for (let i = 0; i < propertiesValues.length; i++) { - // We want to use null-prototype objects to not rely on globally mutable - // %Object.prototype%. - ObjectSetPrototypeOf(propertiesValues[i], null); - } - ObjectDefineProperties(MixedEventEmitter.prototype, protoProps); - return MixedEventEmitter; -}; - module.exports = { Event, CustomEvent, - EventEmitterMixin, EventTarget, NodeEventTarget, defineEventHandler, diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 1a5618b6955bb8..74af3c9782b7a9 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -90,7 +90,7 @@ const { lazyDOMException, promisify, } = require('internal/util'); -const { EventEmitterMixin } = require('internal/event_target'); +const EventEmitter = require('events'); const { StringDecoder } = require('string_decoder'); const { kFSWatchStart, watch } = require('internal/fs/watchers'); const nonNativeWatcher = require('internal/fs/recursive_watch'); @@ -110,7 +110,7 @@ const kLocked = Symbol('kLocked'); const { kUsePromises } = binding; const { Interface } = require('internal/readline/interface'); const { - JSTransferable, kDeserialize, kTransfer, kTransferList, + kDeserialize, kTransfer, kTransferList, markTransferMode, } = require('internal/worker/js_transferable'); const getDirectoryEntriesPromise = promisify(getDirents); @@ -130,12 +130,13 @@ function lazyFsStreams() { return fsStreams ??= require('internal/fs/streams'); } -class FileHandle extends EventEmitterMixin(JSTransferable) { +class FileHandle extends EventEmitter { /** * @param {InternalFSBinding.FileHandle | undefined} filehandle */ constructor(filehandle) { super(); + markTransferMode(this, false, true); this[kHandle] = filehandle; this[kFd] = filehandle ? filehandle.fd : -1; diff --git a/lib/internal/histogram.js b/lib/internal/histogram.js index 37bde8195e4d34..079fbce50d54e3 100644 --- a/lib/internal/histogram.js +++ b/lib/internal/histogram.js @@ -45,7 +45,7 @@ const kRecordable = Symbol('kRecordable'); const { kClone, kDeserialize, - makeTransferable, + markTransferMode, } = require('internal/worker/js_transferable'); function isHistogram(object) { @@ -319,21 +319,23 @@ class RecordableHistogram extends Histogram { } function internalHistogram(handle) { - return makeTransferable(ReflectConstruct( + return ReflectConstruct( function() { + markTransferMode(this, true, false); this[kHandle] = handle; this[kMap] = new SafeMap(); - }, [], Histogram)); + }, [], Histogram); } internalHistogram.prototype[kDeserialize] = () => {}; function internalRecordableHistogram(handle) { - return makeTransferable(ReflectConstruct( + return ReflectConstruct( function() { + markTransferMode(this, true, false); this[kHandle] = handle; this[kMap] = new SafeMap(); this[kRecordable] = true; - }, [], RecordableHistogram)); + }, [], RecordableHistogram); } internalRecordableHistogram.prototype[kDeserialize] = () => {}; diff --git a/lib/internal/perf/event_loop_delay.js b/lib/internal/perf/event_loop_delay.js index 1c1d657a9a0e61..8281ea105f4083 100644 --- a/lib/internal/perf/event_loop_delay.js +++ b/lib/internal/perf/event_loop_delay.js @@ -32,7 +32,7 @@ const { } = require('internal/util'); const { - makeTransferable, + markTransferMode, } = require('internal/worker/js_transferable'); const kEnabled = Symbol('kEnabled'); @@ -79,12 +79,13 @@ function monitorEventLoopDelay(options = kEmptyObject) { const { resolution = 10 } = options; validateInteger(resolution, 'options.resolution', 1); - return makeTransferable(ReflectConstruct( + return ReflectConstruct( function() { + markTransferMode(this, true, false); this[kEnabled] = false; this[kHandle] = createELDHistogram(resolution); this[kMap] = new SafeMap(); - }, [], ELDHistogram)); + }, [], ELDHistogram); } module.exports = monitorEventLoopDelay; diff --git a/lib/internal/socketaddress.js b/lib/internal/socketaddress.js index 2fd524bc7237f0..e432c8a7d7593a 100644 --- a/lib/internal/socketaddress.js +++ b/lib/internal/socketaddress.js @@ -32,7 +32,7 @@ const { const { inspect } = require('internal/util/inspect'); const { - JSTransferable, + markTransferMode, kClone, kDeserialize, } = require('internal/worker/js_transferable'); @@ -40,13 +40,14 @@ const { const kHandle = Symbol('kHandle'); const kDetail = Symbol('kDetail'); -class SocketAddress extends JSTransferable { +class SocketAddress { static isSocketAddress(value) { return value?.[kHandle] !== undefined; } constructor(options = kEmptyObject) { - super(); + markTransferMode(this, true, false); + validateObject(options, 'options'); let { family = 'ipv4' } = options; const { @@ -139,9 +140,10 @@ class SocketAddress extends JSTransferable { } } -class InternalSocketAddress extends JSTransferable { +class InternalSocketAddress { constructor(handle) { - super(); + markTransferMode(this, true, false); + this[kHandle] = handle; this[kDetail] = this[kHandle]?.detail({ address: undefined, diff --git a/lib/internal/test/transfer.js b/lib/internal/test/transfer.js index 635f3033e76642..72361fb22199ef 100644 --- a/lib/internal/test/transfer.js +++ b/lib/internal/test/transfer.js @@ -1,7 +1,7 @@ 'use strict'; const { - makeTransferable, + markTransferMode, kClone, kDeserialize, } = require('internal/worker/js_transferable'); @@ -23,8 +23,7 @@ class E { class F extends E { constructor(b) { super(b); - /* eslint-disable-next-line no-constructor-return */ - return makeTransferable(this); + markTransferMode(this, true, false); } [kClone]() { diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index 1f96a709959301..f6776c44e23efc 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -69,7 +69,7 @@ const { kDeserialize, kTransfer, kTransferList, - makeTransferable, + markTransferMode, } = require('internal/worker/js_transferable'); const { @@ -242,6 +242,7 @@ class ReadableStream { * @param {QueuingStrategy} [strategy] */ constructor(source = {}, strategy = kEmptyObject) { + markTransferMode(this, false, true); if (source === null) throw new ERR_INVALID_ARG_VALUE('source', 'Object', source); this[kState] = { @@ -287,9 +288,6 @@ class ReadableStream { extractHighWaterMark(highWaterMark, 1), extractSizeAlgorithm(size)); } - - // eslint-disable-next-line no-constructor-return - return makeTransferable(this); } get [kIsDisturbed]() { @@ -645,8 +643,9 @@ ObjectDefineProperties(ReadableStream.prototype, { }); function TransferredReadableStream() { - return makeTransferable(ReflectConstruct( + return ReflectConstruct( function() { + markTransferMode(this, false, true); this[kType] = 'ReadableStream'; this[kState] = { disturbed: false, @@ -661,7 +660,7 @@ function TransferredReadableStream() { }; this[kIsClosedPromise] = createDeferredPromise(); }, - [], ReadableStream)); + [], ReadableStream); } TransferredReadableStream.prototype[kDeserialize] = () => {}; @@ -1209,6 +1208,7 @@ function createReadableByteStreamController() { function createTeeReadableStream(start, pull, cancel) { return ReflectConstruct( function() { + markTransferMode(this, false, true); this[kType] = 'ReadableStream'; this[kState] = { disturbed: false, @@ -1231,7 +1231,6 @@ function createTeeReadableStream(start, pull, cancel) { }), 1, () => 1); - return makeTransferable(this); }, [], ReadableStream, ); } diff --git a/lib/internal/webstreams/transfer.js b/lib/internal/webstreams/transfer.js index 367daf4bc17339..136b0d81a99464 100644 --- a/lib/internal/webstreams/transfer.js +++ b/lib/internal/webstreams/transfer.js @@ -35,7 +35,7 @@ const { const assert = require('internal/assert'); const { - makeTransferable, + markTransferMode, kClone, kDeserialize, } = require('internal/worker/js_transferable'); @@ -49,13 +49,12 @@ const { class CloneableDOMException extends DOMException { constructor(message, name) { super(message, name); + markTransferMode(this, true, false); this[kDeserialize]({ message: this.message, name: this.name, code: this.code, }); - // eslint-disable-next-line no-constructor-return - return makeTransferable(this); } [kClone]() { @@ -95,11 +94,10 @@ class CloneableDOMException extends DOMException { } function InternalCloneableDOMException() { - return makeTransferable( - ReflectConstruct( - CloneableDOMException, - [], - DOMException)); + return ReflectConstruct( + CloneableDOMException, + [], + DOMException); } InternalCloneableDOMException[kDeserialize] = () => {}; diff --git a/lib/internal/webstreams/transformstream.js b/lib/internal/webstreams/transformstream.js index 3a7fbebc042a22..c5b2aa90ffae5f 100644 --- a/lib/internal/webstreams/transformstream.js +++ b/lib/internal/webstreams/transformstream.js @@ -34,7 +34,7 @@ const { kDeserialize, kTransfer, kTransferList, - makeTransferable, + markTransferMode, } = require('internal/worker/js_transferable'); const { @@ -120,6 +120,7 @@ class TransformStream { transformer = null, writableStrategy = kEmptyObject, readableStrategy = kEmptyObject) { + markTransferMode(this, false, true); const readableType = transformer?.readableType; const writableType = transformer?.writableType; const start = transformer?.start; @@ -170,9 +171,6 @@ class TransformStream { } else { startPromise.resolve(); } - - // eslint-disable-next-line no-constructor-return - return makeTransferable(this); } /** @@ -247,8 +245,9 @@ ObjectDefineProperties(TransformStream.prototype, { }); function TransferredTransformStream() { - return makeTransferable(ReflectConstruct( + return ReflectConstruct( function() { + markTransferMode(this, false, true); this[kType] = 'TransformStream'; this[kState] = { readable: undefined, @@ -262,7 +261,7 @@ function TransferredTransformStream() { controller: undefined, }; }, - [], TransformStream)); + [], TransformStream); } TransferredTransformStream.prototype[kDeserialize] = () => {}; diff --git a/lib/internal/webstreams/writablestream.js b/lib/internal/webstreams/writablestream.js index b0eb5f20abb80e..2544c179f53952 100644 --- a/lib/internal/webstreams/writablestream.js +++ b/lib/internal/webstreams/writablestream.js @@ -44,7 +44,7 @@ const { kDeserialize, kTransfer, kTransferList, - makeTransferable, + markTransferMode, } = require('internal/worker/js_transferable'); const { @@ -153,6 +153,7 @@ class WritableStream { * @param {QueuingStrategy} [strategy] */ constructor(sink = null, strategy = kEmptyObject) { + markTransferMode(this, false, true); const type = sink?.type; if (type !== undefined) throw new ERR_INVALID_ARG_VALUE.RangeError('type', type); @@ -208,9 +209,6 @@ class WritableStream { sink, highWaterMark, size); - - // eslint-disable-next-line no-constructor-return - return makeTransferable(this); } /** @@ -328,8 +326,9 @@ ObjectDefineProperties(WritableStream.prototype, { }); function TransferredWritableStream() { - return makeTransferable(ReflectConstruct( + return ReflectConstruct( function() { + markTransferMode(this, false, true); this[kType] = 'WritableStream'; this[kState] = { close: createDeferredPromise(), @@ -373,7 +372,7 @@ function TransferredWritableStream() { this[kIsClosedPromise] = createDeferredPromise(); this[kControllerErrorFunction] = () => {}; }, - [], WritableStream)); + [], WritableStream); } TransferredWritableStream.prototype[kDeserialize] = () => {}; diff --git a/lib/internal/worker/js_transferable.js b/lib/internal/worker/js_transferable.js index 41ef278b33174d..083702149f29d9 100644 --- a/lib/internal/worker/js_transferable.js +++ b/lib/internal/worker/js_transferable.js @@ -1,12 +1,6 @@ 'use strict'; const { Error, - ObjectDefineProperties, - ObjectGetOwnPropertyDescriptors, - ObjectGetPrototypeOf, - ObjectSetPrototypeOf, - ObjectValues, - ReflectConstruct, StringPrototypeSplit, } = primordials; const { @@ -16,9 +10,18 @@ const { messaging_transfer_list_symbol, } = internalBinding('symbols'); const { - JSTransferable, setDeserializerCreateObjectFunction, } = internalBinding('messaging'); +const { + privateSymbols: { + transfer_mode_private_symbol, + }, + constants: { + kDisallowCloneAndTransfer, + kTransferable, + kCloneable, + }, +} = internalBinding('util'); function setup() { // Register the handler that will be used when deserializing JS-based objects @@ -39,26 +42,57 @@ function setup() { }); } -function makeTransferable(obj) { - // If the object is already transferable, skip all this. - if (obj instanceof JSTransferable) return obj; - const inst = ReflectConstruct(JSTransferable, [], obj.constructor); - const properties = ObjectGetOwnPropertyDescriptors(obj); - const propertiesValues = ObjectValues(properties); - for (let i = 0; i < propertiesValues.length; i++) { - // We want to use null-prototype objects to not rely on globally mutable - // %Object.prototype%. - ObjectSetPrototypeOf(propertiesValues[i], null); - } - ObjectDefineProperties(inst, properties); - ObjectSetPrototypeOf(inst, ObjectGetPrototypeOf(obj)); - return inst; +/** + * Mark an object as being transferable or customized cloneable in + * `.postMessage()`. + * This should only applied to host objects like Web API interfaces, Node.js' + * built-in objects. + * Objects marked as cloneable and transferable should implement the method + * `@@kClone` and `@@kTransfer` respectively. Method `@@kDeserialize` is + * required to deserialize the data to a new instance. + * + * Example implementation of a cloneable interface (assuming its located in + * `internal/my_interface.js`): + * + * ``` + * class MyInterface { + * constructor(...args) { + * markTransferMode(this, true); + * this.args = args; + * } + * [kDeserialize](data) { + * this.args = data.args; + * } + * [kClone]() { + * return { + * data: { args: this.args }, + * deserializeInfo: 'internal/my_interface:MyInterface', + * } + * } + * } + * + * module.exports = { + * MyInterface, + * }; + * ``` + * @param {object} obj Host objects that can be either cloned or transferred. + * @param {boolean} [cloneable] if the object can be cloned and `@@kClone` is + * implemented. + * @param {boolean} [transferable] if the object can be transferred and + * `@@kTransfer` is implemented. + */ +function markTransferMode(obj, cloneable = false, transferable = false) { + if ((typeof obj !== 'object' && typeof obj !== 'function') || obj === null) + return; // This object is a primitive and therefore already untransferable. + let mode = kDisallowCloneAndTransfer; + if (cloneable) mode |= kCloneable; + if (transferable) mode |= kTransferable; + obj[transfer_mode_private_symbol] = mode; } module.exports = { - makeTransferable, + markTransferMode, setup, - JSTransferable, kClone: messaging_clone_symbol, kDeserialize: messaging_deserialize_symbol, kTransfer: messaging_transfer_symbol, diff --git a/src/base_object.cc b/src/base_object.cc index ed0fafe74d43a9..9fb3efe03869ab 100644 --- a/src/base_object.cc +++ b/src/base_object.cc @@ -1,15 +1,20 @@ #include "base_object.h" #include "env-inl.h" +#include "node_messaging.h" #include "node_realm-inl.h" namespace node { +using v8::Context; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; using v8::HandleScope; +using v8::Just; using v8::Local; +using v8::Maybe; using v8::Object; using v8::Value; +using v8::ValueDeserializer; using v8::WeakCallbackInfo; using v8::WeakCallbackType; @@ -93,6 +98,28 @@ Local BaseObject::MakeLazilyInitializedJSTemplate( return t; } +BaseObject::TransferMode BaseObject::GetTransferMode() const { + return TransferMode::kDisallowCloneAndTransfer; +} + +std::unique_ptr BaseObject::TransferForMessaging() { + return {}; +} + +std::unique_ptr BaseObject::CloneForMessaging() const { + return {}; +} + +Maybe>> BaseObject::NestedTransferables() + const { + return Just(std::vector>{}); +} + +Maybe BaseObject::FinalizeTransferRead(Local context, + ValueDeserializer* deserializer) { + return Just(true); +} + BaseObject::PointerData* BaseObject::pointer_data() { if (!has_pointer_data()) { PointerData* metadata = new PointerData(); diff --git a/src/base_object.h b/src/base_object.h index cfd6af673cec97..de66a83e7fff76 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -135,11 +135,11 @@ class BaseObject : public MemoryRetainer { // method of MessagePorts (and, by extension, Workers). // GetTransferMode() returns a transfer mode that indicates how to deal with // the current object: - // - kUntransferable: - // No transfer is possible, either because this type of BaseObject does - // not know how to be transferred, or because it is not in a state in - // which it is possible to do so (e.g. because it has already been - // transferred). + // - kDisallowCloneAndTransfer: + // No transfer or clone is possible, either because this type of + // BaseObject does not know how to be transferred, or because it is not + // in a state in which it is possible to do so (e.g. because it has + // already been transferred). // - kTransferable: // This object can be transferred in a destructive fashion, i.e. will be // rendered unusable on the sending side of the channel in the process @@ -155,15 +155,20 @@ class BaseObject : public MemoryRetainer { // This object can be cloned without being modified. // CloneForMessaging() will be called to get a representation of the // object that is used for subsequent deserialization, unless the - // object is listed in transferList, in which case TransferForMessaging() - // is attempted first. + // object is listed in transferList and is kTransferable, in which case + // TransferForMessaging() is attempted first. + // - kTransferableAndCloneable: + // This object can be transferred or cloned. // After a successful clone, FinalizeTransferRead() is called on the receiving // end, and can read deserialize JS data possibly serialized by a previous // FinalizeTransferWrite() call. - enum class TransferMode { - kUntransferable, - kTransferable, - kCloneable + // By default, a BaseObject is kDisallowCloneAndTransfer and a JS Object is + // kCloneable unless otherwise specified. + enum TransferMode : uint32_t { + kDisallowCloneAndTransfer = 0, + kTransferable = 1 << 0, + kCloneable = 1 << 1, + kTransferableAndCloneable = kTransferable | kCloneable, }; virtual TransferMode GetTransferMode() const; virtual std::unique_ptr TransferForMessaging(); diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc index 84f2528a28f8d0..3ce4d26be84e7b 100644 --- a/src/crypto/crypto_x509.cc +++ b/src/crypto/crypto_x509.cc @@ -493,7 +493,6 @@ X509Certificate::X509CertificateTransferData::Deserialize( Unwrap(handle.As())); } - BaseObject::TransferMode X509Certificate::GetTransferMode() const { return BaseObject::TransferMode::kCloneable; } diff --git a/src/env_properties.h b/src/env_properties.h index bc4e840f956c04..687422d1a4fb0b 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -21,6 +21,8 @@ V(arrow_message_private_symbol, "node:arrowMessage") \ V(contextify_context_private_symbol, "node:contextify:context") \ V(decorated_private_symbol, "node:decorated") \ + V(transfer_mode_private_symbol, "node:transfer_mode") \ + V(js_transferable_wrapper_private_symbol, "node:js_transferable_wrapper") \ V(napi_type_tag, "node:napi:type_tag") \ V(napi_wrapper, "node:napi:wrapper") \ V(untransferable_object_private_symbol, "node:untransferableObject") \ @@ -357,6 +359,7 @@ V(http2ping_constructor_template, v8::ObjectTemplate) \ V(i18n_converter_template, v8::ObjectTemplate) \ V(intervalhistogram_constructor_template, v8::FunctionTemplate) \ + V(js_transferable_constructor_template, v8::FunctionTemplate) \ V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \ V(message_port_constructor_template, v8::FunctionTemplate) \ V(microtask_queue_ctor_template, v8::FunctionTemplate) \ diff --git a/src/histogram.h b/src/histogram.h index 773161fedbd2ed..a637c1a2e4e348 100644 --- a/src/histogram.h +++ b/src/histogram.h @@ -137,7 +137,7 @@ class HistogramBase : public BaseObject, public HistogramImpl { v8::Local wrap, std::shared_ptr histogram); - TransferMode GetTransferMode() const override { + BaseObject::TransferMode GetTransferMode() const override { return TransferMode::kCloneable; } std::unique_ptr CloneForMessaging() const override; @@ -213,7 +213,7 @@ class IntervalHistogram : public HandleWrap, public HistogramImpl { static void Start(const v8::FunctionCallbackInfo& args); static void Stop(const v8::FunctionCallbackInfo& args); - TransferMode GetTransferMode() const override { + BaseObject::TransferMode GetTransferMode() const override { return TransferMode::kCloneable; } std::unique_ptr CloneForMessaging() const override; diff --git a/src/node_blob.cc b/src/node_blob.cc index 7b04a658fc9e73..1a69b99965f81a 100644 --- a/src/node_blob.cc +++ b/src/node_blob.cc @@ -392,7 +392,7 @@ Blob::BlobTransferData::Deserialize( } BaseObject::TransferMode Blob::GetTransferMode() const { - return BaseObject::TransferMode::kCloneable; + return TransferMode::kCloneable; } std::unique_ptr Blob::CloneForMessaging() const { diff --git a/src/node_file.cc b/src/node_file.cc index fbcde2fb9b2788..e2f01c293abc86 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -300,13 +300,14 @@ void FileHandle::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("current_read", current_read_); } -FileHandle::TransferMode FileHandle::GetTransferMode() const { - return reading_ || closing_ || closed_ ? - TransferMode::kUntransferable : TransferMode::kTransferable; +BaseObject::TransferMode FileHandle::GetTransferMode() const { + return reading_ || closing_ || closed_ + ? TransferMode::kDisallowCloneAndTransfer + : TransferMode::kTransferable; } std::unique_ptr FileHandle::TransferForMessaging() { - CHECK_NE(GetTransferMode(), TransferMode::kUntransferable); + CHECK_NE(GetTransferMode(), TransferMode::kDisallowCloneAndTransfer); auto ret = std::make_unique(fd_); closed_ = true; return ret; diff --git a/src/node_file.h b/src/node_file.h index 4599546c524530..0fa2da0b4f7f8a 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -381,7 +381,7 @@ class FileHandle final : public AsyncWrap, public StreamBase { FileHandle(const FileHandle&&) = delete; FileHandle& operator=(const FileHandle&&) = delete; - TransferMode GetTransferMode() const override; + BaseObject::TransferMode GetTransferMode() const override; std::unique_ptr TransferForMessaging() override; private: diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 19b393bb308db6..15f6af079fdba7 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -42,32 +42,12 @@ using v8::WasmModuleObject; namespace node { using BaseObjectList = std::vector>; +using TransferMode = BaseObject::TransferMode; // Hack to have WriteHostObject inform ReadHostObject that the value // should be treated as a regular JS object. Used to transfer process.env. static const uint32_t kNormalObject = static_cast(-1); -BaseObject::TransferMode BaseObject::GetTransferMode() const { - return BaseObject::TransferMode::kUntransferable; -} - -std::unique_ptr BaseObject::TransferForMessaging() { - return CloneForMessaging(); -} - -std::unique_ptr BaseObject::CloneForMessaging() const { - return {}; -} - -Maybe BaseObject::NestedTransferables() const { - return Just(BaseObjectList {}); -} - -Maybe BaseObject::FinalizeTransferRead( - Local context, ValueDeserializer* deserializer) { - return Just(true); -} - namespace worker { Maybe TransferData::FinalizeTransferWrite( @@ -95,7 +75,8 @@ class DeserializerDelegate : public ValueDeserializer::Delegate { const std::vector>& shared_array_buffers, const std::vector& wasm_modules, const std::optional& shared_value_conveyor) - : host_objects_(host_objects), + : env_(env), + host_objects_(host_objects), shared_array_buffers_(shared_array_buffers), wasm_modules_(wasm_modules), shared_value_conveyor_(shared_value_conveyor) {} @@ -107,7 +88,12 @@ class DeserializerDelegate : public ValueDeserializer::Delegate { return MaybeLocal(); if (id != kNormalObject) { CHECK_LT(id, host_objects_.size()); - return host_objects_[id]->object(isolate); + Local object = host_objects_[id]->object(isolate); + if (env_->js_transferable_constructor_template()->HasInstance(object)) { + return Unwrap(object)->target(); + } else { + return object; + } } EscapableHandleScope scope(isolate); Local context = isolate->GetCurrentContext(); @@ -139,6 +125,7 @@ class DeserializerDelegate : public ValueDeserializer::Delegate { ValueDeserializer* deserializer = nullptr; private: + Environment* env_; const std::vector>& host_objects_; const std::vector>& shared_array_buffers_; const std::vector& wasm_modules_; @@ -317,12 +304,27 @@ class SerializerDelegate : public ValueSerializer::Delegate { ThrowDataCloneException(context_, message); } + bool HasCustomHostObject(Isolate* isolate) override { return true; } + + Maybe IsHostObject(Isolate* isolate, Local object) override { + if (BaseObject::IsBaseObject(object)) { + return Just(true); + } + + return Just(JSTransferable::IsJSTransferable(env_, context_, object)); + } + Maybe WriteHostObject(Isolate* isolate, Local object) override { if (BaseObject::IsBaseObject(object)) { return WriteHostObject( BaseObjectPtr { Unwrap(object) }); } + if (JSTransferable::IsJSTransferable(env_, context_, object)) { + JSTransferable* js_transferable = JSTransferable::Wrap(env_, object); + return WriteHostObject(BaseObjectPtr{js_transferable}); + } + // Convert process.env to a regular object. auto env_proxy_ctor_template = env_->env_proxy_ctor_template(); if (!env_proxy_ctor_template.IsEmpty() && @@ -374,10 +376,11 @@ class SerializerDelegate : public ValueSerializer::Delegate { for (uint32_t i = 0; i < host_objects_.size(); i++) { BaseObjectPtr host_object = std::move(host_objects_[i]); std::unique_ptr data; - if (i < first_cloned_object_index_) + if (i < first_cloned_object_index_) { data = host_object->TransferForMessaging(); - if (!data) + } else { data = host_object->CloneForMessaging(); + } if (!data) return Nothing(); if (data->FinalizeTransferWrite(context, serializer).IsNothing()) return Nothing(); @@ -416,24 +419,22 @@ class SerializerDelegate : public ValueSerializer::Delegate { private: Maybe WriteHostObject(BaseObjectPtr host_object) { BaseObject::TransferMode mode = host_object->GetTransferMode(); - if (mode == BaseObject::TransferMode::kUntransferable) { + if (mode == TransferMode::kDisallowCloneAndTransfer) { ThrowDataCloneError(env_->clone_unsupported_type_str()); return Nothing(); } - for (uint32_t i = 0; i < host_objects_.size(); i++) { - if (host_objects_[i] == host_object) { - serializer->WriteUint32(i); - return Just(true); + if (mode & TransferMode::kTransferable) { + for (uint32_t i = 0; i < host_objects_.size(); i++) { + if (host_objects_[i] == host_object) { + serializer->WriteUint32(i); + return Just(true); + } } - } - - if (mode == BaseObject::TransferMode::kTransferable) { THROW_ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST(env_); return Nothing(); } - CHECK_EQ(mode, BaseObject::TransferMode::kCloneable); uint32_t index = host_objects_.size(); if (first_cloned_object_index_ == SIZE_MAX) first_cloned_object_index_ = index; @@ -471,21 +472,19 @@ Maybe Message::Serialize(Environment* env, std::vector> array_buffers; for (uint32_t i = 0; i < transfer_list_v.length(); ++i) { - Local entry = transfer_list_v[i]; - if (entry->IsObject()) { - // See https://github.com/nodejs/node/pull/30339#issuecomment-552225353 - // for details. - bool untransferable; - if (!entry.As()->HasPrivate( - context, - env->untransferable_object_private_symbol()) - .To(&untransferable)) { - return Nothing(); - } - if (untransferable) { - ThrowDataCloneException(context, env->transfer_unsupported_type_str()); - return Nothing(); - } + Local entry_val = transfer_list_v[i]; + if (!entry_val->IsObject()) { + // Only object can be transferred. + THROW_ERR_INVALID_TRANSFER_OBJECT(env); + return Nothing(); + } + Local entry = entry_val.As(); + // See https://github.com/nodejs/node/pull/30339#issuecomment-552225353 + // for details. + if (entry->HasPrivate(context, env->untransferable_object_private_symbol()) + .ToChecked()) { + ThrowDataCloneException(context, env->transfer_unsupported_type_str()); + return Nothing(); } // Currently, we support ArrayBuffers and BaseObjects for which @@ -519,49 +518,55 @@ Maybe Message::Serialize(Environment* env, array_buffers.push_back(ab); serializer.TransferArrayBuffer(id, ab); continue; - } else if (entry->IsObject() && - BaseObject::IsBaseObject(entry.As())) { - // Check if the source MessagePort is being transferred. - if (!source_port.IsEmpty() && entry == source_port) { - ThrowDataCloneException( - context, - FIXED_ONE_BYTE_STRING(env->isolate(), - "Transfer list contains source port")); - return Nothing(); - } - BaseObjectPtr host_object { - Unwrap(entry.As()) }; - if (env->message_port_constructor_template()->HasInstance(entry) && - (!host_object || - static_cast(host_object.get())->IsDetached())) { - ThrowDataCloneException( - context, - FIXED_ONE_BYTE_STRING( - env->isolate(), - "MessagePort in transfer list is already detached")); - return Nothing(); - } - if (std::find(delegate.host_objects_.begin(), - delegate.host_objects_.end(), - host_object) != delegate.host_objects_.end()) { - ThrowDataCloneException( - context, - String::Concat(env->isolate(), - FIXED_ONE_BYTE_STRING( - env->isolate(), - "Transfer list contains duplicate "), - entry.As()->GetConstructorName())); + } + + // Check if the source MessagePort is being transferred. + if (!source_port.IsEmpty() && entry == source_port) { + ThrowDataCloneException( + context, + FIXED_ONE_BYTE_STRING(env->isolate(), + "Transfer list contains source port")); + return Nothing(); + } + BaseObjectPtr host_object; + if (BaseObject::IsBaseObject(entry)) { + host_object = BaseObjectPtr{Unwrap(entry)}; + } else { + if (!JSTransferable::IsJSTransferable(env, context, entry)) { + THROW_ERR_INVALID_TRANSFER_OBJECT(env); return Nothing(); } - if (host_object && host_object->GetTransferMode() == - BaseObject::TransferMode::kTransferable) { - delegate.AddHostObject(host_object); - continue; - } + JSTransferable* js_transferable = JSTransferable::Wrap(env, entry); + host_object = BaseObjectPtr{js_transferable}; } - THROW_ERR_INVALID_TRANSFER_OBJECT(env); - return Nothing(); + if (env->message_port_constructor_template()->HasInstance(entry) && + (!host_object || + static_cast(host_object.get())->IsDetached())) { + ThrowDataCloneException( + context, + FIXED_ONE_BYTE_STRING( + env->isolate(), + "MessagePort in transfer list is already detached")); + return Nothing(); + } + if (std::find(delegate.host_objects_.begin(), + delegate.host_objects_.end(), + host_object) != delegate.host_objects_.end()) { + ThrowDataCloneException( + context, + String::Concat( + env->isolate(), + FIXED_ONE_BYTE_STRING(env->isolate(), + "Transfer list contains duplicate "), + entry->GetConstructorName())); + return Nothing(); + } + if (host_object && + host_object->GetTransferMode() == TransferMode::kTransferable) { + delegate.AddHostObject(host_object); + continue; + } } if (delegate.AddNestedHostObjects().IsNothing()) return Nothing(); @@ -878,9 +883,8 @@ std::unique_ptr MessagePort::Detach() { } BaseObject::TransferMode MessagePort::GetTransferMode() const { - if (IsDetached()) - return BaseObject::TransferMode::kUntransferable; - return BaseObject::TransferMode::kTransferable; + if (IsDetached()) return TransferMode::kDisallowCloneAndTransfer; + return TransferMode::kTransferable; } std::unique_ptr MessagePort::TransferForMessaging() { @@ -1185,80 +1189,112 @@ Local GetMessagePortConstructorTemplate(Environment* env) { return GetMessagePortConstructorTemplate(env); } -JSTransferable::JSTransferable(Environment* env, Local obj) +// static +JSTransferable* JSTransferable::Wrap(Environment* env, Local target) { + Local context = env->context(); + Local wrapper_val = + target->GetPrivate(context, env->js_transferable_wrapper_private_symbol()) + .ToLocalChecked(); + DCHECK(wrapper_val->IsObject() || wrapper_val->IsUndefined()); + JSTransferable* wrapper; + if (wrapper_val->IsObject()) { + wrapper = Unwrap(wrapper_val); + } else { + Local wrapper_obj = env->js_transferable_constructor_template() + ->GetFunction(context) + .ToLocalChecked() + ->NewInstance(context) + .ToLocalChecked(); + wrapper = new JSTransferable(env, wrapper_obj, target); + target + ->SetPrivate( + context, env->js_transferable_wrapper_private_symbol(), wrapper_obj) + .ToChecked(); + } + return wrapper; +} + +// static +bool JSTransferable::IsJSTransferable(Environment* env, + v8::Local context, + v8::Local object) { + return object->HasPrivate(context, env->transfer_mode_private_symbol()) + .ToChecked(); +} + +JSTransferable::JSTransferable(Environment* env, + Local obj, + Local target) : BaseObject(env, obj) { MakeWeak(); + target_.Reset(env->isolate(), target); + target_.SetWeak(); } -void JSTransferable::New(const FunctionCallbackInfo& args) { - CHECK(args.IsConstructCall()); - new JSTransferable(Environment::GetCurrent(args), args.This()); +Local JSTransferable::target() const { + return target_.Get(env()->isolate()); } -JSTransferable::TransferMode JSTransferable::GetTransferMode() const { +BaseObject::TransferMode JSTransferable::GetTransferMode() const { // Implement `kClone in this ? kCloneable : kTransferable`. HandleScope handle_scope(env()->isolate()); errors::TryCatchScope ignore_exceptions(env()); - bool has_clone; - if (!object()->Has(env()->context(), - env()->messaging_clone_symbol()).To(&has_clone)) { - return TransferMode::kUntransferable; + Local transfer_mode_val = + target() + ->GetPrivate(env()->context(), env()->transfer_mode_private_symbol()) + .ToLocalChecked(); + if (!transfer_mode_val->IsUint32()) { + return TransferMode::kDisallowCloneAndTransfer; } - - return has_clone ? TransferMode::kCloneable : TransferMode::kTransferable; + return static_cast(transfer_mode_val.As()->Value()); } std::unique_ptr JSTransferable::TransferForMessaging() { - return TransferOrClone(TransferMode::kTransferable); + return TransferOrClone(); } std::unique_ptr JSTransferable::CloneForMessaging() const { - return TransferOrClone(TransferMode::kCloneable); + return TransferOrClone(); } -std::unique_ptr JSTransferable::TransferOrClone( - TransferMode mode) const { +template +std::unique_ptr JSTransferable::TransferOrClone() const { // Call `this[symbol]()` where `symbol` is `kClone` or `kTransfer`, // which should return an object with `data` and `deserializeInfo` properties; // `data` is written to the serializer later, and `deserializeInfo` is stored // on the `TransferData` instance as a string. HandleScope handle_scope(env()->isolate()); Local context = env()->isolate()->GetCurrentContext(); - Local method_name = mode == TransferMode::kCloneable ? - env()->messaging_clone_symbol() : env()->messaging_transfer_symbol(); + Local method_name = mode == TransferMode::kCloneable + ? env()->messaging_clone_symbol() + : env()->messaging_transfer_symbol(); Local method; - if (!object()->Get(context, method_name).ToLocal(&method)) { + if (!target()->Get(context, method_name).ToLocal(&method) || + !method->IsFunction()) { return {}; } - if (method->IsFunction()) { - Local result_v; - if (!method.As()->Call( - context, object(), 0, nullptr).ToLocal(&result_v)) { - return {}; - } - - if (result_v->IsObject()) { - Local result = result_v.As(); - Local data; - Local deserialize_info; - if (!result->Get(context, env()->data_string()).ToLocal(&data) || - !result->Get(context, env()->deserialize_info_string()) - .ToLocal(&deserialize_info)) { - return {}; - } - Utf8Value deserialize_info_str(env()->isolate(), deserialize_info); - if (*deserialize_info_str == nullptr) return {}; - return std::make_unique( - *deserialize_info_str, Global(env()->isolate(), data)); - } + Local result_v; + if (!method.As() + ->Call(context, target(), 0, nullptr) + .ToLocal(&result_v) || + !result_v->IsObject()) { + return {}; } - if (mode == TransferMode::kTransferable) - return TransferOrClone(TransferMode::kCloneable); - else + Local result = result_v.As(); + Local data; + Local deserialize_info; + if (!result->Get(context, env()->data_string()).ToLocal(&data) || + !result->Get(context, env()->deserialize_info_string()) + .ToLocal(&deserialize_info)) { return {}; + } + Utf8Value deserialize_info_str(env()->isolate(), deserialize_info); + if (*deserialize_info_str == nullptr) return {}; + return std::make_unique(*deserialize_info_str, + Global(env()->isolate(), data)); } Maybe @@ -1269,14 +1305,15 @@ JSTransferable::NestedTransferables() const { Local method_name = env()->messaging_transfer_list_symbol(); Local method; - if (!object()->Get(context, method_name).ToLocal(&method)) { + if (!target()->Get(context, method_name).ToLocal(&method)) { return Nothing(); } if (!method->IsFunction()) return Just(BaseObjectList {}); Local list_v; - if (!method.As()->Call( - context, object(), 0, nullptr).ToLocal(&list_v)) { + if (!method.As() + ->Call(context, target(), 0, nullptr) + .ToLocal(&list_v)) { return Nothing(); } if (!list_v->IsArray()) return Just(BaseObjectList {}); @@ -1287,8 +1324,19 @@ JSTransferable::NestedTransferables() const { Local value; if (!list->Get(context, i).ToLocal(&value)) return Nothing(); - if (value->IsObject() && BaseObject::IsBaseObject(value.As())) - ret.emplace_back(Unwrap(value)); + if (!value->IsObject()) { + continue; + } + Local obj = value.As(); + if (BaseObject::IsBaseObject(obj)) { + ret.emplace_back(Unwrap(obj)); + continue; + } + if (!JSTransferable::IsJSTransferable(env(), context, obj)) { + continue; + } + JSTransferable* js_transferable = JSTransferable::Wrap(env(), obj); + ret.emplace_back(js_transferable); } return Just(ret); } @@ -1303,12 +1351,12 @@ Maybe JSTransferable::FinalizeTransferRead( Local method_name = env()->messaging_deserialize_symbol(); Local method; - if (!object()->Get(context, method_name).ToLocal(&method)) { + if (!target()->Get(context, method_name).ToLocal(&method)) { return Nothing(); } if (!method->IsFunction()) return Just(true); - if (method.As()->Call(context, object(), 1, &data).IsEmpty()) { + if (method.As()->Call(context, target(), 1, &data).IsEmpty()) { return Nothing(); } return Just(true); @@ -1342,11 +1390,15 @@ BaseObjectPtr JSTransferable::Data::Deserialize( if (!env->messaging_deserialize_create_object() ->Call(context, Null(env->isolate()), 1, &info) .ToLocal(&ret) || - !ret->IsObject() || !BaseObject::IsBaseObject(ret.As())) { + !ret->IsObject()) { return {}; } - return BaseObjectPtr { Unwrap(ret) }; + if (!JSTransferable::IsJSTransferable(env, context, ret.As())) { + return {}; + } + JSTransferable* js_transferable = JSTransferable::Wrap(env, ret.As()); + return BaseObjectPtr{js_transferable}; } Maybe JSTransferable::Data::FinalizeTransferWrite( @@ -1521,13 +1573,11 @@ static void InitMessaging(Local target, } { - Local t = - NewFunctionTemplate(isolate, JSTransferable::New); + Local t = FunctionTemplate::New(isolate); t->InstanceTemplate()->SetInternalFieldCount( JSTransferable::kInternalFieldCount); t->SetClassName(OneByteString(isolate, "JSTransferable")); - SetConstructorFunction( - context, target, "JSTransferable", t, SetConstructorFunctionFlag::NONE); + env->isolate_data()->set_js_transferable_constructor_template(t); } SetConstructorFunction(context, @@ -1564,7 +1614,6 @@ static void InitMessaging(Local target, static void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(MessageChannel); registry->Register(BroadcastChannel); - registry->Register(JSTransferable::New); registry->Register(MessagePort::New); registry->Register(MessagePort::PostMessage); registry->Register(MessagePort::Start); diff --git a/src/node_messaging.h b/src/node_messaging.h index 1c2a564d8d58e1..d1c16bc1c03cb7 100644 --- a/src/node_messaging.h +++ b/src/node_messaging.h @@ -290,7 +290,7 @@ class MessagePort : public HandleWrap { // NULL pointer to the C++ MessagePort object is also detached. inline bool IsDetached() const; - TransferMode GetTransferMode() const override; + BaseObject::TransferMode GetTransferMode() const override; std::unique_ptr TransferForMessaging() override; void MemoryInfo(MemoryTracker* tracker) const override; @@ -319,15 +319,17 @@ class MessagePort : public HandleWrap { friend class MessagePortData; }; -// Provide a base class from which JS classes that should be transferable or -// cloneable by postMessage() can inherit. +// Provide a wrapper class created when a built-in JS classes that being +// transferable or cloneable by postMessage(). // See e.g. FileHandle in internal/fs/promises.js for an example. class JSTransferable : public BaseObject { public: - JSTransferable(Environment* env, v8::Local obj); - static void New(const v8::FunctionCallbackInfo& args); + static JSTransferable* Wrap(Environment* env, v8::Local target); + static bool IsJSTransferable(Environment* env, + v8::Local context, + v8::Local object); - TransferMode GetTransferMode() const override; + BaseObject::TransferMode GetTransferMode() const override; std::unique_ptr TransferForMessaging() override; std::unique_ptr CloneForMessaging() const override; v8::Maybe>> @@ -340,8 +342,17 @@ class JSTransferable : public BaseObject { SET_MEMORY_INFO_NAME(JSTransferable) SET_SELF_SIZE(JSTransferable) + v8::Local target() const; + private: - std::unique_ptr TransferOrClone(TransferMode mode) const; + JSTransferable(Environment* env, + v8::Local obj, + v8::Local target); + + template + std::unique_ptr TransferOrClone() const; + + v8::Global target_; class Data : public TransferData { public: diff --git a/src/node_sockaddr.h b/src/node_sockaddr.h index 0a4633b9a33d7e..84aa3adf5fc72a 100644 --- a/src/node_sockaddr.h +++ b/src/node_sockaddr.h @@ -176,7 +176,7 @@ class SocketAddressBase : public BaseObject { SET_MEMORY_INFO_NAME(SocketAddressBase) SET_SELF_SIZE(SocketAddressBase) - TransferMode GetTransferMode() const override { + BaseObject::TransferMode GetTransferMode() const override { return TransferMode::kCloneable; } std::unique_ptr CloneForMessaging() const override; @@ -367,7 +367,7 @@ class SocketAddressBlockListWrap : public BaseObject { SET_MEMORY_INFO_NAME(SocketAddressBlockListWrap) SET_SELF_SIZE(SocketAddressBlockListWrap) - TransferMode GetTransferMode() const override { + BaseObject::TransferMode GetTransferMode() const override { return TransferMode::kCloneable; } std::unique_ptr CloneForMessaging() const override; diff --git a/src/node_util.cc b/src/node_util.cc index ea991873b9a033..2af3b5a7276aae 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -474,6 +474,20 @@ void Initialize(Local target, V(SKIP_SYMBOLS); #undef V +#define V(name) \ + constants \ + ->Set( \ + context, \ + FIXED_ONE_BYTE_STRING(isolate, #name), \ + Integer::New(isolate, \ + static_cast(BaseObject::TransferMode::name))) \ + .Check(); + + V(kDisallowCloneAndTransfer); + V(kTransferable); + V(kCloneable); +#undef V + target->Set(context, env->constants_string(), constants).Check(); } diff --git a/test/parallel/test-messaging-maketransferable.js b/test/parallel/test-messaging-marktransfermode.js similarity index 82% rename from test/parallel/test-messaging-maketransferable.js rename to test/parallel/test-messaging-marktransfermode.js index 07b1081045d99f..20084b6c9c2078 100644 --- a/test/parallel/test-messaging-maketransferable.js +++ b/test/parallel/test-messaging-marktransfermode.js @@ -4,9 +4,6 @@ const common = require('../common'); const assert = require('assert'); -const { - JSTransferable, -} = require('internal/worker/js_transferable'); const { E, F } = require('internal/test/transfer'); // Tests that F is transferable even tho it does not directly, @@ -17,7 +14,6 @@ const mc = new MessageChannel(); mc.port1.onmessageerror = common.mustNotCall(); mc.port1.onmessage = common.mustCall(({ data }) => { - assert(!(data instanceof JSTransferable)); assert(data instanceof F); assert(data instanceof E); assert.strictEqual(data.b, 1); diff --git a/test/parallel/test-whatwg-webstreams-transfer.js b/test/parallel/test-whatwg-webstreams-transfer.js index 6abc2fe2a87f91..f8783259422890 100644 --- a/test/parallel/test-whatwg-webstreams-transfer.js +++ b/test/parallel/test-whatwg-webstreams-transfer.js @@ -31,7 +31,7 @@ const { } = require('internal/webstreams/util'); const { - makeTransferable, + markTransferMode, kClone, kTransfer, kDeserialize, @@ -324,7 +324,7 @@ const theData = 'hello'; port2.postMessage(readable, [readable]); - const notActuallyTransferable = makeTransferable({ + const notActuallyTransferable = { [kClone]() { return { data: {}, @@ -332,7 +332,8 @@ const theData = 'hello'; }; }, [kDeserialize]: common.mustNotCall(), - }); + }; + markTransferMode(notActuallyTransferable, true, false); controller.enqueue(notActuallyTransferable); } @@ -351,7 +352,7 @@ const theData = 'hello'; const writable = new WritableStream(source); - const notActuallyTransferable = makeTransferable({ + const notActuallyTransferable = { [kClone]() { return { data: {}, @@ -359,7 +360,8 @@ const theData = 'hello'; }; }, [kDeserialize]: common.mustNotCall(), - }); + }; + markTransferMode(notActuallyTransferable, true, false); port1.onmessage = common.mustCall(({ data }) => { const writer = data.getWriter(); diff --git a/test/parallel/test-worker-message-port-jstransferable-nested-untransferable.js b/test/parallel/test-worker-message-port-jstransferable-nested-untransferable.js index 621c42be5149cd..2f94fccd394452 100644 --- a/test/parallel/test-worker-message-port-jstransferable-nested-untransferable.js +++ b/test/parallel/test-worker-message-port-jstransferable-nested-untransferable.js @@ -3,16 +3,16 @@ const common = require('../common'); const assert = require('assert'); const { - JSTransferable, kTransfer, kTransferList + markTransferMode, kTransfer, kTransferList } = require('internal/worker/js_transferable'); const { MessageChannel } = require('worker_threads'); // Transferring a JSTransferable that refers to another, untransferable, value // in its transfer list should not crash hard. -class OuterTransferable extends JSTransferable { +class OuterTransferable { constructor() { - super(); + markTransferMode(this, false, true); // Create a detached MessagePort at this.inner const c = new MessageChannel(); this.inner = c.port1;