Skip to content

fix(protoc-gen-openapiv2): prevent panic when generating OpenAPI for multiple files#6275

Merged
johanbrandhorst merged 2 commits intogrpc-ecosystem:mainfrom
franchb:main
Jan 28, 2026
Merged

fix(protoc-gen-openapiv2): prevent panic when generating OpenAPI for multiple files#6275
johanbrandhorst merged 2 commits intogrpc-ecosystem:mainfrom
franchb:main

Conversation

@franchb
Copy link
Copy Markdown
Contributor

@franchb franchb commented Jan 25, 2026

Summary

Fixes #6274

When multiple proto files are specified as targets (e.g., via buf or protoc), the plugin would panic with "failed to resolve method FQN" because the naming cache retained a filtered mapping from a previous file that excluded method FQNs needed by the current file.

Changes

  • Clear the naming cache at the start of each file's applyTemplate() so each file builds a fresh mapping with all needed names
  • Add a fallback for method FQN lookup that uses FQN naming strategy instead of panicking if the lookup fails
  • Add test cases for the merge scenario with body:"*" and path parameters

Root Cause

The registriesSeen cache in template.go uses *descriptor.Registry as the key. When processing multiple target files, each file's applyTemplate() overwrites the cache with its own filtered subset of names (to avoid naming collisions with unused types).

When messages.proto (with no services) is processed first:

  1. The filtered mapping excludes method FQNs (since services list is empty)
  2. This mapping is cached in registriesSeen[p.reg]
  3. When service.proto is processed, lookups in renderServices() use the cached mapping
  4. Method FQN not found → panic

Test plan

  • Added TestGenerateMergeFilesWithBodyAndPathParams - tests merged files with body:"*" and path parameters
  • Added TestGenerateMergeWithServiceNotInTargets - tests generating service file with messages in dependency
  • All existing tests pass

🤖 Generated with Claude Code

…multiple files

When multiple proto files are specified as targets, the naming cache
(registriesSeen) would retain a filtered mapping from a previous file
that excluded method FQNs needed by the current file, causing a panic
with "failed to resolve method FQN".

This fix:
- Clears the naming cache at the start of each file's applyTemplate()
  so each file builds a fresh mapping with all needed names
- Adds a fallback for method FQN lookup that uses FQN naming strategy
  instead of panicking if the lookup fails

Fixes grpc-ecosystem#6274

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR, I had a question about one of the comments.


// TestGenerateMergeWithServiceNotInTargets tests the scenario where a service
// file is available in the proto file set but not marked for generation.
// This can happen with Edition 2023 files where dependencies are structured differently.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find a reference for this assertion, could you explain what you mean? If this part is AI generated, please back it up with sources or remove it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right to call this out. This test was speculative coverage I added while working on the fix, but I can't point to a specific issue it prevents. I'll remove it to keep the PR focused on the actual fix for #6274.

Copy link
Copy Markdown
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@johanbrandhorst johanbrandhorst enabled auto-merge (squash) January 28, 2026 17:23
@johanbrandhorst johanbrandhorst merged commit bba4e0a into grpc-ecosystem:main Jan 28, 2026
14 checks passed
franchb added a commit to franchb/grpc-gateway that referenced this pull request Feb 3, 2026
Properly fixes grpc-ecosystem#6274 (panic) without causing grpc-ecosystem#6308 (naming mismatch).
Replaces the reverted approach from PR grpc-ecosystem#6275.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
johanbrandhorst pushed a commit that referenced this pull request Feb 6, 2026
…#6315)

Properly fixes #6274 (panic) without causing #6308 (naming mismatch).
Replaces the reverted approach from PR #6275.

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
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.

protoc-gen-openapiv2 panics when generating OpenAPI for services with messages in separate files (Protobuf Edition 2023)

2 participants