diff --git a/packages/protobuf-bench/README.md b/packages/protobuf-bench/README.md index 5d938e1d3..a90f258a0 100644 --- a/packages/protobuf-bench/README.md +++ b/packages/protobuf-bench/README.md @@ -10,5 +10,5 @@ server would usually do. | code generator | bundle size | minified | compressed | |---------------------|------------------------:|-----------------------:|-------------------:| -| protobuf-es | 126,666 b | 65,334 b | 15,946 b | +| protobuf-es | 126,944 b | 65,418 b | 15,948 b | | protobuf-javascript | 394,384 b | 288,654 b | 45,122 b | diff --git a/packages/protobuf-test/src/wire/binary-encoding.test.ts b/packages/protobuf-test/src/wire/binary-encoding.test.ts index 69940455e..4c7ad8b38 100644 --- a/packages/protobuf-test/src/wire/binary-encoding.test.ts +++ b/packages/protobuf-test/src/wire/binary-encoding.test.ts @@ -13,9 +13,9 @@ // limitations under the License. import { describe, expect, it } from "@jest/globals"; -import { BinaryWriter, WireType } from "@bufbuild/protobuf/wire"; -import { UserDesc } from "../gen/ts/extra/example_pb.js"; import { fromBinary } from "@bufbuild/protobuf"; +import { BinaryWriter, BinaryReader, WireType } from "@bufbuild/protobuf/wire"; +import { UserDesc } from "../gen/ts/extra/example_pb.js"; describe("BinaryWriter example", () => { it("should work as expected", () => { @@ -32,3 +32,78 @@ describe("BinaryWriter example", () => { expect(user.active).toBe(true); }); }); + +describe("BinaryReader", () => { + describe("skip", () => { + it("should skip group", () => { + const reader = new BinaryReader( + new BinaryWriter() + .tag(1, WireType.StartGroup) + .tag(33, WireType.Varint) + .bool(true) + .tag(1, WireType.EndGroup) + .finish(), + ); + const [fieldNo, wireType] = reader.tag(); + expect(fieldNo).toBe(1); + expect(wireType).toBe(WireType.StartGroup); + reader.skip(WireType.StartGroup, 1); + expect(reader.pos).toBe(reader.len); + }); + it("should skip nested group", () => { + const reader = new BinaryReader( + new BinaryWriter() + .tag(1, WireType.StartGroup) + .tag(1, WireType.StartGroup) + .tag(1, WireType.EndGroup) + .tag(1, WireType.EndGroup) + .finish(), + ); + const [fieldNo, wireType] = reader.tag(); + expect(fieldNo).toBe(1); + expect(wireType).toBe(WireType.StartGroup); + reader.skip(WireType.StartGroup, 1); + expect(reader.pos).toBe(reader.len); + }); + it("should error on unexpected end group field number", () => { + const reader = new BinaryReader( + new BinaryWriter() + .tag(1, WireType.StartGroup) + .tag(2, WireType.EndGroup) + .finish(), + ); + const [fieldNo, wireType] = reader.tag(); + expect(fieldNo).toBe(1); + expect(wireType).toBe(WireType.StartGroup); + expect(() => { + reader.skip(WireType.StartGroup, 1); + }).toThrow(/^invalid end group tag$/); + }); + it("should return skipped group data", () => { + const reader = new BinaryReader( + new BinaryWriter() + .tag(1, WireType.StartGroup) + .tag(33, WireType.Varint) + .bool(true) + .tag(1, WireType.EndGroup) + .finish(), + ); + reader.tag(); + const skipped = reader.skip(WireType.StartGroup, 1); + const sr = new BinaryReader(skipped); + { + const [fieldNo, wireType] = sr.tag(); + expect(fieldNo).toBe(33); + expect(wireType).toBe(WireType.Varint); + const bool = sr.bool(); + expect(bool).toBe(true); + } + { + const [fieldNo, wireType] = sr.tag(); + expect(fieldNo).toBe(1); + expect(wireType).toBe(WireType.EndGroup); + expect(sr.pos).toBe(sr.len); + } + }); + }); +}); diff --git a/packages/protobuf/src/extensions.ts b/packages/protobuf/src/extensions.ts index 81faaa97a..786efc7dc 100644 --- a/packages/protobuf/src/extensions.ts +++ b/packages/protobuf/src/extensions.ts @@ -80,7 +80,7 @@ export function setExtension( const reader = new BinaryReader(writer.finish()); while (reader.pos < reader.len) { const [no, wireType] = reader.tag(); - const data = reader.skip(wireType); + const data = reader.skip(wireType, no); ufs.push({ no, wireType, data }); } message.$unknown = ufs; diff --git a/packages/protobuf/src/from-binary.ts b/packages/protobuf/src/from-binary.ts index 8cc4aad52..d0371c7dc 100644 --- a/packages/protobuf/src/from-binary.ts +++ b/packages/protobuf/src/from-binary.ts @@ -108,12 +108,12 @@ function readMessage( const unknownFields = message.getUnknown() ?? []; while (reader.pos < end) { [fieldNo, wireType] = reader.tag(); - if (wireType == WireType.EndGroup) { + if (delimited && wireType == WireType.EndGroup) { break; } const field = message.findNumber(fieldNo); if (!field) { - const data = reader.skip(wireType); + const data = reader.skip(wireType, fieldNo); if (options.readUnknownFields) { unknownFields.push({ no: fieldNo, wireType, data }); } diff --git a/packages/protobuf/src/wire/binary-encoding.ts b/packages/protobuf/src/wire/binary-encoding.ts index db86f50e5..6b9825963 100644 --- a/packages/protobuf/src/wire/binary-encoding.ts +++ b/packages/protobuf/src/wire/binary-encoding.ts @@ -374,10 +374,12 @@ export class BinaryReader { } /** - * Skip one element on the wire and return the skipped data. - * Supports WireType.StartGroup since v2.0.0-alpha.23. + * Skip one element and return the skipped data. + * + * When skipping StartGroup, provide the tags field number to check for + * matching field number in the EndGroup tag. */ - skip(wireType: WireType): Uint8Array { + skip(wireType: WireType, fieldNo?: number): Uint8Array { let start = this.pos; switch (wireType) { case WireType.Varint: @@ -398,10 +400,15 @@ export class BinaryReader { this.pos += len; break; case WireType.StartGroup: - // TODO check for matching field numbers in StartGroup / EndGroup tags - let t: WireType; - while ((t = this.tag()[1]) !== WireType.EndGroup) { - this.skip(t); + for (;;) { + const [fn, wt] = this.tag(); + if (wt === WireType.EndGroup) { + if (fieldNo !== undefined && fn !== fieldNo) { + throw new Error("invalid end group tag"); + } + break; + } + this.skip(wt, fn); } break; default: