buffer: decode hex from the low byte of each UTF-16 code unit like node

[robobun]

Problem

• Buffer.from(str, "hex") on a two-byte string stops decoding at the first code unit above 0xFF. Node decodes such a unit from its low byte instead:

  Buffer.from("f\uff41", "hex")        // node: <Buffer fa>     bun: <Buffer >
  Buffer.from("fa\uff41\uff41", "hex") // node: <Buffer fa aa>  bun: <Buffer fa>
  Buffer.from("f\u0141", "hex")        // node: <Buffer fa>     bun: <Buffer >

• The same decoder is behind every hex-encoded string input (buf.write, buf.hexWrite, buf.fill, Buffer.alloc(n, str, "hex"), buf.indexOf(str, 0, "hex"), createHash().update(str, "hex"), Bun.CryptoHasher.update(str, "hex"), fs.writeFileSync(p, str, "hex"), ...), so all of them differ from node the same way.

• Cause: the u16 instantiation of _decode_hex_to_bytes (src/bun_core/string/immutable.rs) had an explicit > u8::MAX bail-out in the scalar loop, and the Highway kernel DecodeHex16Impl (src/jsc/bindings/highway_strings.cpp) classified the full 16-bit lane, so a unit above 0xFF never passed as a hex digit.

• One-byte (Latin-1) strings already matched node; only strings stored as UTF-16 were affected.

Fix

• HexChar gains hex_byte(), the byte the decoder classifies: identity for u8, the low byte for u16. The scalar loop looks that byte up in the table, and the > u8::MAX check is gone.
• DecodeHexVectorLoop masks 16-bit lanes with 0xFF before classifying them (two extra Ands per iteration on the UTF-16 path only; the 8-bit path is unchanged), and the kernel's scalar tail narrows the same way. The two paths still agree, which the HIGHWAY_MIN_PAIRS split relies on.
• Why this is correct: it matches node. Node's hex decoder narrows each code unit with static_cast<uint8_t> before looking it up, so a unit above 0xFF whose low byte is a hex digit decodes as that digit, and one whose low byte is not a hex digit stops the decode. The second half is unchanged here; only the first half was missing. Bun already made the same call for base64/base64url two-byte strings in Decode Buffer base64/base64url with simdutf's lenient accept-garbage mode #31423, and this brings hex in line with that.
• Blast radius: the u16 decoder is only instantiated from construct_from_u16 and write_u16 in src/runtime/webcore/encoding.rs (the node encoding paths). The other callers of the shared decoder (csrf tokens, postgres bytea, IPC, dev server) pass byte slices and compile to the unchanged u8 instantiation. Buffer.byteLength(str, "hex") is length / 2 and is unaffected. Uint8Array.fromHex is JSC's own implementation and does not go through this code.
• Tests:
  • test/js/node/buffer.test.js: a short-input test (scalar path) covering Buffer.from, write, hexWrite, fill, Buffer.alloc(n, str, "hex") and indexOf; and, in the SIMD boundary block, the test that pinned the old behaviour is replaced by two tests placing a wide unit inside the vector blocks, in the 128-bit mop-up region of wide targets and in the scalar tail of a 75-pair input, one for units that decode and one for units that stop the decode. The block's JS reference decoder now narrows like node. The "decodes" tests fail on the released bun and pass with this change; the "stops" tests pass both ways and guard the unchanged half.
  • test/js/bun/util/bun-cryptohasher.test.ts: Bun.CryptoHasher (the String::encode entry point), its HMAC form and node:crypto's createHash hash the narrowed bytes.
  • Every expected value in the new tests was checked against node v26.3.0.
  • bun bd test test/js/node/buffer.test.js (637 pass), bun bd test test/js/bun/util/bun-cryptohasher.test.ts (403 pass), test/js/bun/util/csrf.test.ts, and the vendored node tests test-buffer-badhex, -alloc, -fill, -write, -from, -bytelength, -indexof, -includes pass with the debug build. The local machine has AVX-512 (VBMI), so the widest kernel, its 128-bit mop-up and the scalar tail all ran.
  • A 4000-case randomized corpus (details below) is byte-for-byte identical between node v26.3.0 and the debug build; the released bun differs from node on 812 of the lines.

