lib: convert transfer sequence to array in js

This commit lets `tranfer` passed to `structuredClone` get validated at
JS layer by doing webidl conversion. This avoids the C++ to JS function
call overhead in the native implementaiton of `structuredClone`

PR-URL: https://github.com/nodejs/node/pull/55317
Fixes: https://github.com/nodejs/node/issues/55280
Refs: https://github.com/nodejs/node/pull/50330
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
This commit is contained in:
Jason Zhang 2024-10-14 04:41:21 +10:30 committed by GitHub
parent ac49b20c75
commit 50b4ada551
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 89 additions and 24 deletions

View File

@ -38,8 +38,11 @@ const {
} = require('internal/process/task_queues');
defineOperation(globalThis, 'queueMicrotask', queueMicrotask);
const { structuredClone } = internalBinding('messaging');
defineOperation(globalThis, 'structuredClone', structuredClone);
defineLazyProperties(
globalThis,
'internal/worker/js_transferable',
['structuredClone'],
);
defineLazyProperties(globalThis, 'buffer', ['atob', 'btoa']);
// https://html.spec.whatwg.org/multipage/web-messaging.html#broadcasting-to-other-browsing-contexts

View File

@ -31,7 +31,7 @@ const {
},
} = require('internal/errors');
const { structuredClone } = internalBinding('messaging');
const { structuredClone } = require('internal/worker/js_transferable');
const {
lazyDOMException,
kEnumerableProperty,

View File

@ -38,6 +38,16 @@ converters.any = (V) => {
return V;
};
converters.object = (V, opts = kEmptyObject) => {
if (type(V) !== 'Object') {
throw makeException(
'is not an object',
kEmptyObject,
);
}
return V;
};
// https://webidl.spec.whatwg.org/#abstract-opdef-integerpart
const integerPart = MathTrunc;
@ -189,6 +199,8 @@ converters.DOMString = function DOMString(V) {
return String(V);
};
converters['sequence<object>'] = createSequenceConverter(converters.object);
function codedTypeError(message, errorProperties = kEmptyObject) {
// eslint-disable-next-line no-restricted-syntax
const err = new TypeError(message);

View File

@ -74,6 +74,7 @@ const {
kTransfer,
kTransferList,
markTransferMode,
structuredClone,
} = require('internal/worker/js_transferable');
const {
@ -88,8 +89,6 @@ const {
kControllerErrorFunction,
} = require('internal/streams/utils');
const { structuredClone } = internalBinding('messaging');
const {
ArrayBufferViewGetBuffer,
ArrayBufferViewGetByteLength,

View File

@ -3,6 +3,13 @@ const {
Error,
StringPrototypeSplit,
} = primordials;
const {
codes: {
ERR_INVALID_ARG_TYPE,
ERR_MISSING_ARGS,
},
} = require('internal/errors');
const webidl = require('internal/webidl');
const {
messaging_deserialize_symbol,
messaging_transfer_symbol,
@ -11,6 +18,7 @@ const {
} = internalBinding('symbols');
const {
setDeserializerCreateObjectFunction,
structuredClone: nativeStructuredClone,
} = internalBinding('messaging');
const {
privateSymbols: {
@ -90,9 +98,38 @@ function markTransferMode(obj, cloneable = false, transferable = false) {
obj[transfer_mode_private_symbol] = mode;
}
function structuredClone(value, options) {
if (arguments.length === 0) {
throw new ERR_MISSING_ARGS('The value argument must be specified');
}
// TODO(jazelly): implement generic webidl dictionary converter
const prefix = 'Options';
const optionsType = webidl.type(options);
if (optionsType !== 'Undefined' && optionsType !== 'Null' && optionsType !== 'Object') {
throw new ERR_INVALID_ARG_TYPE(
prefix,
['object', 'null', 'undefined'],
options,
);
}
const key = 'transfer';
const idlOptions = { __proto__: null, [key]: [] };
if (options != null && key in options && options[key] !== undefined) {
idlOptions[key] = webidl.converters['sequence<object>'](options[key], {
__proto__: null,
context: 'Transfer',
});
}
const serializedData = nativeStructuredClone(value, idlOptions);
return serializedData;
}
module.exports = {
markTransferMode,
setup,
structuredClone,
kClone: messaging_clone_symbol,
kDeserialize: messaging_deserialize_symbol,
kTransfer: messaging_transfer_symbol,

View File

@ -1578,28 +1578,21 @@ static void StructuredClone(const FunctionCallbackInfo<Value>& args) {
Realm* realm = Realm::GetCurrent(context);
Environment* env = realm->env();
if (args.Length() == 0) {
return THROW_ERR_MISSING_ARGS(env, "The value argument must be specified");
}
Local<Value> value = args[0];
TransferList transfer_list;
if (!args[1]->IsNullOrUndefined()) {
if (!args[1]->IsObject()) {
return THROW_ERR_INVALID_ARG_TYPE(
env, "The options argument must be either an object or undefined");
}
Local<Object> options = args[1].As<Object>();
Local<Value> transfer_list_v;
if (!options->Get(context, env->transfer_string())
.ToLocal(&transfer_list_v)) {
return;
}
Local<Object> options = args[1].As<Object>();
Local<Value> transfer_list_v;
if (!options->Get(context, env->transfer_string())
.ToLocal(&transfer_list_v)) {
return;
}
// TODO(joyeecheung): implement this in JS land to avoid the C++ -> JS
// cost to convert a sequence into an array.
if (!GetTransferList(env, context, transfer_list_v, &transfer_list)) {
Local<Array> arr = transfer_list_v.As<Array>();
size_t length = arr->Length();
transfer_list.AllocateSufficientStorage(length);
for (size_t i = 0; i < length; i++) {
if (!arr->Get(context, i).ToLocal(&transfer_list[i])) {
return;
}
}

View File

@ -8,12 +8,12 @@ assert.throws(() => structuredClone(undefined, ''), { code: 'ERR_INVALID_ARG_TYP
assert.throws(() => structuredClone(undefined, 1), { code: 'ERR_INVALID_ARG_TYPE' });
assert.throws(() => structuredClone(undefined, { transfer: 1 }), { code: 'ERR_INVALID_ARG_TYPE' });
assert.throws(() => structuredClone(undefined, { transfer: '' }), { code: 'ERR_INVALID_ARG_TYPE' });
assert.throws(() => structuredClone(undefined, { transfer: null }), { code: 'ERR_INVALID_ARG_TYPE' });
// Options can be null or undefined.
assert.strictEqual(structuredClone(undefined), undefined);
assert.strictEqual(structuredClone(undefined, null), undefined);
// Transfer can be null or undefined.
assert.strictEqual(structuredClone(undefined, { transfer: null }), undefined);
assert.strictEqual(structuredClone(undefined, { }), undefined);
// Transferables or its subclasses should be received with its closest transferable superclass
@ -43,6 +43,27 @@ for (const Transferrable of [File, Blob]) {
assert.ok(extendedTransfer instanceof Transferrable);
}
// Transfer can be iterable
{
const value = {
a: new ReadableStream(),
b: new WritableStream(),
};
const cloned = structuredClone(value, {
transfer: {
*[Symbol.iterator]() {
for (const key in value) {
yield value[key];
}
}
}
});
for (const key in value) {
assert.ok(value[key].locked);
assert.ok(!cloned[key].locked);
}
}
{
// See: https://github.com/nodejs/node/issues/49940
const cloned = structuredClone({}, {