Merge pull request #50603 from shiywang/loop

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Fix Content negotiation incorrect when Accept header uses type parame…

Fixes https://github.com/kubernetes/kubernetes/issues/50519
@smarterclayton @liggitt still wip,  I'll add some unit test soon, and simplify the logic

Kubernetes-commit: ee13444144e046a13824b5a92aa11fc16447b0c1

Godeps/Godeps.json

@@ -764,119 +764,119 @@
 		},
 		{
 			"ImportPath": "k8s.io/api/admission/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/admissionregistration/v1alpha1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/admissionregistration/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/apps/v1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/apps/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/apps/v1beta2",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/authentication/v1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/authentication/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/authorization/v1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/authorization/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/autoscaling/v1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/autoscaling/v2beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/batch/v1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/batch/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/batch/v2alpha1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/certificates/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/core/v1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/events/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/extensions/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/networking/v1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/policy/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/rbac/v1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/rbac/v1alpha1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/rbac/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/scheduling/v1alpha1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/settings/v1alpha1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/storage/v1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/storage/v1alpha1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/api/storage/v1beta1",
-			"Rev": "168091b7c0f030ab2b7a9fc734598880a116c6b8"
+			"Rev": "5cb170de1f060ebf2dde62cc4b8241fdb3c01203"
 		},
 		{
 			"ImportPath": "k8s.io/apimachinery/pkg/api/equality",

pkg/endpoints/handlers/negotiation/negotiate.go

@@ -273,6 +273,13 @@ func acceptMediaTypeOptions(params map[string]string, accepts *AcceptedMediaType
 	return options, true
 }
 
+type candidateMediaType struct {
+	accepted *AcceptedMediaType
+	clauses  goautoneg.Accept
+}
+
+type candidateMediaTypeSlice []candidateMediaType
+
 // NegotiateMediaTypeOptions returns the most appropriate content type given the accept header and
 // a list of alternatives along with the accepted media type parameters.
 func NegotiateMediaTypeOptions(header string, accepted []AcceptedMediaType, endpoint EndpointRestrictions) (MediaTypeOptions, bool) {
@@ -282,6 +289,7 @@ func NegotiateMediaTypeOptions(header string, accepted []AcceptedMediaType, endp
 		}, true
 	}
 
+	var candidates candidateMediaTypeSlice
 	clauses := goautoneg.ParseAccept(header)
 	for _, clause := range clauses {
 		for i := range accepted {
@@ -290,12 +298,17 @@ func NegotiateMediaTypeOptions(header string, accepted []AcceptedMediaType, endp
 			case clause.Type == accepts.Type && clause.SubType == accepts.SubType,
 				clause.Type == accepts.Type && clause.SubType == "*",
 				clause.Type == "*" && clause.SubType == "*":
-				// TODO: should we prefer the first type with no unrecognized options?  Do we need to ignore unrecognized
+				candidates = append(candidates, candidateMediaType{accepted: accepts, clauses: clause})
-				// parameters.
-				return acceptMediaTypeOptions(clause.Params, accepts, endpoint)
 			}
 		}
 	}
+
+	for _, v := range candidates {
+		if retVal, ret := acceptMediaTypeOptions(v.clauses.Params, v.accepted, endpoint); ret {
+			return retVal, true
+		}
+	}
+
 	return MediaTypeOptions{}, false
 }
 

pkg/endpoints/handlers/negotiation/negotiate_test.go

@@ -181,7 +181,26 @@ func TestNegotiate(t *testing.T) {
 			serializer:  fakeCodec,
 			params:      map[string]string{"pretty": "1"},
 		},
-
+		{
+			req: &http.Request{
+				Header: http.Header{
+					"Accept": []string{"application/json;as=BOGUS;v=v1alpha1;g=meta.k8s.io, application/json"},
+				},
+			},
+			contentType: "application/json",
+			ns:          &fakeNegotiater{serializer: fakeCodec, types: []string{"application/json"}},
+			serializer:  fakeCodec,
+		},
+		{
+			req: &http.Request{
+				Header: http.Header{
+					"Accept": []string{"application/BOGUS, application/json"},
+				},
+			},
+			contentType: "application/json",
+			ns:          &fakeNegotiater{serializer: fakeCodec, types: []string{"application/json"}},
+			serializer:  fakeCodec,
+		},
 		// "application" is not a valid media type, so the server will reject the response during
 		// negotiation (the server, in error, has specified an invalid media type)
 		{