cloudflare / cloudflare/workerd

🐛 Bug Report — Runtime APIs: workerd bug in Buffer.prototype.utf8Write under nodejs_compat

Open
#6,897 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
8.7k
Forks
739
Avg merge
2d 20h
Merged PRs (30d)
174

Description

protobufjs picks a BufferWriter when Buffer exists (which it does, under `nodejs_compat`). For string fields it splits on length — `node_modules/protobufjs/src/writer_buffer.js:74`:

```
function writeStringBuffer(val, buf, pos) {
if (val.length < 40) // short strings: pure JS, safe
util.utf8.write(val, buf, pos);
else if (buf.utf8Write) // >= 40 chars: workerd's buggy path
buf.utf8Write(val, pos);
```

workerd's `utf8Write(string, offset)` defaults the optional third length argument to the buffer's total length instead of the space remaining after offset — then rejects that default for exceeding the remaining space. I confirmed it in a local `wrangler dev`:

```
Buffer.allocUnsafe(100).utf8Write("x".repeat(45), 40)
→ RangeError: ... It must be >= 0 && <= 60. Received 100
```

60 is 100 - 40 (remaining), 100 is the whole buffer.

Trigger: any protobuf string field of 40+ characters written at a non-zero offset.

# The fix

New `src/buffer-utf8write-fix.ts` — passes length explicitly so the bad default never applies. It probes for the bug first and no-ops if `workerd` fixes it, so it can stay in place safely. Called once at the top of `src/index.ts`:

```
import { Buffer } from "node:buffer";

// workerd's `Buffer.prototype.utf8Write(string, offset)` defaults its optional
// `length` argument to the buffer's total length instead of the space remaining
// after `offset`, and then rejects that default for exceeding the remaining
// space:
//
// RangeError: The value of "length" is out of range. It must be >= 0 && <= 60. Received 85
//
// protobufjs's BufferWriter uses exactly that two-argument call for every string
// field of 40 or more characters (shorter ones take a pure-JS path), so encoding
// any message with a long string field throws once the field is written at a
// non-zero offset — which is always, unless it is the only field. See
// node_modules/protobufjs/src/writer_buffer.js, writeStringBuffer().
//
// Passing `length` explicitly avoids the bad default. The patch is applied only
// when the bug is detected, so it turns itself off if workerd fixes this.
export function patchBufferUtf8Write(): void {
const utf8Write = (Buffer.prototype as any).utf8Write;
if (typeof utf8Write !== "function") {
return;
}

// Writing 4 bytes at offset 4 of an 8 byte buffer fits exactly; it only
// throws if `length` defaulted to the whole buffer length.
try {
utf8Write.call(Buffer.alloc(8), "abcd", 4);
return;
} catch {
// Bug is present: fall through and patch.
}

(Buffer.prototype as any).utf8Write = function (this: Buffer, string: string, offset?: number, length?: number): number {
const start = offset ?? 0;
return utf8Write.call(this, string, start, length ?? this.byteLength - start);
};
}
```

Patch is idempotent, and plain Buffer semantics are unchanged (explicit length still respected, no over-write past it).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.