Background

• JSC stores a string either as one byte per character (Latin-1) or as UTF-16 code units; a string becomes two-byte as soon as it contains any character above U+00FF, and stays two-byte through substring/slice even if the remaining characters are ASCII (that is how hex strings end up on this path in practice, see Error while calling Buffer.from with substring contains chinese character #4919). encoding.rs dispatches the two representations to the u8 and u16 instantiations of the same decoder.
• Node's Buffer string decoders (hex, base64, latin1) work on the low byte of each UTF-16 code unit; they never reject a unit for being above 0xFF. For hex this means a non-hex unit stops the decode exactly like an ASCII 'g' would, and a unit whose low byte is a hex digit contributes that digit.
• The shared decoder (_decode_hex_to_bytes) hands inputs of 16 or more pairs to a runtime-dispatched Highway kernel and decodes shorter inputs in a scalar loop. The kernel decodes whole vector blocks and returns at the first block containing an invalid pair; its own scalar tail then finds the exact pair. On wide targets (AVX2/AVX-512) a 128-bit pass mops up the remainder before the scalar tail. The semantics therefore live in three places (Rust scalar loop, vector loop, kernel scalar tail), and all three are changed together here.

Differential corpus

Generator: 4000 strings of up to 200 code units drawn from ASCII hex digits, wide units whose low byte is a hex digit, wide units with an arbitrary low byte, Latin-1 bytes, lone surrogates and fullwidth forms (U+FF00..U+FF5F), half of them forced into two-byte storage even when every unit fits in a byte. For each string the script prints Buffer.byteLength, Buffer.from(s, "hex"), the return value and resulting contents of buf.write(s, "hex") into a buffer of random length 0..63, Buffer.alloc(7).fill(s, "hex") (or the thrown error code), and indexOf(s, 0, "hex") on a buffer containing the decoded bytes at offset 1.

binary	lines differing from node v26.3.0
this branch (debug build)	0 / 4000
released bun (1.4.0 canary)	812 / 4000

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

Limit details: You’ve used all 5 included reviews currently available under your plan.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5f6bdad9-47a2-45c1-a181-05abbe4d1bd5

📥 Commits

Reviewing files that changed from the base of the PR and between 079cb0a and 187597c.

📒 Files selected for processing (5)

• src/bun_core/string/immutable.rs
• src/highway/lib.rs
• src/jsc/bindings/highway_strings.cpp
• test/js/bun/util/bun-cryptohasher.test.ts
• test/js/node/buffer.test.js

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: merged as 7a8ce75.

Reproduced on the released bun and on main at 079cb0a against node v26.3.0:

Buffer.from("f\uff41", "hex")        // node: <Buffer fa>     bun: <Buffer >
Buffer.from("fa\uff41\uff41", "hex") // node: <Buffer fa aa>  bun: <Buffer fa>

Self-review after merge found nothing wrong with the fix itself; two notes came out of it:

• The pre-existing "16-bit path" sweeps in this test block (toUTF16 = s => (s + "\u0100").slice(0, -1)) actually produce 8-bit strings, so before this PR nothing reached DecodeHex16Impl with valid input across lengths; the wide-unit tests added here do. Follow-up with a harness helper and the five affected call sites: test: add a forceUTF16 harness helper; the slice-off-a-wide-char idiom yields 8-bit strings #39478.
• Now that the two-byte hex decode is by definition "narrow each code unit, then decode as 8-bit", the u16 hex kernel (DecodeHex16Impl, decode_hex_u16, impl HexChar for u16 and their baseline allowlist entries) could be deleted in favour of narrowing in encoding.rs the way the base64 arms already do, at the cost of one copy for two-byte inputs. Left as-is here since Stop zero-filling buffers that are overwritten anyway (compressors, image, text encoding, I/O, networking, paths) and make the compressors' allocations fallible #39417 is reworking those arms; noting it in case someone wants to pick it up afterwards.

[claude]

LGTM — focused Node compat fix applied consistently across all three hex-decode paths.

What was reviewed:

• Traced the three implementation sites (Rust scalar loop, Highway vector loop, kernel scalar tail) — all narrow u16 to low byte before classifying; the u8 path is byte-identical (if constexpr guard, identity hex_byte()).
• Checked callers of decode_hex_to_bytes*: only encoding.rs reaches the u16 instantiation; postgres/csrf/IPC/DevServer pass &[u8] and are unaffected. The removed Into<u32> bound on HexChar has no other consumers.
• The replaced test was pinning the old (Node-divergent) behavior; its replacement covers both the new "decodes" half and the unchanged "stops" half at positions spanning vector blocks, the 128-bit mop-up and the scalar tail.

Extended reasoning...

Overview

This PR aligns Bun's UTF-16 hex decoder with Node.js: code units above 0xFF are now classified by their low byte (so U+FF41 decodes as 'A') instead of stopping the decode. Three implementation sites are changed in lockstep: the Rust scalar loop in _decode_hex_to_bytes (via a new HexChar::hex_byte() that truncates u16 to u8), the Highway vector loop DecodeHexVectorLoop (an if constexpr masking 16-bit lanes with 0xFF before classification), and the kernel's scalar tail in DecodeHex16Impl (a static_cast<uint8_t> before ScalarHexNibble). Doc comments in highway/lib.rs are updated to match. Tests are added in buffer.test.js and bun-cryptohasher.test.ts.

Security risks

None. This is a Node-compat behavior change in a string decoder; no auth, crypto key handling, or untrusted-size arithmetic is touched. The change makes the u16 decoder more permissive in the same way Node is, but the classification still goes through the same HEX_TABLE / IsAsciiHexDigit gate — a low byte outside [0-9A-Fa-f] still stops the decode. No new allocation or bounds arithmetic.

Level of scrutiny

Medium. The change touches a runtime-dispatched SIMD kernel, which is cross-platform-sensitive, but the added operations (hn::And with a broadcast constant, guarded by if constexpr on lane width) are basic Highway primitives available on every target. The u8 path is provably unchanged: hex_byte() for u8 is identity, and the if constexpr (sizeof == 2) branch is dead. I traced IsAsciiHexAlpha / IsAsciiHexDigit / HexNibbleValue on masked values (U+FF41 → 0x41 → nibble 10; U+0147 → 0x47 → invalid) and they behave correctly.

Other factors

• Blast radius verified: grepped all decode_hex_to_bytes* callers — only encoding.rs (the Buffer/String encoding dispatch) instantiates the u16 form; postgres bytea, CSRF, IPC and the dev server all pass byte slices and compile to the unchanged u8 path.
• Test coverage is thorough: short-input scalar path, 75-pair inputs with wide units placed at vector-block boundaries / the 128-bit mop-up region / the scalar tail, both the "decodes as low byte" and "stops on non-hex low byte" halves, and multiple entry points (Buffer.from, write, hexWrite, fill, alloc, indexOf, CryptoHasher.update, createHash). The reference JS decoder was updated to & 0xff to model Node.
• Replaced test: the deleted test explicitly pinned the old Node-divergent behavior ("treats UTF-16 code units above 0xFF as invalid even when their low byte is a hex digit"). Its replacement guards both the new behavior and the unchanged invariant (units whose low byte is not a hex digit still stop the decode), so no coverage is lost — this is the documented exception to "never delete a test".
• Trait bound removal: HexChar: Into<u32> was dropped; grep confirms no consumer relies on it beyond the removed .into() calls.
• Differential testing against Node v26.3.0 (4000-case corpus, 0 diffs) and precedent for the same low-byte narrowing in base64 (#31423) give additional confidence.

[robobun]

Thanks for the review. Nothing to change from it: the three decode sites, the u8 callers and the replaced test are as described. A second review pass is still running on my side; I will update the status comment above once it and the Buildkite build (#100129) finish.