fix(node): preserve v8 deserializer view offsets - #36460
Conversation
bartlomieju
left a comment
There was a problem hiding this comment.
Confirmed the bug by reading the surrounding code. op_v8_new_deserializer built buf_slice from data.add(offset) but stored ptr: Some(data.cast()) β the backing-store base, not the view start. So op_v8_read_raw_bytes returned an offset measured from the ArrayBuffer, and the JS side then did getViewByteOffset(view) + byteOffset, adding the view offset a second time. Any host object deserialized from a view with a nonzero byteOffset read from the wrong place. NonNull::new(data_ptr) fixes the base, and the JS addition then becomes correct.
The second half is a separate real bug in the same function: the unaligned fallback did Buffer.from(getViewBuffer(view), byteOffset, byteLength) β indexing the whole ArrayBuffer with a view-relative offset. Switching it to offset lines it up with the aligned branch above.
The -1 sentinel is worth having on its own: returning 0 on a failed read_raw_bytes was indistinguishable from a legitimate read at offset 0, so a truncated payload silently deserialized from the start of the buffer instead of erroring. The checked_sub/checked_add/end > deser.buf.len chain and the zero-length-backing-store branch look right.
One nit: Node throws ERR_BUFFER_OUT_OF_BOUNDS here (node_serdes.cc: THROW_ERR_BUFFER_OUT_OF_BOUNDS(env, "ReadRawBytes() failed")). The message matches but the code doesn't, so err.code is undefined for anyone branching on it. Worth using the node error class. Not blocking. LGTM.
## Summary - compute raw-byte positions relative to the supplied ArrayBufferView - reject failed or out-of-range raw-byte reads - apply the view offset consistently when constructing host-object views - cover nonzero-offset, truncated, and empty-input cases ## Problem The deserializer initialized V8 with bytes from the supplied view but calculated returned positions from the start of the underlying ArrayBuffer. The JavaScript layer then added the view offset again, so host objects deserialized from slices could use the wrong bytes. Failed raw-byte reads also returned zero, making them indistinguishable from a valid offset at the beginning of the view. ## Validation - ./tools/format.js ext/node/ops/v8.rs ext/node/polyfills/v8.ts tests/unit_node/v8_test.ts - ./tools/lint.js --js ext/node/polyfills/v8.ts tests/unit_node/v8_test.ts - cargo check -p deno_node --features deno_core/v8 - cargo build --bin deno - cargo build -p test_server - cargo test -p unit_node_tests --test unit_node v8_test -- --nocapture
Summary
Problem
The deserializer initialized V8 with bytes from the supplied view but calculated returned positions from the start of the underlying ArrayBuffer. The JavaScript layer then added the view offset again, so host objects deserialized from slices could use the wrong bytes. Failed raw-byte reads also returned zero, making them indistinguishable from a valid offset at the beginning of the view.
Validation