Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src,lib: make JSTransferables based on private symbols #47956

Merged
merged 5 commits into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions deps/v8/include/v8-value-serializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions deps/v8/src/api/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
30 changes: 28 additions & 2 deletions deps/v8/src/objects/value-serializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions deps/v8/src/objects/value-serializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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_;
Expand Down
53 changes: 52 additions & 1 deletion deps/v8/test/unittests/objects/value-serializer-unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 {
Expand Down Expand Up @@ -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:
Expand Down
28 changes: 16 additions & 12 deletions lib/internal/abort_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}

/**
Expand Down
16 changes: 9 additions & 7 deletions lib/internal/blob.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const {
const { URL } = require('internal/url');

const {
makeTransferable,
markTransferMode,
kClone,
kDeserialize,
} = require('internal/worker/js_transferable');
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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, {
Expand Down
10 changes: 5 additions & 5 deletions lib/internal/blocklist.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const {
} = require('internal/socketaddress');

const {
JSTransferable,
markTransferMode,
kClone,
kDeserialize,
} = require('internal/worker/js_transferable');
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 3 additions & 5 deletions lib/internal/crypto/keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const {
} = require('internal/util/types');

const {
makeTransferable,
markTransferMode,
kClone,
kDeserialize,
} = require('internal/worker/js_transferable');
Expand Down Expand Up @@ -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]() {
Expand Down
10 changes: 5 additions & 5 deletions lib/internal/crypto/x509.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const {
} = require('internal/errors');

const {
JSTransferable,
markTransferMode,
kClone,
kDeserialize,
} = require('internal/worker/js_transferable');
Expand Down Expand Up @@ -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) {
Expand All @@ -115,7 +115,7 @@ class X509Certificate extends JSTransferable {
['string', 'Buffer', 'TypedArray', 'DataView'],
buffer);
}
super();
markTransferMode(this, true, false);
this[kHandle] = parseX509(buffer);
}

Expand Down
Loading