Skip to content

fix(decoder): zero-field messages rejected well-formed unknown groups - #2433

Open
JavaGT wants to merge 1 commit into
protobufjs:masterfrom
JavaGT:fix/decoder-unknown-group-zero-fields
Open

JavaGT wants to merge 1 commit into
protobufjs:masterfrom
JavaGT:fix/decoder-unknown-group-zero-fields

Conversation

@JavaGT

@JavaGT JavaGT commented Sep 13, 2026

Copy link
Copy Markdown

Fixes #2431.

What changed

One line in the generated-decoder codegen, plus a regression test.

Zero-field message types (and editions DELIMITED fields) threw invalid end group tag when decoding bytes containing a well-formed unknown group, instead of skipping it:

const root = protobuf.Root.fromJSON({ nested: {
    T: { fields: {} },                        // zero fields
    U: { fields: { x: { type: "int32", id: 1 } } }
}});
root.lookupType("T").decode([0x0b, 0x0c]); // THREW: invalid end group tag
root.lookupType("U").decode([0x0c]);       // fine

Why

The generated field loop shifts t to the field number via switch(t >>> 3), but that switch only exists when the type has fields. For zero-field types, the unknown-fields call r.skipType(t&7, q, t) passed the full tag (fieldNumber*8 + wireType) as the expected field number, so the end-group check in Reader.prototype.skipType could never match. C++ and other conformant runtimes accept (and are required to accept) unknown groups with matching end tags, so this breaks the "empty message as a marker; schema evolved later" case.

The fix

When the type has no fields, shift the tag in the generated expression: r.skipType(t&7, q, t>>>3). Types with fields are untouched (t is already the field number there). Mismatched end tags are still rejected — the new test covers both.

Verification

  • New tape test (tests/api_decoder_bounds.js): zero-field message accepts a matching end tag, still rejects a mismatched one, and a one-field message behaves unchanged.
  • npm test passes (sources + types) on this branch.

AI transparency

Found during an agent-run improvement audit of a downstream consumer's dependency. Exploration/verification: GPT-5.6 Luna and Grok 4.6 via OpenCode CLI (both independently confirmed the bug by executing the repro). Planning and implementation: GLM (ZCode agent). Verified against master at 0ad9a28.

…eld messages

Zero-field types emit no field loop, so `t` was never shifted to the
field number before the unknown-fields call. The generated decoder then
asked skipType to match the full tag against the end-group field number,
which can never succeed — well-formed unknown groups (and editions
DELIMITED fields) threw "invalid end group tag" instead of being skipped.

Fixes protobufjs#2431.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

decoder: zero-field messages reject well-formed unknown groups ("invalid end group tag")

1 participant