Skip to content

Commit

Permalink
http2: validate client/outgoing trailers
Browse files Browse the repository at this point in the history
This change is a counterpart to the HTTP/1.1 trailers
validation CL 572615. This change will ensure that we
validate trailers that were set on the HTTP client
before they are transformed to HTTP/2 equivalents.

Updates golang/go#64766

Change-Id: Id1afd7f7e9af820ea969ef226bbb16e4de6d57a5
Reviewed-on: https://go-review.googlesource.com/c/net/+/572655
Auto-Submit: Damien Neil <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Run-TryBot: Emmanuel Odeke <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: David Chase <[email protected]>
  • Loading branch information
odeke-em authored and gopherbot committed Mar 19, 2024
1 parent 6e2c99c commit 89f602b
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 12 deletions.
33 changes: 22 additions & 11 deletions http2/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -2019,6 +2019,22 @@ func (cs *clientStream) awaitFlowControl(maxBytes int) (taken int32, err error)
}
}

func validateHeaders(hdrs http.Header) string {
for k, vv := range hdrs {
if !httpguts.ValidHeaderFieldName(k) {
return fmt.Sprintf("name %q", k)
}
for _, v := range vv {
if !httpguts.ValidHeaderFieldValue(v) {
// Don't include the value in the error,
// because it may be sensitive.
return fmt.Sprintf("value for header %q", k)
}
}
}
return ""
}

var errNilRequestURL = errors.New("http2: Request.URI is nil")

// requires cc.wmu be held.
Expand Down Expand Up @@ -2056,19 +2072,14 @@ func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trail
}
}

// Check for any invalid headers and return an error before we
// Check for any invalid headers+trailers and return an error before we
// potentially pollute our hpack state. (We want to be able to
// continue to reuse the hpack encoder for future requests)
for k, vv := range req.Header {
if !httpguts.ValidHeaderFieldName(k) {
return nil, fmt.Errorf("invalid HTTP header name %q", k)
}
for _, v := range vv {
if !httpguts.ValidHeaderFieldValue(v) {
// Don't include the value in the error, because it may be sensitive.
return nil, fmt.Errorf("invalid HTTP header value for header %q", k)
}
}
if err := validateHeaders(req.Header); err != "" {
return nil, fmt.Errorf("invalid HTTP header %s", err)
}
if err := validateHeaders(req.Trailer); err != "" {
return nil, fmt.Errorf("invalid HTTP trailer %s", err)
}

enumerateHeaders := func(f func(name, value string)) {
Expand Down
13 changes: 12 additions & 1 deletion http2/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2290,7 +2290,8 @@ func TestTransportRejectsContentLengthWithSign(t *testing.T) {
}

// golang.org/issue/14048
func TestTransportFailsOnInvalidHeaders(t *testing.T) {
// golang.org/issue/64766
func TestTransportFailsOnInvalidHeadersAndTrailers(t *testing.T) {
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
var got []string
for k := range r.Header {
Expand All @@ -2303,6 +2304,7 @@ func TestTransportFailsOnInvalidHeaders(t *testing.T) {

tests := [...]struct {
h http.Header
t http.Header
wantErr string
}{
0: {
Expand All @@ -2321,6 +2323,14 @@ func TestTransportFailsOnInvalidHeaders(t *testing.T) {
h: http.Header{"foo": {"foo\x01bar"}},
wantErr: `invalid HTTP header value for header "foo"`,
},
4: {
t: http.Header{"foo": {"foo\x01bar"}},
wantErr: `invalid HTTP trailer value for header "foo"`,
},
5: {
t: http.Header{"x-\r\nda": {"foo\x01bar"}},
wantErr: `invalid HTTP trailer name "x-\r\nda"`,
},
}

tr := &Transport{TLSClientConfig: tlsConfigInsecure}
Expand All @@ -2329,6 +2339,7 @@ func TestTransportFailsOnInvalidHeaders(t *testing.T) {
for i, tt := range tests {
req, _ := http.NewRequest("GET", st.ts.URL, nil)
req.Header = tt.h
req.Trailer = tt.t
res, err := tr.RoundTrip(req)
var bad bool
if tt.wantErr == "" {
Expand Down

0 comments on commit 89f602b

Please sign in to comment.