Skip to content

Commit 57efc9c

Browse files
rstombergan
authored andcommitted
http2: reject DATA frame before HEADERS frame
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]>
1 parent 1c05540 commit 57efc9c

File tree

2 files changed

+63
-0
lines changed

2 files changed

+63
-0
lines changed

http2/transport.go

+8
Original file line numberDiff line numberDiff line change
@@ -1789,6 +1789,14 @@ func (rl *clientConnReadLoop) processData(f *DataFrame) error {
17891789
}
17901790
return nil
17911791
}
1792+
if !cs.firstByte {
1793+
cc.logf("protocol error: received DATA before a HEADERS frame")
1794+
rl.endStreamError(cs, StreamError{
1795+
StreamID: f.StreamID,
1796+
Code: ErrCodeProtocol,
1797+
})
1798+
return nil
1799+
}
17921800
if f.Length > 0 {
17931801
// Check connection-level flow control.
17941802
cc.mu.Lock()

http2/transport_test.go

+55
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"math/rand"
1717
"net"
1818
"net/http"
19+
"net/http/httptest"
1920
"net/url"
2021
"os"
2122
"reflect"
@@ -3036,6 +3037,60 @@ func TestTransportRetryHasLimit(t *testing.T) {
30363037
ct.run()
30373038
}
30383039

3040+
func TestTransportResponseDataBeforeHeaders(t *testing.T) {
3041+
ct := newClientTester(t)
3042+
ct.client = func() error {
3043+
defer ct.cc.(*net.TCPConn).CloseWrite()
3044+
req := httptest.NewRequest("GET", "https://dummy.tld/", nil)
3045+
// First request is normal to ensure the check is per stream and not per connection.
3046+
_, err := ct.tr.RoundTrip(req)
3047+
if err != nil {
3048+
return fmt.Errorf("RoundTrip expected no error, got: %v", err)
3049+
}
3050+
// Second request returns a DATA frame with no HEADERS.
3051+
resp, err := ct.tr.RoundTrip(req)
3052+
if err == nil {
3053+
return fmt.Errorf("RoundTrip expected error, got response: %+v", resp)
3054+
}
3055+
if err, ok := err.(StreamError); !ok || err.Code != ErrCodeProtocol {
3056+
return fmt.Errorf("expected stream PROTOCOL_ERROR, got: %v", err)
3057+
}
3058+
return nil
3059+
}
3060+
ct.server = func() error {
3061+
ct.greet()
3062+
for {
3063+
f, err := ct.fr.ReadFrame()
3064+
if err == io.EOF {
3065+
return nil
3066+
} else if err != nil {
3067+
return err
3068+
}
3069+
switch f := f.(type) {
3070+
case *WindowUpdateFrame, *SettingsFrame:
3071+
case *HeadersFrame:
3072+
switch f.StreamID {
3073+
case 1:
3074+
// Send a valid response to first request.
3075+
var buf bytes.Buffer
3076+
enc := hpack.NewEncoder(&buf)
3077+
enc.WriteField(hpack.HeaderField{Name: ":status", Value: "200"})
3078+
ct.fr.WriteHeaders(HeadersFrameParam{
3079+
StreamID: f.StreamID,
3080+
EndHeaders: true,
3081+
EndStream: true,
3082+
BlockFragment: buf.Bytes(),
3083+
})
3084+
case 3:
3085+
ct.fr.WriteData(f.StreamID, true, []byte("payload"))
3086+
}
3087+
default:
3088+
return fmt.Errorf("Unexpected client frame %v", f)
3089+
}
3090+
}
3091+
}
3092+
ct.run()
3093+
}
30393094
func TestTransportRequestsStallAtServerLimit(t *testing.T) {
30403095
const maxConcurrent = 2
30413096

0 commit comments

Comments
 (0)