Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
johanbrandhorst merged 1 commit into from
Feb 2, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 0 additions & 132 deletions protoc-gen-openapiv2/internal/genopenapi/generator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2075,135 +2075,3 @@ func TestIssue5684_UnusedMethodsNotInOpenAPI(t *testing.T) {
t.Errorf("expected exactly 1 path, got %d paths", len(paths))
}
}

// TestGenerateMergeFilesWithBodyAndPathParams tests that OpenAPI generation
// doesn't panic when merging files where a service uses body:"*" with path parameters.
// This reproduces the bug from https://github.com/grpc-ecosystem/grpc-gateway/issues/6274
func TestGenerateMergeFilesWithBodyAndPathParams(t *testing.T) {
t.Parallel()

// First proto file: contains only message definitions, with swagger option
// This file will be the merge target since it has the swagger option
const messagesProto = `
proto_file: {
name: "example/v1/messages.proto"
package: "example.v1"
message_type: {
name: "Item"
field: {
name: "id"
number: 1
label: LABEL_OPTIONAL
type: TYPE_STRING
json_name: "id"
}
field: {
name: "name"
number: 2
label: LABEL_OPTIONAL
type: TYPE_STRING
json_name: "name"
}
}
message_type: {
name: "UpdateItemRequest"
field: {
name: "id"
number: 1
label: LABEL_OPTIONAL
type: TYPE_STRING
json_name: "id"
}
field: {
name: "name"
number: 2
label: LABEL_OPTIONAL
type: TYPE_STRING
json_name: "name"
}
}
options: {
go_package: "example/v1;examplev1"
[grpc.gateway.protoc_gen_openapiv2.options.openapiv2_swagger]: {
info: {
title: "Test API"
version: "1.0"
}
}
}
syntax: "proto3"
}`

// Second proto file: contains the service that references messages from first file
// This file does NOT have the swagger option, so it won't be the merge target
const serviceProto = `
proto_file: {
name: "example/v1/service.proto"
package: "example.v1"
dependency: "example/v1/messages.proto"
service: {
name: "ItemService"
method: {
name: "UpdateItem"
input_type: ".example.v1.UpdateItemRequest"
output_type: ".example.v1.Item"
options: {
[google.api.http]: {
put: "/v1/items/{id}"
body: "*"
}
}
}
}
options: {
go_package: "example/v1;examplev1"
}
syntax: "proto3"
}`

var msgReq, svcReq pluginpb.CodeGeneratorRequest
if err := prototext.Unmarshal([]byte(messagesProto), &msgReq); err != nil {
t.Fatalf("failed to unmarshal messages proto: %s", err)
}
if err := prototext.Unmarshal([]byte(serviceProto), &svcReq); err != nil {
t.Fatalf("failed to unmarshal service proto: %s", err)
}

// Combine into a single request with both files to generate
req := &pluginpb.CodeGeneratorRequest{
ProtoFile: append(msgReq.ProtoFile, svcReq.ProtoFile...),
FileToGenerate: []string{"example/v1/messages.proto", "example/v1/service.proto"},
}

formats := [...]genopenapi.Format{
genopenapi.FormatJSON,
genopenapi.FormatYAML,
}

for _, format := range formats {
format := format
t.Run(string(format), func(t *testing.T) {
t.Parallel()

// This should not panic - the bug causes panic with
// "failed to resolve method FQN: '.example.v1.ItemService.UpdateItem'"
resp := requireGenerate(t, req, format, false, true)
if len(resp) != 1 {
t.Fatalf("invalid count, expected: 1, actual: %d", len(resp))
}

content := resp[0].GetContent()
t.Log(content)

// Verify the path exists in output
if !strings.Contains(content, "/v1/items/{id}") {
t.Error("expected /v1/items/{id} path in output")
}

// Verify the body definition was created (this is what triggers the bug)
if !strings.Contains(content, "ItemServiceUpdateItemBody") {
t.Error("expected ItemServiceUpdateItemBody definition in output")
}
})
}
}
18 changes: 1 addition & 17 deletions protoc-gen-openapiv2/internal/genopenapi/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1569,14 +1569,7 @@ func renderServices(services []*descriptor.Service, paths *openapiPathsObject, r
if meth.Name != nil {
methFQN, ok := fullyQualifiedNameToOpenAPIName(meth.FQMN(), reg)
if !ok {
// Fallback: use FQN naming (strip leading dot) if method FQN not found in mapping.
// This can happen when files are merged and method FQNs are not properly registered.
fqmn := meth.FQMN()
if len(fqmn) > 0 && fqmn[0] == '.' {
methFQN = fqmn[1:]
} else {
methFQN = fqmn
}
panic(fmt.Errorf("failed to resolve method FQN: '%s'", meth.FQMN()))
}
defName := methFQN + "Body"
schema.Ref = fmt.Sprintf("#/definitions/%s", defName)
Expand Down Expand Up @@ -2085,15 +2078,6 @@ func operationForMethod(httpMethod string) func(*openapiPathItemObject) *openapi

// This function is called with a param which contains the entire definition of a method.
func applyTemplate(p param) (*openapiSwaggerObject, error) {
// Clear the naming cache for this registry at the start of each file processing.
// This is necessary because when multiple files are processed, the cache may contain
// a filtered mapping from a previous file that doesn't include all names needed
// for the current file (e.g., method FQNs from the current file's services).
// The cache will be rebuilt with the appropriate filtered names later in this function.
registriesSeenMutex.Lock()
delete(registriesSeen, p.reg)
registriesSeenMutex.Unlock()

// Create the basic template object. This is the object that everything is
// defined off of.
s := openapiSwaggerObject{
Expand Down
Loading