Skip to content

reflect: require slash-delimited Any type URLs - #524

Closed
fallintoplace wants to merge 1 commit into
anthropics:mainfrom
fallintoplace:fix/reflective-any-url-validation
Closed

fallintoplace wants to merge 1 commit into
anthropics:mainfrom
fallintoplace:fix/reflective-any-url-validation

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What

  • Reject reflective Any type URLs without a slash.

Why

  • Bare type names were accepted, unlike typed Any helpers.

Implementation

  • Share URL resolution between unpacking and JSON.
  • Keep custom prefixes and existing errors.

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

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

@iainmcgin

Copy link
Copy Markdown
Collaborator

[claude code] Thank you for this PR: it is what showed that buffa's two JSON parsers disagreed about an Any @type with no /. DynamicMessage resolved a bare full name and the generated Any parser rejected it.

We settled the disagreement in the other direction, in #692. protobuf-go and Python read a bare name, and C++ and Java reject it, so #692 makes both parsers accept a bare name that resolves to a known message, and adds an option on each for the behaviour this PR implements:

  • DynamicMessageSeed::strict_any_type_urls(true) rejects an @type without a / for a DynamicMessage.
  • JsonParseOptions::strict_any_type_urls(true) does the same for generated messages.

The other inputs this PR tests are errors with the option on or off: an @type that is empty or ends in /, and a bare name that does not resolve. That last one is what the conformance test AnyWktRepresentationWithBadType requires.

#692 supersedes this PR, so it will be closed. If the strict option is missing a case you had in mind, a comment on #692 is the best place for it.

@iainmcgin iainmcgin closed this Oct 10, 2026
Luccacvb pushed a commit to Luccacvb/buffa that referenced this pull request Oct 10, 2026
…#692)

A `google.protobuf.Any` whose `type_url` has no `/` was resolved by
`DynamicMessage` and rejected by the generated `Any` JSON parser.
`any.proto` requires the `/`; protobuf-go and Python read a bare full
name anyway, and C++ and Java reject it.

- The generated `Any` JSON parser accepts `{"@type": "pkg.Message",
...}` when the type registry has a JSON entry for `pkg.Message`. Such an
`Any` is now written expanded; it was written as base64 under `value`,
which the same parser rejected. A payload that does not decode as that
message now fails to serialize.
- A bare name that does not resolve is a parse error and never takes the
base64 form. The conformance test `AnyWktRepresentationWithBadType`
requires that. The serializer still writes the base64 form for such an
`Any`, so it does not round-trip.
- `AnyRegistry::lookup` and `TypeRegistry::json_any_by_url` find an
entry by a bare name, and an entry registered under a bare name is found
under any URL prefix.
- `Any::type_name`, `is_message` and `unpack_message` (unreleased, anthropics#499)
read a URL without a `/` as the name.
- `JsonParseOptions::strict_any_type_urls` and
`DynamicMessageSeed::strict_any_type_urls` reject an `@type` without a
`/`, at any depth of `Any` in `Any`. The reflective seeds pass both
parse options down in one `ParseFlags` value, which is most of the
change in `reflect/json.rs`.

```rust
let opts = JsonParseOptions::new().strict_any_type_urls(true);
let msg = with_json_parse_options(&opts, || serde_json::from_str::<MyMsg>(json))?;
```

Serialization, `DynamicMessage::unpack_any` and `Any::type_name` take no
options and resolve a bare name. Text format does not resolve a bare
name, because `[pkg.Message] { ... }` is the syntax of an extension
field.

The error for an empty `@type`, or one ending in `/`, now reads `@type
"..." is not a valid type URL: it names no type`.

The changelog entry is `Changed`. anthropics#485, which changed the same output
for a URL with an unregistered prefix, is filed under `Breaking
changes`; 0.9.2 could not parse a bare `@type` at all, so less depends
on the old output here.

Supersedes anthropics#524, which made the reflective path reject a bare name.
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.

2 participants