diff options
| -rw-r--r-- | cli/tests/unit/structured_clone_test.ts | 23 | ||||
| -rw-r--r-- | core/ops_builtin_v8.rs | 26 | ||||
| -rw-r--r-- | ext/web/06_streams.js | 8 | ||||
| -rw-r--r-- | ext/web/13_message_port.js | 32 |
4 files changed, 63 insertions, 26 deletions
diff --git a/cli/tests/unit/structured_clone_test.ts b/cli/tests/unit/structured_clone_test.ts index fdad0dba7..c60b38a8a 100644 --- a/cli/tests/unit/structured_clone_test.ts +++ b/cli/tests/unit/structured_clone_test.ts @@ -27,4 +27,27 @@ Deno.test("correct DataCloneError message", () => { DOMException, "Value not transferable", ); + + const ab = new ArrayBuffer(1); + // detach ArrayBuffer + structuredClone(ab, { transfer: [ab] }); + assertThrows( + () => { + structuredClone(ab, { transfer: [ab] }); + }, + DOMException, + "ArrayBuffer at index 0 is already detached", + ); + + const ab2 = new ArrayBuffer(0); + assertThrows( + () => { + structuredClone([ab2, ab], { transfer: [ab2, ab] }); + }, + DOMException, + "ArrayBuffer at index 1 is already detached", + ); + + // ab2 should not be detached after above failure + structuredClone(ab2, { transfer: [ab2] }); }); diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs index c04fe3400..818ba03ee 100644 --- a/core/ops_builtin_v8.rs +++ b/core/ops_builtin_v8.rs @@ -1,5 +1,6 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. use crate::bindings::script_origin; +use crate::error::custom_error; use crate::error::is_instance_of_error; use crate::error::range_error; use crate::error::type_error; @@ -51,6 +52,7 @@ pub(crate) fn init_builtins_v8() -> Vec<OpDecl> { op_store_pending_promise_exception::decl(), op_remove_pending_promise_exception::decl(), op_has_pending_promise_exception::decl(), + op_arraybuffer_was_detached::decl(), ] } @@ -448,22 +450,27 @@ fn op_serialize( if let Some(transferred_array_buffers) = transferred_array_buffers { let state_rc = JsRuntime::state(scope); let state = state_rc.borrow_mut(); - for i in 0..transferred_array_buffers.length() { - let i = v8::Number::new(scope, i as f64).into(); + for index in 0..transferred_array_buffers.length() { + let i = v8::Number::new(scope, index as f64).into(); let buf = transferred_array_buffers.get(scope, i).unwrap(); let buf = v8::Local::<v8::ArrayBuffer>::try_from(buf).map_err(|_| { type_error("item in transferredArrayBuffers not an ArrayBuffer") })?; if let Some(shared_array_buffer_store) = &state.shared_array_buffer_store { - // TODO(lucacasonato): we need to check here that the buffer is not - // already detached. We can not do that because V8 does not provide - // a way to check if a buffer is already detached. if !buf.is_detachable() { return Err(type_error( "item in transferredArrayBuffers is not transferable", )); } + + if buf.was_detached() { + return Err(custom_error( + "DOMExceptionOperationError", + format!("ArrayBuffer at index {} is already detached", index), + )); + } + let backing_store = buf.get_backing_store(); buf.detach(); let id = shared_array_buffer_store.insert(backing_store); @@ -877,3 +884,12 @@ fn op_has_pending_promise_exception<'a>( .pending_promise_exceptions .contains_key(&promise_global) } + +#[op(v8)] +fn op_arraybuffer_was_detached<'a>( + _scope: &mut v8::HandleScope<'a>, + input: serde_v8::Value<'a>, +) -> Result<bool, Error> { + let ab = v8::Local::<v8::ArrayBuffer>::try_from(input.v8_value)?; + Ok(ab.was_detached()) +} diff --git a/ext/web/06_streams.js b/ext/web/06_streams.js index 52488efb6..6dbf69951 100644 --- a/ext/web/06_streams.js +++ b/ext/web/06_streams.js @@ -193,7 +193,13 @@ * @returns {boolean} */ function isDetachedBuffer(O) { - return ReflectHas(O, isFakeDetached); + if (O.byteLength !== 0) { + return false; + } + // TODO(marcosc90) remove isFakeDetached once transferArrayBuffer + // actually detaches the buffer + return ReflectHas(O, isFakeDetached) || + core.ops.op_arraybuffer_was_detached(O); } /** diff --git a/ext/web/13_message_port.js b/ext/web/13_message_port.js index 253ed7ecd..7343fbe58 100644 --- a/ext/web/13_message_port.js +++ b/ext/web/13_message_port.js @@ -26,9 +26,6 @@ SymbolFor, SymbolIterator, TypeError, - WeakSet, - WeakSetPrototypeAdd, - WeakSetPrototypeHas, } = window.__bootstrap.primordials; class MessageChannel { @@ -239,30 +236,25 @@ return [data, transferables]; } - const detachedArrayBuffers = new WeakSet(); - /** * @param {any} data * @param {object[]} transferables * @returns {globalThis.__bootstrap.messagePort.MessageData} */ function serializeJsMessageData(data, transferables) { - const transferredArrayBuffers = ArrayPrototypeFilter( - transferables, - (a) => ObjectPrototypeIsPrototypeOf(ArrayBufferPrototype, a), - ); - - for (const arrayBuffer of transferredArrayBuffers) { - // This is hacky with both false positives and false negatives for - // detecting detached array buffers. V8 needs to add a way to tell if a - // buffer is detached or not. - if (WeakSetPrototypeHas(detachedArrayBuffers, arrayBuffer)) { - throw new DOMException( - "Can not transfer detached ArrayBuffer", - "DataCloneError", - ); + const transferredArrayBuffers = []; + for (let i = 0, j = 0; i < transferables.length; i++) { + const ab = transferables[i]; + if (ObjectPrototypeIsPrototypeOf(ArrayBufferPrototype, ab)) { + if (ab.byteLength === 0 && core.ops.op_arraybuffer_was_detached(ab)) { + throw new DOMException( + `ArrayBuffer at index ${j} is already detached`, + "DataCloneError", + ); + } + j++; + transferredArrayBuffers.push(ab); } - WeakSetPrototypeAdd(detachedArrayBuffers, arrayBuffer); } const serializedData = core.serialize(data, { |