Skip to content

Commit 40d6adb

Browse files
authored
transport: Make error-handling for bad HTTP method consistent between HTTP/2 server transport and handler server transport (#6989)
1 parent 3c2a44d commit 40d6adb

File tree

2 files changed

+20
-17
lines changed

2 files changed

+20
-17
lines changed

internal/transport/handler_server.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,10 @@ import (
5151
// inside an http.Handler, or writes an HTTP error to w and returns an error.
5252
// It requires that the http Server supports HTTP/2.
5353
func NewServerHandlerTransport(w http.ResponseWriter, r *http.Request, stats []stats.Handler) (ServerTransport, error) {
54-
if r.ProtoMajor != 2 {
55-
msg := "gRPC requires HTTP/2"
56-
http.Error(w, msg, http.StatusBadRequest)
57-
return nil, errors.New(msg)
58-
}
59-
if r.Method != "POST" {
54+
if r.Method != http.MethodPost {
55+
w.Header().Set("Allow", http.MethodPost)
6056
msg := fmt.Sprintf("invalid gRPC request method %q", r.Method)
61-
http.Error(w, msg, http.StatusBadRequest)
57+
http.Error(w, msg, http.StatusMethodNotAllowed)
6258
return nil, errors.New(msg)
6359
}
6460
contentType := r.Header.Get("Content-Type")
@@ -69,6 +65,11 @@ func NewServerHandlerTransport(w http.ResponseWriter, r *http.Request, stats []s
6965
http.Error(w, msg, http.StatusUnsupportedMediaType)
7066
return nil, errors.New(msg)
7167
}
68+
if r.ProtoMajor != 2 {
69+
msg := "gRPC requires HTTP/2"
70+
http.Error(w, msg, http.StatusHTTPVersionNotSupported)
71+
return nil, errors.New(msg)
72+
}
7273
if _, ok := w.(http.Flusher); !ok {
7374
msg := "gRPC requires a ResponseWriter supporting http.Flusher"
7475
http.Error(w, msg, http.StatusInternalServerError)

internal/transport/handler_server_test.go

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,6 @@ func (s) TestHandlerTransport_NewServerHandlerTransport(t *testing.T) {
5050
check func(*serverHandlerTransport, *testCase) error
5151
}
5252
tests := []testCase{
53-
{
54-
name: "http/1.1",
55-
req: &http.Request{
56-
ProtoMajor: 1,
57-
ProtoMinor: 1,
58-
},
59-
wantErr: "gRPC requires HTTP/2",
60-
wantErrCode: http.StatusBadRequest,
61-
},
6253
{
6354
name: "bad method",
6455
req: &http.Request{
@@ -67,7 +58,7 @@ func (s) TestHandlerTransport_NewServerHandlerTransport(t *testing.T) {
6758
Header: http.Header{},
6859
},
6960
wantErr: `invalid gRPC request method "GET"`,
70-
wantErrCode: http.StatusBadRequest,
61+
wantErrCode: http.StatusMethodNotAllowed,
7162
},
7263
{
7364
name: "bad content type",
@@ -81,6 +72,17 @@ func (s) TestHandlerTransport_NewServerHandlerTransport(t *testing.T) {
8172
wantErr: `invalid gRPC request content-type "application/foo"`,
8273
wantErrCode: http.StatusUnsupportedMediaType,
8374
},
75+
{
76+
name: "http/1.1",
77+
req: &http.Request{
78+
ProtoMajor: 1,
79+
ProtoMinor: 1,
80+
Method: "POST",
81+
Header: http.Header{"Content-Type": []string{"application/grpc"}},
82+
},
83+
wantErr: "gRPC requires HTTP/2",
84+
wantErrCode: http.StatusHTTPVersionNotSupported,
85+
},
8486
{
8587
name: "not flusher",
8688
req: &http.Request{

0 commit comments

Comments
 (0)