Skip to content

Commit ea3f329

Browse files
urldbradfitz
authored andcommitted
net/http: omit forbidden Trailer headers from response
Use the vendored ValidTrailerHeader function from x/net/http/httpguts to check Trailer headers according to RFC 7230. The previous implementation only omitted illegal Trailer headers defined in RFC 2616. This CL adds x/net/http/httpguts from CL 104042 (git rev a35a21de97) Fixes #23908 Change-Id: Ib2329a384040494093c18e209db9b62aaf86e921 Reviewed-on: https://go-review.googlesource.com/104075 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent 7b7affa commit ea3f329

File tree

4 files changed

+67
-17
lines changed

4 files changed

+67
-17
lines changed

src/go/build/deps_test.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ var pkgDeps = map[string][]string{
400400
"context",
401401
"crypto/rand",
402402
"crypto/tls",
403+
"golang_org/x/net/http/httpguts",
403404
"golang_org/x/net/http2/hpack",
404405
"golang_org/x/net/idna",
405406
"golang_org/x/net/lex/httplex",
@@ -419,11 +420,14 @@ var pkgDeps = map[string][]string{
419420
"net/http/cgi": {"L4", "NET", "OS", "crypto/tls", "net/http", "regexp"},
420421
"net/http/cookiejar": {"L4", "NET", "net/http"},
421422
"net/http/fcgi": {"L4", "NET", "OS", "context", "net/http", "net/http/cgi"},
422-
"net/http/httptest": {"L4", "NET", "OS", "crypto/tls", "flag", "net/http", "net/http/internal", "crypto/x509"},
423-
"net/http/httputil": {"L4", "NET", "OS", "context", "net/http", "net/http/internal"},
424-
"net/http/pprof": {"L4", "OS", "html/template", "net/http", "runtime/pprof", "runtime/trace"},
425-
"net/rpc": {"L4", "NET", "encoding/gob", "html/template", "net/http"},
426-
"net/rpc/jsonrpc": {"L4", "NET", "encoding/json", "net/rpc"},
423+
"net/http/httptest": {
424+
"L4", "NET", "OS", "crypto/tls", "flag", "net/http", "net/http/internal", "crypto/x509",
425+
"golang_org/x/net/http/httpguts",
426+
},
427+
"net/http/httputil": {"L4", "NET", "OS", "context", "net/http", "net/http/internal"},
428+
"net/http/pprof": {"L4", "OS", "html/template", "net/http", "runtime/pprof", "runtime/trace"},
429+
"net/rpc": {"L4", "NET", "encoding/gob", "html/template", "net/http"},
430+
"net/rpc/jsonrpc": {"L4", "NET", "encoding/json", "net/rpc"},
427431
}
428432

429433
// isMacro reports whether p is a package dependency macro

src/net/http/httptest/recorder.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111
"net/http"
1212
"strconv"
1313
"strings"
14+
15+
"golang_org/x/net/http/httpguts"
1416
)
1517

1618
// ResponseRecorder is an implementation of http.ResponseWriter that
@@ -186,16 +188,11 @@ func (rw *ResponseRecorder) Result() *http.Response {
186188
if trailers, ok := rw.snapHeader["Trailer"]; ok {
187189
res.Trailer = make(http.Header, len(trailers))
188190
for _, k := range trailers {
189-
// TODO: use http2.ValidTrailerHeader, but we can't
190-
// get at it easily because it's bundled into net/http
191-
// unexported. This is good enough for now:
192-
switch k {
193-
case "Transfer-Encoding", "Content-Length", "Trailer":
194-
// Ignore since forbidden by RFC 2616 14.40.
195-
// TODO: inconsistent with RFC 7230, section 4.1.2.
191+
k = http.CanonicalHeaderKey(k)
192+
if !httpguts.ValidTrailerHeader(k) {
193+
// Ignore since forbidden by RFC 7230, section 4.1.2.
196194
continue
197195
}
198-
k = http.CanonicalHeaderKey(k)
199196
vv, ok := rw.HeaderMap[k]
200197
if !ok {
201198
continue

src/net/http/server.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"sync/atomic"
2929
"time"
3030

31+
"golang_org/x/net/http/httpguts"
3132
"golang_org/x/net/lex/httplex"
3233
)
3334

@@ -510,10 +511,8 @@ func (b *atomicBool) setTrue() { atomic.StoreInt32((*int32)(b), 1) }
510511
// written in the trailers at the end of the response.
511512
func (w *response) declareTrailer(k string) {
512513
k = CanonicalHeaderKey(k)
513-
switch k {
514-
case "Transfer-Encoding", "Content-Length", "Trailer":
515-
// Forbidden by RFC 2616 14.40.
516-
// TODO: inconsistent with RFC 7230, section 4.1.2
514+
if !httpguts.ValidTrailerHeader(k) {
515+
// Forbidden by RFC 7230, section 4.1.2
517516
return
518517
}
519518
w.trailers = append(w.trailers, k)
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright 2018 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Package httpguts provides functions implementing various details
6+
// of the HTTP specification.
7+
//
8+
// This package is shared by the standard library (which vendors it)
9+
// and x/net/http2. It comes with no API stability promise.
10+
package httpguts
11+
12+
import (
13+
"net/textproto"
14+
"strings"
15+
)
16+
17+
// ValidTrailerHeader reports whether name is a valid header field name to appear
18+
// in trailers.
19+
// See RFC 7230, Section 4.1.2
20+
func ValidTrailerHeader(name string) bool {
21+
name = textproto.CanonicalMIMEHeaderKey(name)
22+
if strings.HasPrefix(name, "If-") || badTrailer[name] {
23+
return false
24+
}
25+
return true
26+
}
27+
28+
var badTrailer = map[string]bool{
29+
"Authorization": true,
30+
"Cache-Control": true,
31+
"Connection": true,
32+
"Content-Encoding": true,
33+
"Content-Length": true,
34+
"Content-Range": true,
35+
"Content-Type": true,
36+
"Expect": true,
37+
"Host": true,
38+
"Keep-Alive": true,
39+
"Max-Forwards": true,
40+
"Pragma": true,
41+
"Proxy-Authenticate": true,
42+
"Proxy-Authorization": true,
43+
"Proxy-Connection": true,
44+
"Range": true,
45+
"Realm": true,
46+
"Te": true,
47+
"Trailer": true,
48+
"Transfer-Encoding": true,
49+
"Www-Authenticate": true,
50+
}

0 commit comments

Comments
 (0)