protoc-gen-openapiv2: Fix message recursion detection (v2)

go.sum

@@ -70,6 +70,7 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=
 golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
+golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 h1:5Beo0mZN8dRzgrMMkDp0jc8YXQKx9DiJ2k1dkvGsn5A=
 golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

protoc-gen-openapiv2/internal/genopenapi/template.go

@@ -131,7 +131,7 @@ func queryParams(message *descriptor.Message, field *descriptor.Field, prefix st
 //      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 []openapiParameterObject, err error) {
+func nestedQueryParams(message *descriptor.Message, field *descriptor.Field, prefix string, reg *descriptor.Registry, pathParams []descriptor.Parameter, touchedIn map[string]bool) (params []openapiParameterObject, err error) {
 	// make sure the parameter is not already listed as a path parameter
 	for _, pathParam := range pathParams {
 		if pathParam.Target == field {
@@ -226,13 +226,21 @@ func nestedQueryParams(message *descriptor.Message, field *descriptor.Field, pre
 	if err != nil {
 		return nil, fmt.Errorf("unknown message type %s", fieldType)
 	}
+
 	// Check for cyclical message reference:
-	isCycle := touched[*msg.Name]
+	isCycle := touchedIn[*msg.Name]
 	if isCycle {
 		return nil, fmt.Errorf("recursive types are not allowed for query parameters, cycle found on %q", fieldType)
 	}
-	// Update map with the massage name so a cycle further down the recursive path can be detected.
-	touched[*msg.Name] = true
+
+	// Construct a new map with the message name so a cycle further down the recursive path can be detected.
+	// Do not keep anything in the original touched reference and do not pass that reference along.  This will
+	// prevent clobbering adjacent records while recursing.
+	touchedOut := make(map[string]bool)
+	for k, v := range touchedIn {
+		touchedOut[k] = v
+	}
+	touchedOut[*msg.Name] = true
 
 	for _, nestedField := range msg.Fields {
 		var fieldName string
@@ -241,7 +249,7 @@ func nestedQueryParams(message *descriptor.Message, field *descriptor.Field, pre
 		} else {
 			fieldName = field.GetName()
 		}
-		p, err := nestedQueryParams(msg, nestedField, prefix+fieldName+".", reg, pathParams, touched)
+		p, err := nestedQueryParams(msg, nestedField, prefix+fieldName+".", reg, pathParams, touchedOut)
 		if err != nil {
 			return nil, err
 		}

protoc-gen-openapiv2/internal/genopenapi/template_test.go

@@ -411,6 +411,119 @@ func TestMessageToQueryParameters(t *testing.T) {
 	}
 }
 
+// TestMessagetoQueryParametersNoRecursive, is a check that cyclical references between messages
+//  are not falsely detected given previous known edge-cases.
+func TestMessageToQueryParametersNoRecursive(t *testing.T) {
+	type test struct {
+		MsgDescs []*descriptorpb.DescriptorProto
+		Message  string
+	}
+
+	tests := []test{
+		// First test:
+		// Here is a message that has two of another message adjacent to one another in a nested message.
+		// There is no loop but this was previouly falsely flagged as a cycle.
+		// Example proto:
+		// message NonRecursiveMessage {
+		//      string field = 1;
+		// }
+		// message BaseMessage {
+		//      NonRecursiveMessage first = 1;
+		//      NonRecursiveMessage second = 2;
+		// }
+		// message QueryMessage {
+		//      BaseMessage first = 1;
+		//      string second = 2;
+		// }
+		{
+			MsgDescs: []*descriptorpb.DescriptorProto{
+				&descriptorpb.DescriptorProto{
+					Name: proto.String("QueryMessage"),
+					Field: []*descriptorpb.FieldDescriptorProto{
+						{
+							Name:     proto.String("first"),
+							Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
+							TypeName: proto.String(".example.BaseMessage"),
+							Number:   proto.Int32(1),
+						},
+						{
+							Name:   proto.String("second"),
+							Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
+							Number: proto.Int32(2),
+						},
+					},
+				},
+				&descriptorpb.DescriptorProto{
+					Name: proto.String("BaseMessage"),
+					Field: []*descriptorpb.FieldDescriptorProto{
+						{
+							Name:     proto.String("first"),
+							Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
+							TypeName: proto.String(".example.NonRecursiveMessage"),
+							Number:   proto.Int32(1),
+						},
+						{
+							Name:     proto.String("second"),
+							Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
+							TypeName: proto.String(".example.NonRecursiveMessage"),
+							Number:   proto.Int32(2),
+						},
+					},
+				},
+				// Note there is no recursive nature to this message
+				&descriptorpb.DescriptorProto{
+					Name: proto.String("NonRecursiveMessage"),
+					Field: []*descriptorpb.FieldDescriptorProto{
+						{
+							Name: proto.String("field"),
+							//Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
+							Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
+							Number: proto.Int32(1),
+						},
+					},
+				},
+			},
+			Message: "QueryMessage",
+		},
+	}
+
+	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: &descriptorpb.FileDescriptorProto{
+				SourceCodeInfo: &descriptorpb.SourceCodeInfo{},
+				Name:           proto.String("example.proto"),
+				Package:        proto.String("example"),
+				Dependency:     []string{},
+				MessageType:    test.MsgDescs,
+				Service:        []*descriptorpb.ServiceDescriptorProto{},
+			},
+			GoPkg: descriptor.GoPackage{
+				Path: "example.com/path/to/example/example.pb",
+				Name: "example_pb",
+			},
+			Messages: msgs,
+		}
+		reg.Load(&pluginpb.CodeGeneratorRequest{
+			ProtoFile: []*descriptorpb.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("No recursion error should be thrown: %s", err)
+		}
+	}
+}
+
 // 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.