From 15a843bdc73685349a4d47f7d4134bf0c7085612 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Ichikawa <30754019+ichizero@users.noreply.github.com> Date: Thu, 26 Jan 2023 14:03:40 +0900 Subject: [PATCH] Match Content-Type charset case-insensitively (#440) The connect handler returns http.StatusUnsupportedMediaType when a request has a `Content-Type: application/json; charset=UTF-8` header. However, according to [RFC 9110 Section 8.3.2]( https://httpwg.org/specs/rfc9110.html#rfc.section.8.3.2), the charset parameter value should be treated as case-insensitive. In this PR, I have modified the charset parameter value for user requests and acceptable content types of handlers to all be handled in lowercase. --- handler_ext_test.go | 2 +- protocol.go | 10 ++++++++++ protocol_connect.go | 4 ++-- protocol_grpc.go | 2 +- protocol_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 protocol_test.go diff --git a/handler_ext_test.go b/handler_ext_test.go index fdc56ae3..1e3d828c 100644 --- a/handler_ext_test.go +++ b/handler_ext_test.go @@ -95,7 +95,7 @@ func TestHandler_ServeHTTP(t *testing.T) { strings.NewReader("{}"), ) assert.Nil(t, err) - req.Header.Set("Content-Type", "application/json;Charset=utf-8") + req.Header.Set("Content-Type", "application/json;Charset=Utf-8") resp, err := client.Do(req) assert.Nil(t, err) defer resp.Body.Close() diff --git a/protocol.go b/protocol.go index 918ad2c4..5fefd790 100644 --- a/protocol.go +++ b/protocol.go @@ -320,5 +320,15 @@ func canonicalizeContentType(ct string) string { if err != nil { return ct } + + // According to RFC 9110 Section 8.3.2, the charset parameter value should be treated as case-insensitive. + // mime.FormatMediaType canonicalizes parameter names, but not parameter values, + // because the case sensitivity of a parameter value depends on its semantics. + // Therefore, the charset parameter value should be canonicalized here. + // ref.) https://httpwg.org/specs/rfc9110.html#rfc.section.8.3.2 + if charset, ok := params["charset"]; ok { + params["charset"] = strings.ToLower(charset) + } + return mime.FormatMediaType(base, params) } diff --git a/protocol_connect.go b/protocol_connect.go index c6a3582e..db5ff27b 100644 --- a/protocol_connect.go +++ b/protocol_connect.go @@ -56,10 +56,10 @@ func (*protocolConnect) NewHandler(params *protocolHandlerParams) protocolHandle contentTypes := make(map[string]struct{}) for _, name := range params.Codecs.Names() { if params.Spec.StreamType == StreamTypeUnary { - contentTypes[connectUnaryContentTypePrefix+name] = struct{}{} + contentTypes[canonicalizeContentType(connectUnaryContentTypePrefix+name)] = struct{}{} continue } - contentTypes[connectStreamingContentTypePrefix+name] = struct{}{} + contentTypes[canonicalizeContentType(connectStreamingContentTypePrefix+name)] = struct{}{} } return &connectHandler{ protocolHandlerParams: *params, diff --git a/protocol_grpc.go b/protocol_grpc.go index 8af62309..5ef09c1e 100644 --- a/protocol_grpc.go +++ b/protocol_grpc.go @@ -85,7 +85,7 @@ func (g *protocolGRPC) NewHandler(params *protocolHandlerParams) protocolHandler } contentTypes := make(map[string]struct{}) for _, name := range params.Codecs.Names() { - contentTypes[prefix+name] = struct{}{} + contentTypes[canonicalizeContentType(prefix+name)] = struct{}{} } if params.Codecs.Get(codecNameProto) != nil { contentTypes[bare] = struct{}{} diff --git a/protocol_test.go b/protocol_test.go new file mode 100644 index 00000000..ab1c8115 --- /dev/null +++ b/protocol_test.go @@ -0,0 +1,40 @@ +// Copyright 2021-2023 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package connect + +import ( + "testing" + + "github.com/bufbuild/connect-go/internal/assert" +) + +func TestCanonicalizeContentType(t *testing.T) { + t.Parallel() + tests := []struct { + name string + arg string + want string + }{ + {name: "charset param should be treated as lowercase", arg: "application/json; charset=UTF-8", want: "application/json; charset=utf-8"}, + {name: "non charset param should not be changed", arg: "multipart/form-data; boundary=fooBar", want: "multipart/form-data; boundary=fooBar"}, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, canonicalizeContentType(tt.arg), tt.want) + }) + } +}