Skip to content

Commit

Permalink
http2: ignore unknown 1xx responses like HTTP/1
Browse files Browse the repository at this point in the history
Updates golang/go#26189 (fixes after vendor into std)
Updates golang/go#17739

Change-Id: I076cdbb57841b7dbbaa764d11240913bc3a8b05d
Reviewed-on: https://go-review.googlesource.com/123615
Reviewed-by: Ian Lance Taylor <[email protected]>
  • Loading branch information
bradfitz committed Jul 12, 2018
1 parent cffdcf6 commit a45b4ab
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 23 deletions.
9 changes: 9 additions & 0 deletions http2/go111.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

package http2

import "net/textproto"

func traceHasWroteHeaderField(trace *clientTrace) bool {
return trace != nil && trace.WroteHeaderField != nil
}
Expand All @@ -15,3 +17,10 @@ func traceWroteHeaderField(trace *clientTrace, k, v string) {
trace.WroteHeaderField(k, []string{v})
}
}

func traceGot1xxResponseFunc(trace *clientTrace) func(int, textproto.MIMEHeader) error {
if trace != nil {
return trace.Got1xxResponse
}
return nil
}
6 changes: 6 additions & 0 deletions http2/not_go111.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@

package http2

import "net/textproto"

func traceHasWroteHeaderField(trace *clientTrace) bool { return false }

func traceWroteHeaderField(trace *clientTrace, k, v string) {}

func traceGot1xxResponseFunc(trace *clientTrace) func(int, textproto.MIMEHeader) error {
return nil
}
75 changes: 52 additions & 23 deletions http2/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
mathrand "math/rand"
"net"
"net/http"
"net/textproto"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -212,9 +213,10 @@ type clientStream struct {
done chan struct{} // closed when stream remove from cc.streams map; close calls guarded by cc.mu

// owned by clientConnReadLoop:
firstByte bool // got the first response byte
pastHeaders bool // got first MetaHeadersFrame (actual headers)
pastTrailers bool // got optional second MetaHeadersFrame (trailers)
firstByte bool // got the first response byte
pastHeaders bool // got first MetaHeadersFrame (actual headers)
pastTrailers bool // got optional second MetaHeadersFrame (trailers)
num1xx uint8 // number of 1xx responses seen

trailer http.Header // accumulated trailers
resTrailer *http.Header // client's Response.Trailer
Expand All @@ -238,6 +240,17 @@ func awaitRequestCancel(req *http.Request, done <-chan struct{}) error {
}
}

var got1xxFuncForTests func(int, textproto.MIMEHeader) error

// get1xxTraceFunc returns the value of request's httptrace.ClientTrace.Got1xxResponse func,
// if any. It returns nil if not set or if the Go version is too old.
func (cs *clientStream) get1xxTraceFunc() func(int, textproto.MIMEHeader) error {
if fn := got1xxFuncForTests; fn != nil {
return fn
}
return traceGot1xxResponseFunc(cs.trace)
}

