Skip to content

fix(ext/node): fix Buffer.concat, expose internal/buffer, implement markAsUntransferable#32760

Merged
bartlomieju merged 5 commits intomainfrom
fix/node-compat-buffer-concat
Mar 17, 2026
Merged

fix(ext/node): fix Buffer.concat, expose internal/buffer, implement markAsUntransferable#32760
bartlomieju merged 5 commits intomainfrom
fix/node-compat-buffer-concat

Conversation

@bartlomieju
Copy link
Copy Markdown
Member

Summary

Fixes 3 Buffer compat tests from #32706 and fixes #31824.

  • Buffer.concat: Use TypedArrayPrototypeGetByteLength instead of .length to prevent spoofed length getters from causing uninitialized memory exposure
  • internal/buffer: Register as a requireable internal module and export utf8Write
  • markAsUntransferable: Implement using V8's set_detach_key API. Applied to the Buffer pool so ArrayBuffer.prototype.transfer() throws TypeError instead of detaching the shared pool. This prevents the bug where transferring a Buffer's backing store would break all subsequent Buffer.from() calls (Buffer is totally broken in the presence of transferable buffers #31824)

Enabled tests

  • test-buffer-concat.js
  • test-buffer-write-fast.js
  • test-buffer-pool-untransferable.js

Closes #31824

Test plan

🤖 Generated with Claude Code

bartlomieju and others added 3 commits March 15, 2026 21:42
…length

Use TypedArrayPrototypeGetByteLength instead of .length in
Buffer.concat() to prevent spoofed length getters from causing
uninitialized memory exposure.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Register internal/buffer as a requireable internal module and export
utf8Write from it, enabling test-buffer-write-fast.js.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ansferable

Add op_mark_as_untransferable using V8's set_detach_key to make
ArrayBuffers truly non-transferable (both via postMessage and
ArrayBuffer.prototype.transfer()). Apply it to the Buffer pool and
implement worker_threads.markAsUntransferable().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@kajukitli kajukitli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed with full repo context. Initial findings didn't hold up on closer inspection. Looks good.

@bartlomieju bartlomieju changed the title fix(node/buffer): fix Buffer.concat, expose internal/buffer, implement markAsUntransferable fix(ext/node): fix Buffer.concat, expose internal/buffer, implement markAsUntransferable Mar 16, 2026
Copy link
Copy Markdown
Member

@nathanwhit nathanwhit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

scope: &mut v8::PinScope<'_, '_>,
ab: v8::Local<v8::ArrayBuffer>,
) {
let key = v8::String::new(scope, "untransferable").unwrap();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could use v8_static_strings here

@bartlomieju bartlomieju enabled auto-merge (squash) March 17, 2026 19:57
@bartlomieju bartlomieju merged commit 6aa3354 into main Mar 17, 2026
220 of 222 checks passed
@bartlomieju bartlomieju deleted the fix/node-compat-buffer-concat branch March 17, 2026 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Buffer is totally broken in the presence of transferable buffers

3 participants