From 4b3bd89f080da4bbbf0cfd754f5ff603f84e4191 Mon Sep 17 00:00:00 2001 From: legendecas Date: Mon, 10 Apr 2023 02:24:06 +0800 Subject: [PATCH] src,lib: mark URL/URLSearchParams as untransferrable --- lib/buffer.js | 2 +- lib/internal/buffer.js | 15 ------- lib/internal/url.js | 5 +++ lib/internal/util.js | 25 ++++++++++++ lib/worker_threads.js | 2 +- src/base_object.h | 16 +++----- src/env_properties.h | 2 +- src/node_api.cc | 10 +++-- src/node_buffer.cc | 17 +++++--- src/node_file.cc | 6 +-- src/node_messaging.cc | 87 ++++++++++++++++++++++++++++++++++------ src/node_util.cc | 14 +++++++ test/wpt/status/url.json | 4 +- 13 files changed, 149 insertions(+), 56 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index ec20e3432fcd61..ae846c28a56349 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -85,6 +85,7 @@ const { normalizeEncoding, kIsEncodingSymbol, defineLazyProperties, + markAsUntransferable, } = require('internal/util'); const { isAnyArrayBuffer, @@ -123,7 +124,6 @@ const validateOffset = (value, name, min = 0, max = kMaxLength) => const { FastBuffer, - markAsUntransferable, addBufferPrototypeMethods, createUnsafeBuffer, } = require('internal/buffer'); diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js index fbe9de249348b3..39f8ceae661f56 100644 --- a/lib/internal/buffer.js +++ b/lib/internal/buffer.js @@ -33,12 +33,6 @@ const { getZeroFillToggle, } = internalBinding('buffer'); -const { - privateSymbols: { - untransferable_object_private_symbol, - }, -} = internalBinding('util'); - // Temporary buffers to convert numbers. const float32Array = new Float32Array(1); const uInt8Float32Array = new Uint8Array(float32Array.buffer); @@ -1045,14 +1039,6 @@ function addBufferPrototypeMethods(proto) { proto.utf8Write = utf8Write; } -// This would better be placed in internal/worker/io.js, but that doesn't work -// because Buffer needs this and that would introduce a cyclic dependency. -function markAsUntransferable(obj) { - if ((typeof obj !== 'object' && typeof obj !== 'function') || obj === null) - return; // This object is a primitive and therefore already untransferable. - obj[untransferable_object_private_symbol] = true; -} - // A toggle used to access the zero fill setting of the array buffer allocator // in C++. // |zeroFill| can be undefined when running inside an isolate where we @@ -1078,7 +1064,6 @@ function reconnectZeroFillToggle() { module.exports = { FastBuffer, addBufferPrototypeMethods, - markAsUntransferable, createUnsafeBuffer, readUInt16BE, readUInt32BE, diff --git a/lib/internal/url.js b/lib/internal/url.js index cb662b90ac052c..4c850419cf41f0 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -45,6 +45,7 @@ const { toUSVString, kEnumerableProperty, SideEffectFreeRegExpPrototypeSymbolReplace, + markAsUncloneable, } = require('internal/util'); const { @@ -376,6 +377,8 @@ class URLSearchParams { init = toUSVString(init); this.#searchParams = init ? parseParams(init) : []; } + + markAsUncloneable(this); } [inspect.custom](recurseTimes, ctx) { @@ -720,6 +723,8 @@ class URL { } this.#updateContext(href); + + markAsUncloneable(this); } [inspect.custom](depth, opts) { diff --git a/lib/internal/util.js b/lib/internal/util.js index 2ec432f838087b..5b08d9ba3f3a9f 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -57,6 +57,11 @@ const { privateSymbols: { arrow_message_private_symbol, decorated_private_symbol, + transfer_mode_private_symbol, + }, + constants: { + kUncloneable, + kCloneable, }, sleep: _sleep, toUSVString: _toUSVString, @@ -789,6 +794,24 @@ function setupCoverageHooks(dir) { return coverageDirectory; } +// This would better be placed in internal/worker/io.js, but that doesn't work +// because Buffer needs this and that would introduce a cyclic dependency. +// Mark the object as untransferable. If object occurs in the transfer list of +// a port.postMessage() call, it is ignored. +function markAsUntransferable(obj) { + if ((typeof obj !== 'object' && typeof obj !== 'function') || obj === null) + return; // This value is a primitive and therefore already cloneable. + obj[transfer_mode_private_symbol] = kCloneable; +} + +// Mark the object as uncloneable. If object occurs in the value of a +// port.postMessage() call, an error is thrown. +function markAsUncloneable(obj) { + if ((typeof obj !== 'object' && typeof obj !== 'function') || obj === null) + return; // This value is a primitive and therefore already cloneable. + obj[transfer_mode_private_symbol] = kUncloneable; +} + module.exports = { getLazy, assertCrypto, @@ -818,6 +841,8 @@ module.exports = { join, lazyDOMException, lazyDOMExceptionClass, + markAsUntransferable, + markAsUncloneable, normalizeEncoding, once, promisify, diff --git a/lib/worker_threads.js b/lib/worker_threads.js index 155cc11ecdaf43..0af63824186e60 100644 --- a/lib/worker_threads.js +++ b/lib/worker_threads.js @@ -20,7 +20,7 @@ const { const { markAsUntransferable, -} = require('internal/buffer'); +} = require('internal/util'); module.exports = { isMainThread, diff --git a/src/base_object.h b/src/base_object.h index 96f661c41284dc..78cabeb3d8a853 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). + // - kUncloneable: + // 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 @@ -160,11 +160,7 @@ class BaseObject : public MemoryRetainer { // 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 - }; + enum class TransferMode : uint8_t { kUncloneable, kTransferable, kCloneable }; virtual TransferMode GetTransferMode() const; virtual std::unique_ptr TransferForMessaging(); virtual std::unique_ptr CloneForMessaging() const; diff --git a/src/env_properties.h b/src/env_properties.h index 095094714aa400..bb5cdfda4e4f5d 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -21,9 +21,9 @@ 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(napi_type_tag, "node:napi:type_tag") \ V(napi_wrapper, "node:napi:wrapper") \ - V(untransferable_object_private_symbol, "node:untransferableObject") \ V(exit_info_private_symbol, "node:exit_info_private_symbol") \ V(require_private_symbol, "node:require_private_symbol") diff --git a/src/node_api.cc b/src/node_api.cc index 4d95d518286b87..b8a1aa2a04408f 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -1,6 +1,7 @@ #include "async_wrap-inl.h" #include "env-inl.h" #define NAPI_EXPERIMENTAL +#include "base_object.h" #include "js_native_api_v8.h" #include "memory_tracker-inl.h" #include "node_api.h" @@ -37,9 +38,12 @@ bool node_napi_env__::can_call_into_js() const { v8::Maybe node_napi_env__::mark_arraybuffer_as_untransferable( v8::Local ab) const { - return ab->SetPrivate(context(), - node_env()->untransferable_object_private_symbol(), - v8::True(isolate)); + return ab->SetPrivate( + context(), + node_env()->transfer_mode_private_symbol(), + v8::Integer::New( + isolate, + static_cast(node::BaseObject::TransferMode::kCloneable))); } void node_napi_env__::CallFinalizer(napi_finalize cb, void* data, void* hint) { diff --git a/src/node_buffer.cc b/src/node_buffer.cc index ff041274f90d24..1d862dd77009d1 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -459,8 +459,11 @@ MaybeLocal New(Environment* env, Local ab = CallbackInfo::CreateTrackedArrayBuffer(env, data, length, callback, hint); if (ab->SetPrivate(env->context(), - env->untransferable_object_private_symbol(), - True(env->isolate())).IsNothing()) { + env->transfer_mode_private_symbol(), + Integer::New(env->isolate(), + static_cast( + BaseObject::TransferMode::kCloneable))) + .IsNothing()) { return Local(); } MaybeLocal maybe_ui = Buffer::New(env, ab, 0, length); @@ -1181,10 +1184,12 @@ void GetZeroFillToggle(const FunctionCallbackInfo& args) { ab = ArrayBuffer::New(env->isolate(), std::move(backing)); } - ab->SetPrivate( - env->context(), - env->untransferable_object_private_symbol(), - True(env->isolate())).Check(); + ab->SetPrivate(env->context(), + env->transfer_mode_private_symbol(), + Integer::New(env->isolate(), + static_cast( + BaseObject::TransferMode::kCloneable))) + .Check(); args.GetReturnValue().Set(Uint32Array::New(ab, 0, 1)); } diff --git a/src/node_file.cc b/src/node_file.cc index 9e0083d10a2f87..992cc032adbfdb 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -300,12 +300,12 @@ void FileHandle::MemoryInfo(MemoryTracker* tracker) const { } FileHandle::TransferMode FileHandle::GetTransferMode() const { - return reading_ || closing_ || closed_ ? - TransferMode::kUntransferable : TransferMode::kTransferable; + return reading_ || closing_ || closed_ ? TransferMode::kUncloneable + : TransferMode::kTransferable; } std::unique_ptr FileHandle::TransferForMessaging() { - CHECK_NE(GetTransferMode(), TransferMode::kUntransferable); + CHECK_NE(GetTransferMode(), TransferMode::kUncloneable); auto ret = std::make_unique(fd_); closed_ = true; return ret; diff --git a/src/node_messaging.cc b/src/node_messaging.cc index b40868a1ceeff4..9d2543d62dc1a9 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -47,7 +47,7 @@ using BaseObjectList = std::vector>; static const uint32_t kNormalObject = static_cast(-1); BaseObject::TransferMode BaseObject::GetTransferMode() const { - return BaseObject::TransferMode::kUntransferable; + return BaseObject::TransferMode::kUncloneable; } std::unique_ptr BaseObject::TransferForMessaging() { @@ -288,6 +288,39 @@ void ThrowDataCloneException(Local context, Local message) { isolate->ThrowException(exception); } +Maybe ObjectHasTransferMode(Environment* env, + Local context, + Local val) { + bool has_transfer_mode; + if (!val->HasPrivate(context, env->transfer_mode_private_symbol()) + .To(&has_transfer_mode)) { + return Nothing(); + } + return Just(has_transfer_mode); +} + +Maybe GetObjectTransferMode(Environment* env, + Local context, + Local val) { + DCHECK(ObjectHasTransferMode(env, context, val).FromJust()); + + Local transfer_mode_val; + if (!val->GetPrivate(context, env->transfer_mode_private_symbol()) + .ToLocal(&transfer_mode_val)) { + return Nothing(); + } + DCHECK(transfer_mode_val->IsInt32()); + + int32_t transfer_mode; + if (!transfer_mode_val->Int32Value(context).To(&transfer_mode)) { + return Nothing(); + } + + DCHECK(transfer_mode >= 0 && + transfer_mode <= static_cast(TransferMode::kCloneable)); + return Just(static_cast(transfer_mode)); +} + // This tells V8 how to serialize objects that it does not understand // (e.g. C++ objects) into the output buffer, in a way that our own // DeserializerDelegate understands how to unpack. @@ -300,6 +333,16 @@ 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 ObjectHasTransferMode(env_, context_, object); + } + Maybe WriteHostObject(Isolate* isolate, Local object) override { if (BaseObject::IsBaseObject(object)) { return WriteHostObject( @@ -321,6 +364,14 @@ class SerializerDelegate : public ValueSerializer::Delegate { return serializer->WriteValue(env_->context(), normal_object); } + BaseObject::TransferMode transfer_mode; + if (!GetObjectTransferMode(env_, context_, object).To(&transfer_mode)) { + return Nothing(); + } + + // TODO(legendecas): custom JS object cloning. + DCHECK(transfer_mode == BaseObject::TransferMode::kUncloneable); + ThrowDataCloneError(env_->clone_unsupported_type_str()); return Nothing(); } @@ -393,7 +444,7 @@ class SerializerDelegate : public ValueSerializer::Delegate { private: Maybe WriteHostObject(BaseObjectPtr host_object) { BaseObject::TransferMode mode = host_object->GetTransferMode(); - if (mode == BaseObject::TransferMode::kUntransferable) { + if (mode == BaseObject::TransferMode::kUncloneable) { ThrowDataCloneError(env_->clone_unsupported_type_str()); return Nothing(); } @@ -452,18 +503,29 @@ Maybe Message::Serialize(Environment* env, 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)) { + bool has_transfer_mode; + if (!ObjectHasTransferMode(env, context, entry.As()) + .To(&has_transfer_mode)) { return Nothing(); } - if (untransferable) continue; + if (has_transfer_mode) { + BaseObject::TransferMode mode; + if (!GetObjectTransferMode(env, context, entry.As()) + .To(&mode)) { + return Nothing(); + } + if (mode == BaseObject::TransferMode::kUncloneable) { + ThrowDataCloneException(context, env->clone_unsupported_type_str()); + return Nothing(); + } + if (mode == BaseObject::TransferMode::kCloneable) { + continue; + } + } } // Currently, we support ArrayBuffers and BaseObjects for which - // GetTransferMode() does not return kUntransferable. + // GetTransferMode() does not return kUncloneable. if (entry->IsArrayBuffer()) { Local ab = entry.As(); // If we cannot render the ArrayBuffer unusable in this Isolate, @@ -525,7 +587,7 @@ Maybe Message::Serialize(Environment* env, return Nothing(); } if (host_object && host_object->GetTransferMode() != - BaseObject::TransferMode::kUntransferable) { + BaseObject::TransferMode::kUncloneable) { delegate.AddHostObject(host_object); continue; } @@ -849,8 +911,7 @@ std::unique_ptr MessagePort::Detach() { } BaseObject::TransferMode MessagePort::GetTransferMode() const { - if (IsDetached()) - return BaseObject::TransferMode::kUntransferable; + if (IsDetached()) return BaseObject::TransferMode::kUncloneable; return BaseObject::TransferMode::kTransferable; } @@ -1174,7 +1235,7 @@ JSTransferable::TransferMode JSTransferable::GetTransferMode() const { bool has_clone; if (!object()->Has(env()->context(), env()->messaging_clone_symbol()).To(&has_clone)) { - return TransferMode::kUntransferable; + return TransferMode::kUncloneable; } return has_clone ? TransferMode::kCloneable : TransferMode::kTransferable; diff --git a/src/node_util.cc b/src/node_util.cc index c434f201e592a4..12785ab9b14a9e 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -433,6 +433,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(kUncloneable); + V(kTransferable); + V(kCloneable); +#undef V + target->Set(context, env->constants_string(), constants).Check(); } diff --git a/test/wpt/status/url.json b/test/wpt/status/url.json index 0b4beb54549495..43f22c5afe5de3 100644 --- a/test/wpt/status/url.json +++ b/test/wpt/status/url.json @@ -6,9 +6,7 @@ "fail": { "note": "We are faking location with a URL object for the sake of the testharness and it has searchParams.", "expected": [ - "searchParams on location object", - "URL: no structured serialize/deserialize support", - "URLSearchParams: no structured serialize/deserialize support" + "searchParams on location object" ] } },