Skip to content

fix(ext/node): convert Buffer string write arguments once - #33

Merged
jackspirou merged 1 commit into
nimbus/v2.9.7from
fix/buffer-string-write-bounds
Sep 24, 2026
Merged

jackspirou merged 1 commit into
nimbus/v2.9.7from
fix/buffer-string-write-bounds

Conversation

@jackspirou

Copy link
Copy Markdown
Contributor

Summary

buf.asciiWrite(), buf.latin1Write(), and buf.utf8Write() compared the raw offset and length values and then used them again. Thus a valueOf() method ran more than once. Node.js 24.21 and 26.8 convert each argument once (nodejs/node#65043). The test-buffer-write.js fixture of those releases asserts this.

The methods also skipped the native steps of Node's StringWrite(). These bugs occurred on all Node.js versions:

Call on a 4-byte buffer Before Node.js and this PR
utf8Write('abcdef', 0, 0) 3 bytes 0
buf.write('abc', 0, 0) 3 bytes 0
asciiWrite('abcdef', NaN) 3 bytes 0
utf8Write('abcdef', '1', '2') 3 bytes 2

Change

  • A shared stringWrite() helper converts each argument once. It truncates each value and clamps the length, as the native write does. It reads the byte length again after conversion, because user code can resize or detach the buffer.
  • The bounds checks differ between Node.js releases. A new BufferStringWriteBoundsPolicy lets an embedder select the contract:
  • hexIndexOutOfRange() is now indexOutOfRange(), because the write path uses it too.

Verification

  • unit_node buffer_test: pass. The two new tests pass. Their expected values match Node.js 26.7, except for the single valueOf() call and the BigInt case, which are the #65043 contract.
  • node_compat test-buffer-write: 6 of 6 pass.
  • cargo test -p deno_node --lib policy: 6 of 6 pass, including the new policy test.
  • The CI clippy command, cargo fmt --check, dprint, and tools/lint.js --js: clean.
  • The Node 24.21 and Node 26.10 test-buffer-write.js fixtures from Nimbus: all assertions pass.
  • Pre-PR autoreview: clean.

`buf.asciiWrite()`, `buf.latin1Write()`, and `buf.utf8Write()` compared
the raw `offset` and `length` values and then used them again, so a
`valueOf()` method ran more than once. The methods also did not do the
native steps of Node's StringWrite(): a NaN offset was not treated as 0,
fractional values were not truncated, `utf8Write()` with a zero length
wrote the full string, and string-number arguments gave wrong results.

A shared stringWrite() helper now converts each argument once, as
Node.js 24.21 and later do (nodejs/node#65043). It then truncates each
value and clamps the length to the bytes that remain, as the native
write does. It reads the byte length again after conversion, because
user code can resize or detach the buffer.

The bounds checks differ between Node.js releases, so a new
BufferStringWriteBoundsPolicy lets an embedder select the contract:

- TruncateThenClamp: Node.js 20 and 22.0 through 22.8.
- CheckThenClamp: Node.js 22.9 and later 22.x (nodejs/node#54310,
  nodejs/node#54524).
- CheckThenReject: Node.js 23 and later (nodejs/node#54588). This is
  the default.
@jackspirou
jackspirou merged commit 3bbccc7 into nimbus/v2.9.7 Sep 24, 2026
1 check passed
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.

1 participant