#1719·protobuf

proto: UnmarshalOptions.RecursionLimit is not applied when skipping unknown group fields

Author: iainmcginCreated Aug 19, 2026Updated Aug 25, 2026
Labelstriaged

What version of protobuf and what language are you using?

google.golang.org/protobuf v1.36.12-devel (master @ f4a5402), Go 1.26.

What did you do?

Set proto.UnmarshalOptions{RecursionLimit: 100} and unmarshalled input consisting only of nested unknown group fields.

go
package repro

import (
	"bytes"
	"testing"

	"google.golang.org/protobuf/proto"
	"google.golang.org/protobuf/types/known/emptypb"
)

// nestedUnknownGroups returns depth nested empty groups on field 1:
// SGROUP(1) x depth, EGROUP(1) x depth.
func nestedUnknownGroups(depth int) []byte {
	return append(bytes.Repeat([]byte{0x0B}, depth), bytes.Repeat([]byte{0x0C}, depth)...)
}

func TestRecursionLimitIgnoredForUnknownGroups(t *testing.T) {
	opts := proto.UnmarshalOptions{RecursionLimit: 100}
	for _, depth := range []int{101, 1000, 10001} {
		if err := opts.Unmarshal(nestedUnknownGroups(depth), &emptypb.Empty{}); err == nil {
			t.Errorf("unknown-group depth %d with RecursionLimit=100: got nil error, want recursion-depth error", depth)
		}
	}
}

What did you expect to see?

Each Unmarshal call to fail with "exceeded maximum recursion depth", as 101 levels of known-message nesting does under the same options.

What did you see instead?

unknown-group depth 101 with RecursionLimit=100: got nil error, want recursion-depth error
unknown-group depth 1000 with RecursionLimit=100: got nil error, want recursion-depth error
unknown-group depth 10001 with RecursionLimit=100: got nil error, want recursion-depth error

Unknown-group nesting is only rejected at 10002 levels, regardless of RecursionLimit. The 10000 allowance is also granted afresh inside every known message, so the effective ceiling is RecursionLimit + 10001 rather than RecursionLimit.

When the hard-coded cap does fire, it is off by one and reports the wrong error:

go
b := nestedUnknownGroups(20000)
proto.Unmarshal(b, &emptypb.Empty{})
// -> "proto: cannot parse invalid wire-format data"   (want "proto: exceeded maximum recursion depth")
n := protowire.ConsumeFieldValue(1, protowire.StartGroupType, b[1:])
// n == -6 (errCodeRecursionDepth); protowire.ParseError(n) -> "proto: parse error"
// nestedUnknownGroups(10001) is accepted: consumeFieldValueD starts at depth=10000 and fails on depth < 0, admitting 10001 levels

errCodeRecursionDepth has no case in protowire.ParseError and falls through to errParse, and the unmarshal paths map any negative length from ConsumeFieldValue to errDecode, so a caller cannot distinguish hostile nesting from corrupt input on this path the way they can for known-message nesting.

This is a known but under-documented gap: the CL that introduced RecursionLimit (cl/385854, 3992ea83) notes in its description that "the configured limit does not apply to pure groups", which fall back to the hard-coded 10000 limit. That caveat never reached the UnmarshalOptions.RecursionLimit godoc ("limits how deeply messages may be nested"), so callers setting a low limit are unaware that this does not apply to this specific case.

Since then, cl/728680 (9197dd0a) treated similar behavior for lazily-decoded extensions (a fresh 10k budget granted inside an already-nested message) as a bug and fixed it, and made maps count toward the limit. Unknown/pure groups are the remaining path with the old behavior, and the recursion tests added there don't exercise groups or unknown fields.

Mechanically, every unknown-field skip site calls protowire.ConsumeFieldValue, which starts from protowire.DefaultRecursionLimit instead of the remaining depth carried in the unmarshal options:

  • internal/impl/decode.go (unmarshalPointer, fast path) — opts.depth is decremented per known message but protowire.ConsumeFieldValue(num, wtyp, b) is used for unknowns
  • proto/decode.go (unmarshalMessageSlow and unmarshalMap, reflection slow path) — same, with o.RecursionLimit
  • internal/impl/codec_map.go, internal/impl/lazy.go, internal/impl/validate.go — same pattern

consumeFieldValueD already takes a depth argument; it just isn't reachable from these callers. One possible shape for a fix is an exported protowire.ConsumeFieldValueDepth(num, typ, b, depth) (or an internal equivalent) that the callers above invoke with their remaining depth, returning errRecursionDepth when it reports errCodeRecursionDepth (and giving ParseError a case for that code), so unknown groups draw down the same budget and surface the same error as known messages — consistent with what cl/728680 did for extensions and maps. If that isn't wanted, documenting the caveat on RecursionLimit would at least make the current behaviour discoverable.

Impact: this is not a crash — consumeFieldValueD frames are small and Go stacks grow, so 10001 levels needs ~1 MiB of goroutine stack, far below the runtime's maximum. It is, however, a per-goroutine memory cost that RecursionLimit is presumably being set low to avoid and currently cannot: a 20 KB body of nested unknown groups grows the decoding goroutine's stack to ~0.9 MiB (~45x the input) even with RecursionLimit: 100, and because gRPC and similar servers decode on the per-request handler goroutine, that stack stays allocated for the handler's lifetime after Unmarshal returns, only halving per GC cycle via stack shrinking. With 200 concurrent handlers each fed that 20 KB payload I measured 177 MiB of stack in use after decode, 90 / 47 / 25 MiB after one / two / three forced GCs, and 3 MiB once the handlers returned. A service that has capped message size and lowered RecursionLimit specifically to bound per-request decode memory still carries this ~1 MiB-per-in-flight-request floor from unknown groups alone.