Skip to content

extension: reject invalid field numbers - #674

Merged
iainmcgin merged 5 commits into
anthropics:mainfrom
fallintoplace:fix/extension-field-number-validation
Oct 12, 2026
Merged

iainmcgin merged 5 commits into
anthropics:mainfrom
fallintoplace:fix/extension-field-number-validation

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What

  • Reject invalid extension field numbers.

Why

  • Invalid numbers can produce tags Buffa cannot decode.

Implementation

  • Validate numbers in both extension constructors.
  • Cover the range limits and reserved band.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

An extension of a MessageSet can be numbered above 2^29 - 1, and
Extension does not know its extendee's wire format, so the constructors
assert only that the number is nonzero and at most i32::MAX. The
reserved band 19000 to 19999 is a schema rule that DescriptorPool
applies; a number in it encodes and decodes like any other.

The changelog entry moves to Changed: the constructors gain a panic.
Field number 0 was documented as accepted and inert, so a hand-written
const with 0 compiled; the changelog entry is now a breaking change and
says so. The constructors and the check carry track_caller, the check
has one assertion per rule, and a compile_fail doctest shows the const
case. The MessageSet type-ID ceiling gets one name that the item
decoder and the constructors share.

The module Panics section is the one list of accessors that check the
extendee; the other docs and the guide refer to it.
The bound applies to every call; MessageSet extensions are the reason
for it, not its scope.
@iainmcgin

Copy link
Copy Markdown
Collaborator

[claude code] Thanks for adding the check. Maintainer commits on top, after a merge of main:

  • The constructors assert only that the number is nonzero and at most i32::MAX. An extension of a message with option message_set_wire_format = true can be numbered above 2^29 - 1 (protoc validates those ranges against the int32 maximum), buffa decodes such items, and Extension does not know the wire format of its extendee, so MAX_FIELD_NUMBER cannot be the ceiling here. The ceiling shares one constant with the MessageSet item decoder.
  • The reserved band 19000 to 19999 is no longer rejected. That is a schema rule, which DescriptorPool applies when it loads a descriptor set; a number in the band encodes and decodes like any other.
  • The constructors carry #[track_caller], each rule has its own panic message, and Extension::new has a compile_fail doctest for a const with number 0.
  • The changelog entry is now under Breaking changes: the docs used to say that number 0 was accepted and inert, so a hand-written const with 0 compiled and now does not.
  • The module # Panics section is the one list of accessors that check the extendee; the other docs and the guide refer to it, and the guide mentions the constructor panic.

@iainmcgin
iainmcgin enabled auto-merge October 12, 2026 20:19
@iainmcgin
iainmcgin added this pull request to the merge queue Oct 12, 2026
Merged via the queue into anthropics:main with commit 814fa87 Oct 12, 2026
11 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 12, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants