Skip to content

Commit

Permalink
http2: reject DATA frame before HEADERS frame
Browse files Browse the repository at this point in the history
If the server returns a DATA frame before a HEADERS frame, the client
must close the connection with a PROTOCOL_ERROR as describe in the
section 8.1.2.6.

The current implementation does not reject such sequence and panics
trying to write on the non-initialized bufPipe.

Fixes golang/go#21466

Change-Id: I55755dba259d026529a031e9ff82c915f5e4afae
Reviewed-on: https://go-review.googlesource.com/56770
Reviewed-by: Tom Bergan <[email protected]>
Run-TryBot: Tom Bergan <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
  • Loading branch information
rs authored and tombergan committed Aug 24, 2017
1 parent 1c05540 commit 57efc9c
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 0 deletions.
8 changes: 8 additions & 0 deletions http2/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -1789,6 +1789,14 @@ func (rl *clientConnReadLoop) processData(f *DataFrame) error {
}
return nil
}
if !cs.firstByte {
cc.logf("protocol error: received DATA before a HEADERS frame")
rl.endStreamError(cs, StreamError{
StreamID: f.StreamID,
Code: ErrCodeProtocol,
})
return nil
}
if f.Length > 0 {
// Check connection-level flow control.
cc.mu.Lock()
Expand Down
55 changes: 55 additions & 0 deletions http2/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"math/rand"
"net"
"net/http"
"net/http/httptest"
"net/url"
"os"
"reflect"
Expand Down Expand Up @@ -3036,6 +3037,60 @@ func TestTransportRetryHasLimit(t *testing.T) {
ct.run()
}

func TestTransportResponseDataBeforeHeaders(t *testing.T) {
ct := newClientTester(t)
ct.client = func() error {
defer ct.cc.(*net.TCPConn).CloseWrite()
req := httptest.NewRequest("GET", "https://dummy.tld/", nil)
// First request is normal to ensure the check is per stream and not per connection.
_, err := ct.tr.RoundTrip(req)
if err != nil {
return fmt.Errorf("RoundTrip expected no error, got: %v", err)
}
// Second request returns a DATA frame with no HEADERS.
resp, err := ct.tr.RoundTrip(req)
if err == nil {
return fmt.Errorf("RoundTrip expected error, got response: %+v", resp)
}
if err, ok := err.(StreamError); !ok || err.Code != ErrCodeProtocol {
return fmt.Errorf("expected stream PROTOCOL_ERROR, got: %v", err)
}
return nil
}
ct.server = func() error {
ct.greet()
for {
f, err := ct.fr.ReadFrame()
if err == io.EOF {
return nil
} else if err != nil {
return err
}
switch f := f.(type) {
case *WindowUpdateFrame, *SettingsFrame:
case *HeadersFrame:
switch f.StreamID {
case 1:
// Send a valid response to first request.
var buf bytes.Buffer
enc := hpack.NewEncoder(&buf)
enc.WriteField(hpack.HeaderField{Name: ":status", Value: "200"})
ct.fr.WriteHeaders(HeadersFrameParam{
StreamID: f.StreamID,
EndHeaders: true,
EndStream: true,
BlockFragment: buf.Bytes(),
})
case 3:
ct.fr.WriteData(f.StreamID, true, []byte("payload"))
}
default:
return fmt.Errorf("Unexpected client frame %v", f)
}
}
}
ct.run()
}
func TestTransportRequestsStallAtServerLimit(t *testing.T) {
const maxConcurrent = 2

Expand Down

0 comments on commit 57efc9c

Please sign in to comment.