Skip to content
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

protoc-gen-swagger, Fix for infinite loop on circular references in query parameters #1266

Merged
merged 3 commits into from
May 4, 2020
Merged

protoc-gen-swagger, Fix for infinite loop on circular references in query parameters #1266

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
02f5169
Added function nestedQueryParams with map[string] parameter for keepi…
Romeren May 4, 2020
ded59b5
Code-review change requests accepted
Romeren May 4, 2020
ea71c3c
More missed circle references changed to cycle
Romeren May 4, 2020
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
22 changes: 20 additions & 2 deletions protoc-gen-swagger/genswagger/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,18 @@ func messageToQueryParameters(message *descriptor.Message, reg *descriptor.Regis
return params, nil
}

// queryParams converts a field to a list of swagger query parameters recursively.
// queryParams converts a field to a list of swagger query parameters recursively through the use of nestedQueryParams.
func queryParams(message *descriptor.Message, field *descriptor.Field, prefix string, reg *descriptor.Registry, pathParams []descriptor.Parameter) (params []swaggerParameterObject, err error) {
return nestedQueryParams(message, field, prefix, reg, pathParams, map[string]bool{})
}

// nestedQueryParams converts a field to a list of swagger query parameters recursively.
// This function is a helper function for queryParams, that keeps track of cyclical message references
// through the use of
// touched map[string]bool
// If a cycle is discovered, an error is returned, as cyclical data structures aren't allowed
// in query parameters.
func nestedQueryParams(message *descriptor.Message, field *descriptor.Field, prefix string, reg *descriptor.Registry, pathParams []descriptor.Parameter, touched map[string]bool) (params []swaggerParameterObject, err error) {
// make sure the parameter is not already listed as a path parameter
for _, pathParam := range pathParams {
if pathParam.Target == field {
Expand Down Expand Up @@ -216,14 +226,22 @@ func queryParams(message *descriptor.Message, field *descriptor.Field, prefix st
if err != nil {
return nil, fmt.Errorf("unknown message type %s", fieldType)
}
// Check for cyclical message reference:
isCycle := touched[*msg.Name]
if isCycle {
return nil, fmt.Errorf("Recursive types are not allowed for query parameters, cycle found on %s", fieldType)
}
// Update map with the massage name so a cycle further down the recursive path can be detected.
touched[*msg.Name] = true

for _, nestedField := range msg.Fields {
var fieldName string
if reg.GetUseJSONNamesForFields() {
fieldName = field.GetJsonName()
} else {
fieldName = field.GetName()
}
p, err := queryParams(msg, nestedField, prefix+fieldName+".", reg, pathParams)
p, err := nestedQueryParams(msg, nestedField, prefix+fieldName+".", reg, pathParams, touched)
if err != nil {
return nil, err
}
Expand Down
118 changes: 118 additions & 0 deletions protoc-gen-swagger/genswagger/template_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,124 @@ func TestMessageToQueryParameters(t *testing.T) {
}
}

// TestMessagetoQueryParametersRecursive, is a check that cyclical references between messages
// are handled gracefully. The goal is to insure that attempts to add messages with cyclical
// references to query-parameters returns an error message.
func TestMessageToQueryParametersRecursive(t *testing.T) {
type test struct {
MsgDescs []*protodescriptor.DescriptorProto
Message string
}

tests := []test{
// First test:
// Here we test that a message that references it self through a field will return an error.
// Example proto:
// message DirectRecursiveMessage {
// DirectRecursiveMessage nested = 1;
// }
{
MsgDescs: []*protodescriptor.DescriptorProto{
&protodescriptor.DescriptorProto{
Name: proto.String("DirectRecursiveMessage"),
Field: []*protodescriptor.FieldDescriptorProto{
{
Name: proto.String("nested"),
Label: protodescriptor.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
Type: protodescriptor.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String(".example.DirectRecursiveMessage"),
Number: proto.Int32(1),
},
},
},
},
Message: "DirectRecursiveMessage",
},
// Second test:
// Here we test that a circle through multiple messages also is detected and that an error is returned.
Romeren marked this conversation as resolved.
Show resolved Hide resolved
// Sample:
// message Root { NodeMessage nested = 1; }
// message NodeMessage { CircleMessage nested = 1; }
// message CircleMessage { Root nested = 1; }
Romeren marked this conversation as resolved.
Show resolved Hide resolved
{
MsgDescs: []*protodescriptor.DescriptorProto{
&protodescriptor.DescriptorProto{
Name: proto.String("RootMessage"),
Field: []*protodescriptor.FieldDescriptorProto{
{
Name: proto.String("nested"),
Label: protodescriptor.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
Type: protodescriptor.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String(".example.NodeMessage"),
Number: proto.Int32(1),
},
},
},
&protodescriptor.DescriptorProto{
Name: proto.String("NodeMessage"),
Field: []*protodescriptor.FieldDescriptorProto{
{
Name: proto.String("nested"),
Label: protodescriptor.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
Type: protodescriptor.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String(".example.CircleMessage"),
Romeren marked this conversation as resolved.
Show resolved Hide resolved
Number: proto.Int32(1),
},
},
},
&protodescriptor.DescriptorProto{
Name: proto.String("CircleMessage"),
Romeren marked this conversation as resolved.
Show resolved Hide resolved
Field: []*protodescriptor.FieldDescriptorProto{
{
Name: proto.String("nested"),
Label: protodescriptor.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
Type: protodescriptor.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String(".example.RootMessage"),
Number: proto.Int32(1),
},
},
},
},
Message: "RootMessage",
},
}

for _, test := range tests {
reg := descriptor.NewRegistry()
msgs := []*descriptor.Message{}
for _, msgdesc := range test.MsgDescs {
msgs = append(msgs, &descriptor.Message{DescriptorProto: msgdesc})
}
file := descriptor.File{
FileDescriptorProto: &protodescriptor.FileDescriptorProto{
SourceCodeInfo: &protodescriptor.SourceCodeInfo{},
Name: proto.String("example.proto"),
Package: proto.String("example"),
Dependency: []string{},
MessageType: test.MsgDescs,
Service: []*protodescriptor.ServiceDescriptorProto{},
},
GoPkg: descriptor.GoPackage{
Path: "example.com/path/to/example/example.pb",
Name: "example_pb",
},
Messages: msgs,
}
reg.Load(&plugin.CodeGeneratorRequest{
ProtoFile: []*protodescriptor.FileDescriptorProto{file.FileDescriptorProto},
})

message, err := reg.LookupMsg("", ".example."+test.Message)
if err != nil {
t.Fatalf("failed to lookup message: %s", err)
}
_, err = messageToQueryParameters(message, reg, []descriptor.Parameter{})
if err == nil {
t.Fatalf("It should not be allowed to have recursive query parameters")
}
}
}

func TestMessageToQueryParametersWithJsonName(t *testing.T) {
type test struct {
MsgDescs []*protodescriptor.DescriptorProto
Expand Down