Skip to content

buffer: match Node's Buffer.prototype.fill offset and end argument handling - #33033

Merged
Jarred-Sumner merged 8 commits into
mainfrom
farm/ba6603b8/buffer-fill-arg-reinterpretation
Jun 30, 2026
Merged

Jarred-Sumner merged 8 commits into
mainfrom
farm/ba6603b8/buffer-fill-arg-reinterpretation

buffer: match Node for zero-argument and five-plus-argument fill()

43d4dfe
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jun 29, 2026 in 22m 46s

Code review found 1 potential issue

Found 1 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 0
🟣 Pre-existing 1
Severity File:Line Issue
🟣 Pre-existing src/jsc/bindings/JSBuffer.cpp:1341 fill(str, offset, end, null) throws where Node accepts null encoding as utf8 (pre-existing)

Annotations

Check notice on line 1341 in src/jsc/bindings/JSBuffer.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

fill(str, offset, end, null) throws where Node accepts null encoding as utf8 (pre-existing)

🟣 One more pre-existing throw-where-Node-succeeds shape in this same intake, not introduced by the PR: `Buffer.alloc(5, 0xaa).fill("a", 1, 3, null)` returns `<aa 61 61 aa aa>` in Node (its `normalizeEncoding` checks `enc == null` and falls back to utf8), but Bun throws `ERR_INVALID_ARG_TYPE` because the gate at line 1365 checks `!encodingValue.isUndefined()` — `null` passes the gate and reaches `parseEncoding` → `validateString`. Since this PR already folded in the other adjacent pre-existing di