From 258c1285a5b1fb32799d621a25b5e2b557556e94 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 22 Apr 2026 13:11:37 +0200 Subject: [PATCH 1/4] Document BinaryReader.tag() validation change and inline for performance --- MANUAL.md | 6 ++ packages/protobuf/src/wire/binary-encoding.ts | 64 +++++++++++++++---- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/MANUAL.md b/MANUAL.md index a1aca0987..cb87c2cb3 100644 --- a/MANUAL.md +++ b/MANUAL.md @@ -1252,6 +1252,12 @@ user.firstName; // "Homer" user.active; // true ``` +`BinaryReader.tag()` validates that the tag is a well-formed uint32 varint. Overlong varints (more than 5 bytes) and +5-byte varints whose value overflows uint32 are rejected with an error. Earlier versions of Protobuf-ES silently decoded +these cases by truncating the high bits, which caused malformed payloads to be stored as unknown fields instead of +failing to parse. Producers generating tags via `BinaryWriter.tag()` are unaffected, since `BinaryWriter` only emits +valid tags. + ### Text encoding We require the WHATWG [Text Encoding API] to convert UTF-8 from and to binary. The API is widely available, but it is diff --git a/packages/protobuf/src/wire/binary-encoding.ts b/packages/protobuf/src/wire/binary-encoding.ts index 1d0ed0122..4266910e3 100644 --- a/packages/protobuf/src/wire/binary-encoding.ts +++ b/packages/protobuf/src/wire/binary-encoding.ts @@ -386,24 +386,45 @@ export class BinaryReader { } /** - * Reads a tag - field number and wire type. Tags are uint32 varints; values - * that do not fit in uint32 are rejected. + * Reads a tag - field number and wire type. + * + * Inlined for the hot path. Tags are uint32 varints: at most 5 bytes, and + * the 5th byte can carry only 4 value bits (its upper 4 bits and the + * continuation bit must all be zero). Rejects overlong or overflowing + * varints. */ tag(): [number, WireType] { - const start = this.pos; - const tag = this.uint32(); - const bytesRead = this.pos - start; - if (bytesRead > 5 || (bytesRead == 5 && this.buf[this.pos - 1] > 0x0f)) { - throw new Error("illegal tag: varint overflows uint32"); + let b = this.buf[this.pos++]; + if ((b & 0x80) == 0) { + this.assertBounds(); + return parseTag(b); + } + let tag = b & 0x7f; + b = this.buf[this.pos++]; + tag |= (b & 0x7f) << 7; + if ((b & 0x80) == 0) { + this.assertBounds(); + return parseTag(tag); } - const fieldNo = tag >>> 3; - const wireType = tag & 7; - if (fieldNo <= 0 || wireType > 5) { - throw new Error( - "illegal tag: field no " + fieldNo + " wire type " + wireType, - ); + b = this.buf[this.pos++]; + tag |= (b & 0x7f) << 14; + if ((b & 0x80) == 0) { + this.assertBounds(); + return parseTag(tag); + } + b = this.buf[this.pos++]; + tag |= (b & 0x7f) << 21; + if ((b & 0x80) == 0) { + this.assertBounds(); + return parseTag(tag); + } + b = this.buf[this.pos++]; + if (b > 0x0f) { + throw new Error("illegal tag: varint overflows uint32"); } - return [fieldNo, wireType]; + tag = (tag | (b << 28)) >>> 0; + this.assertBounds(); + return parseTag(tag); } /** @@ -579,6 +600,21 @@ export class BinaryReader { } } +/** + * Split a decoded tag varint into field number and wire type, rejecting + * field number 0 and wire types outside 0-5. + */ +function parseTag(tag: number): [number, WireType] { + const fieldNo = tag >>> 3; + const wireType = tag & 7; + if (fieldNo <= 0 || wireType > 5) { + throw new Error( + "illegal tag: field no " + fieldNo + " wire type " + wireType, + ); + } + return [fieldNo, wireType]; +} + /** * Assert a valid signed protobuf 32-bit integer as a number or string. */ From 55d94771472672676e4f0537e11deaf05009c69d Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 22 Apr 2026 13:18:35 +0200 Subject: [PATCH 2/4] Fix bundle-size --- packages/bundle-size/README.md | 10 +++++----- packages/bundle-size/chart.svg | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/bundle-size/README.md b/packages/bundle-size/README.md index 4cea0c954..2d8ae6c4e 100644 --- a/packages/bundle-size/README.md +++ b/packages/bundle-size/README.md @@ -16,11 +16,11 @@ usually do. We repeat this for an increasing number of files. | code generator | files | bundle size | minified | compressed | | ------------------- | ----: | ----------: | --------: | ---------: | -| Protobuf-ES | 1 | 133,976 b | 69,171 b | 15,890 b | -| Protobuf-ES | 4 | 136,165 b | 70,678 b | 16,587 b | -| Protobuf-ES | 8 | 138,927 b | 72,449 b | 17,134 b | -| Protobuf-ES | 16 | 149,377 b | 80,430 b | 19,442 b | -| Protobuf-ES | 32 | 177,168 b | 102,448 b | 24,922 b | +| Protobuf-ES | 1 | 134,690 b | 69,523 b | 15,945 b | +| Protobuf-ES | 4 | 136,879 b | 71,027 b | 16,621 b | +| Protobuf-ES | 8 | 139,641 b | 72,798 b | 17,182 b | +| Protobuf-ES | 16 | 150,091 b | 80,779 b | 19,489 b | +| Protobuf-ES | 32 | 177,882 b | 102,800 b | 24,953 b | | protobuf-javascript | 1 | 314,120 b | 244,024 b | 35,999 b | | protobuf-javascript | 4 | 340,137 b | 258,996 b | 37,473 b | | protobuf-javascript | 8 | 360,931 b | 270,573 b | 38,585 b | diff --git a/packages/bundle-size/chart.svg b/packages/bundle-size/chart.svg index 8d0cfef80..10f2e255e 100644 --- a/packages/bundle-size/chart.svg +++ b/packages/bundle-size/chart.svg @@ -43,14 +43,14 @@ 0 KiB - + Protobuf-ES -Protobuf-ES 15.52 KiB for 1 files -Protobuf-ES 16.2 KiB for 4 files -Protobuf-ES 16.73 KiB for 8 files -Protobuf-ES 18.99 KiB for 16 files -Protobuf-ES 24.34 KiB for 32 files +Protobuf-ES 15.57 KiB for 1 files +Protobuf-ES 16.23 KiB for 4 files +Protobuf-ES 16.78 KiB for 8 files +Protobuf-ES 19.03 KiB for 16 files +Protobuf-ES 24.37 KiB for 32 files From 8eebb54695ea738d0ace195af803675fa96c1bb0 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 22 Apr 2026 13:55:03 +0200 Subject: [PATCH 3/4] Drop doc change --- MANUAL.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/MANUAL.md b/MANUAL.md index cb87c2cb3..a1aca0987 100644 --- a/MANUAL.md +++ b/MANUAL.md @@ -1252,12 +1252,6 @@ user.firstName; // "Homer" user.active; // true ``` -`BinaryReader.tag()` validates that the tag is a well-formed uint32 varint. Overlong varints (more than 5 bytes) and -5-byte varints whose value overflows uint32 are rejected with an error. Earlier versions of Protobuf-ES silently decoded -these cases by truncating the high bits, which caused malformed payloads to be stored as unknown fields instead of -failing to parse. Producers generating tags via `BinaryWriter.tag()` are unaffected, since `BinaryWriter` only emits -valid tags. - ### Text encoding We require the WHATWG [Text Encoding API] to convert UTF-8 from and to binary. The API is widely available, but it is From a5b69c31faded6a01d6f0095e6832aea9069ac9d Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 22 Apr 2026 14:16:18 +0200 Subject: [PATCH 4/4] Doc int32 varint max --- packages/protobuf/src/wire/varint.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/protobuf/src/wire/varint.ts b/packages/protobuf/src/wire/varint.ts index 334f22056..3f5a13c5b 100644 --- a/packages/protobuf/src/wire/varint.ts +++ b/packages/protobuf/src/wire/varint.ts @@ -306,6 +306,10 @@ export function varint32write(value: number, bytes: number[]): void { /** * Read an unsigned 32 bit varint. * + * A uint32 value fits in 5 varint bytes, but this reader accepts up to 10 + * bytes and discards the extra high bits. Negative `int32` values encode as + * 10-byte varints and must decode back to the original 32-bit value. + * * See https://github.com/protocolbuffers/protobuf/blob/8a71927d74a4ce34efe2d8769fda198f52d20d12/js/experimental/runtime/kernel/buffer_decoder.js#L220 */ export function varint32read(this: T): number {