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.

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

Added check for POST method in http2_server #4241

Merged
merged 2 commits into from
Mar 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions internal/transport/http2_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"io"
"math"
"net"
"net/http"
"strconv"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -402,6 +403,20 @@ func (t *http2Server) operateHeaders(frame *http2.MetaHeadersFrame, handle func(
return true
}
t.maxStreamID = streamID
if state.data.httpMethod != http.MethodPost {
zasweq marked this conversation as resolved.
Show resolved Hide resolved
t.mu.Unlock()
if logger.V(logLevel) {
logger.Warningf("transport: http2Server.operateHeaders parsed a :method field: %v which should be POST", state.data.httpMethod)
}
t.controlBuf.put(&cleanupStream{
streamID: streamID,
rst: true,
rstCode: http2.ErrCodeProtocol,
onWrite: func() {},
})
s.cancel()
return false
}
t.activeStreams[streamID] = s
if len(t.activeStreams) == 1 {
t.idle = time.Time{}
Expand Down
3 changes: 3 additions & 0 deletions internal/transport/http_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ type parsedHeaderData struct {
timeoutSet bool
timeout time.Duration
method string
httpMethod string
// key-value metadata map from the peer.
mdata map[string][]string
statsTags []byte
Expand Down Expand Up @@ -363,6 +364,8 @@ func (d *decodeState) processHeaderField(f hpack.HeaderField) {
}
d.data.statsTrace = v
d.addMetadata(f.Name, string(v))
case ":method":
d.data.httpMethod = f.Value
default:
if isReservedHeader(f.Name) && !isWhitelistedHeader(f.Name) {
break
Expand Down
78 changes: 78 additions & 0 deletions internal/transport/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1663,6 +1663,84 @@ func (s) TestReadGivesSameErrorAfterAnyErrorOccurs(t *testing.T) {
}
}

// If the client sends an HTTP/2 request with a :method header with a value other than POST, as specified in
// the gRPC over HTTP/2 specification, the server should close the stream.
func (s) TestServerWithClientSendingWrongMethod(t *testing.T) {
zasweq marked this conversation as resolved.
Show resolved Hide resolved
server := setUpServerOnly(t, 0, &ServerConfig{}, suspended)
defer server.stop()
// Create a client directly to not couple what you can send to API of http2_client.go.
zasweq marked this conversation as resolved.
Show resolved Hide resolved
mconn, err := net.Dial("tcp", server.lis.Addr().String())
if err != nil {
t.Fatalf("Client failed to dial: %v", err)
}
defer mconn.Close()

if n, err := mconn.Write(clientPreface); err != nil || n != len(clientPreface) {
t.Fatalf("mconn.Write(clientPreface) = %d, %v, want %d, <nil>", n, err, len(clientPreface))
}

framer := http2.NewFramer(mconn, mconn)
if err := framer.WriteSettings(); err != nil {
t.Fatalf("Error while writing settings: %v", err)
}

// success chan indicates that reader received a RSTStream from server.
// An error will be passed on it if any other frame is received.
success := testutils.NewChannel()

// Launch a reader goroutine.
go func() {
for {
frame, err := framer.ReadFrame()
if err != nil {
return
}
switch frame := frame.(type) {
case *http2.SettingsFrame:
// Do nothing. A settings frame is expected from server preface.
case *http2.RSTStreamFrame:
if frame.Header().StreamID != 1 || http2.ErrCode(frame.ErrCode) != http2.ErrCodeProtocol {
// Client only created a single stream, so RST Stream should be for that single stream.
t.Errorf("RST stream received with streamID: %d and code %v, want streamID: 1 and code: http.ErrCodeFlowControl", frame.Header().StreamID, http2.ErrCode(frame.ErrCode))
}
// Records that client successfully received RST Stream frame.
success.Send(nil)
return
default:
// The server should send nothing but a single RST Stream frame.
success.Send(errors.New("The client received a frame other than RST Stream"))
}
}
}()

// Done with HTTP/2 setup - now create a stream with a bad method header.
var buf bytes.Buffer
henc := hpack.NewEncoder(&buf)
// Method is required to be POST in a gRPC call.
if err := henc.WriteField(hpack.HeaderField{Name: ":method", Value: "PUT"}); err != nil {
t.Fatalf("Error while encoding header: %v", err)
}
// Have the rest of the headers be ok and within the gRPC over HTTP/2 spec.
if err := henc.WriteField(hpack.HeaderField{Name: ":path", Value: "foo"}); err != nil {
t.Fatalf("Error while encoding header: %v", err)
}
if err := henc.WriteField(hpack.HeaderField{Name: ":authority", Value: "localhost"}); err != nil {
t.Fatalf("Error while encoding header: %v", err)
}
if err := henc.WriteField(hpack.HeaderField{Name: "content-type", Value: "application/grpc"}); err != nil {
t.Fatalf("Error while encoding header: %v", err)
}

if err := framer.WriteHeaders(http2.HeadersFrameParam{StreamID: 1, BlockFragment: buf.Bytes(), EndHeaders: true}); err != nil {
t.Fatalf("Error while writing headers: %v", err)
}
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
if e, err := success.Receive(ctx); e != nil || err != nil {
t.Fatalf("Error in frame server should send: %v. Error receiving from channel: %v", e, err)
}
}

func (s) TestPingPong1B(t *testing.T) {
runPingPongTest(t, 1)
}
Expand Down