// awaitRequestCancel waits for the user to cancel a request, its context to
// expire, or for the request to be done (any way it might be removed from the
// cc.streams map: peer reset, successful completion, TCP connection breakage,
Expand Down Expand Up @@ -1734,8 +1747,7 @@ func (rl *clientConnReadLoop) processHeaders(f *MetaHeadersFrame) error {
// is the detail.
//
// As a special case, handleResponse may return (nil, nil) to skip the
// frame (currently only used for 100 expect continue). This special
// case is going away after Issue 13851 is fixed.
// frame (currently only used for 1xx responses).
func (rl *clientConnReadLoop) handleResponse(cs *clientStream, f *MetaHeadersFrame) (*http.Response, error) {
if f.Truncated {
return nil, errResponseHeaderListSize
Expand All @@ -1750,37 +1762,54 @@ func (rl *clientConnReadLoop) handleResponse(cs *clientStream, f *MetaHeadersFra
return nil, errors.New("malformed response from server: malformed non-numeric status pseudo header")
}

if statusCode == 100 {
traceGot100Continue(cs.trace)
if cs.on100 != nil {
cs.on100() // forces any write delay timer to fire
header := make(http.Header)
var trailerValue string
for _, hf := range f.RegularFields() {
key := http.CanonicalHeaderKey(hf.Name)
if key == "Trailer" {
trailerValue = hf.Value
} else {
header[key] = append(header[key], hf.Value)
}
}

if statusCode >= 100 && statusCode <= 199 {
cs.num1xx++
const max1xxResponses = 5 // arbitrary bound on number of informational responses, same as net/http
if cs.num1xx > max1xxResponses {
return nil, errors.New("http2: too many 1xx informational responses")
}
if fn := cs.get1xxTraceFunc(); fn != nil {
if err := fn(statusCode, textproto.MIMEHeader(header)); err != nil {
return nil, err
}
}
if statusCode == 100 {
traceGot100Continue(cs.trace)
if cs.on100 != nil {
cs.on100() // forces any write delay timer to fire
}
}
cs.pastHeaders = false // do it all again
return nil, nil
}

header := make(http.Header)
res := &http.Response{
Proto: "HTTP/2.0",
ProtoMajor: 2,
Header: header,
StatusCode: statusCode,
Status: status + " " + http.StatusText(statusCode),
}
for _, hf := range f.RegularFields() {
key := http.CanonicalHeaderKey(hf.Name)
if key == "Trailer" {
t := res.Trailer
if t == nil {
t = make(http.Header)
res.Trailer = t
}
foreachHeaderElement(hf.Value, func(v string) {
t[http.CanonicalHeaderKey(v)] = nil
})
} else {
header[key] = append(header[key], hf.Value)
if trailerValue != "" {
t := res.Trailer
if t == nil {
t = make(http.Header)
res.Trailer = t
}
foreachHeaderElement(trailerValue, func(v string) {
t[http.CanonicalHeaderKey(v)] = nil
})
}

streamEnded := f.StreamEnded()
Expand Down
72 changes: 72 additions & 0 deletions http2/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"net"
"net/http"
"net/http/httptest"
"net/textproto"
"net/url"
"os"
"reflect"
Expand Down Expand Up @@ -1192,6 +1193,77 @@ func testTransportResPattern(t *testing.T, expect100Continue, resHeader headerTy
ct.run()
}

// Issue 26189, Issue 17739: ignore unknown 1xx responses
func TestTransportUnknown1xx(t *testing.T) {
var buf bytes.Buffer
defer func() { got1xxFuncForTests = nil }()
got1xxFuncForTests = func(code int, header textproto.MIMEHeader) error {
fmt.Fprintf(&buf, "code=%d header=%v\n", code, header)
return nil
}

ct := newClientTester(t)
ct.client = func() error {
req, _ := http.NewRequest("GET", "https://dummy.tld/", nil)
res, err := ct.tr.RoundTrip(req)
if err != nil {
return fmt.Errorf("RoundTrip: %v", err)
}
defer res.Body.Close()
if res.StatusCode != 204 {
return fmt.Errorf("status code = %v; want 204", res.StatusCode)
}
want := `code=110 header=map[Foo-Bar:[110]]
code=111 header=map[Foo-Bar:[111]]
code=112 header=map[Foo-Bar:[112]]
code=113 header=map[Foo-Bar:[113]]
code=114 header=map[Foo-Bar:[114]]
`
if got := buf.String(); got != want {
t.Errorf("Got trace:\n%s\nWant:\n%s", got, want)
}
return nil
}
ct.server = func() error {
ct.greet()
var buf bytes.Buffer
enc := hpack.NewEncoder(&buf)

for {
f, err := ct.fr.ReadFrame()
if err != nil {
return err
}
switch f := f.(type) {
case *WindowUpdateFrame, *SettingsFrame:
case *HeadersFrame:
for i := 110; i <= 114; i++ {
buf.Reset()
enc.WriteField(hpack.HeaderField{Name: ":status", Value: fmt.Sprint(i)})
enc.WriteField(hpack.HeaderField{Name: "foo-bar", Value: fmt.Sprint(i)})
ct.fr.WriteHeaders(HeadersFrameParam{
StreamID: f.StreamID,
EndHeaders: true,
EndStream: false,
BlockFragment: buf.Bytes(),
})
}
buf.Reset()
enc.WriteField(hpack.HeaderField{Name: ":status", Value: "204"})
ct.fr.WriteHeaders(HeadersFrameParam{
StreamID: f.StreamID,
EndHeaders: true,
EndStream: false,
BlockFragment: buf.Bytes(),
})
return nil
}
}
}
ct.run()

}

func TestTransportReceiveUndeclaredTrailer(t *testing.T) {
ct := newClientTester(t)
ct.client = func() error {
Expand Down

0 comments on commit a45b4ab

Please sign in to comment.