Skip to content

Commit 66aacef

Browse files
applebytombergan
authored andcommitted
http2: Respect peer's SETTINGS_MAX_HEADER_LIST_SIZE in ClientConn
Add a new peerMaxHeaderListSize member to ClientConn which records the SETTINGS_MAX_HEADER_LIST_SIZE requested by the client's peer, and respect this limit in (*ClientConn) encodeHeaders / encodeTrailers. Attempting to send more than peerMaxHeaderListSize bytes of headers or trailers will result in RoundTrip returning errRequestHeaderListSize. Updates golang/go#13959 Change-Id: Ic707179782acdf8ae543429ea1af7f4f30e67e59 Reviewed-on: https://go-review.googlesource.com/29243 Run-TryBot: Tom Bergan <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Tom Bergan <[email protected]>
1 parent 57efc9c commit 66aacef

File tree

2 files changed

+426
-69
lines changed

2 files changed

+426
-69
lines changed

http2/transport.go

+114-68
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ type Transport struct {
8787

8888
// MaxHeaderListSize is the http2 SETTINGS_MAX_HEADER_LIST_SIZE to
8989
// send in the initial settings frame. It is how many bytes
90-
// of response headers are allow. Unlike the http2 spec, zero here
90+
// of response headers are allowed. Unlike the http2 spec, zero here
9191
// means to use a default limit (currently 10MB). If you actually
9292
// want to advertise an ulimited value to the peer, Transport
9393
// interprets the highest possible value here (0xffffffff or 1<<32-1)
@@ -172,9 +172,10 @@ type ClientConn struct {
172172
fr *Framer
173173
lastActive time.Time
174174
// Settings from peer: (also guarded by mu)
175-
maxFrameSize uint32
176-
maxConcurrentStreams uint32
177-
initialWindowSize uint32
175+
maxFrameSize uint32
176+
maxConcurrentStreams uint32
177+
peerMaxHeaderListSize uint64
178+
initialWindowSize uint32
178179

179180
hbuf bytes.Buffer // HPACK encoder writes into this
180181
henc *hpack.Encoder
@@ -519,17 +520,18 @@ func (t *Transport) NewClientConn(c net.Conn) (*ClientConn, error) {
519520

520521
func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, error) {
521522
cc := &ClientConn{
522-
t: t,
523-
tconn: c,
524-
readerDone: make(chan struct{}),
525-
nextStreamID: 1,
526-
maxFrameSize: 16 << 10, // spec default
527-
initialWindowSize: 65535, // spec default
528-
maxConcurrentStreams: 1000, // "infinite", per spec. 1000 seems good enough.
529-
streams: make(map[uint32]*clientStream),
530-
singleUse: singleUse,
531-
wantSettingsAck: true,
532-
pings: make(map[[8]byte]chan struct{}),
523+
t: t,
524+
tconn: c,
525+
readerDone: make(chan struct{}),
526+
nextStreamID: 1,
527+
maxFrameSize: 16 << 10, // spec default
528+
initialWindowSize: 65535, // spec default
529+
maxConcurrentStreams: 1000, // "infinite", per spec. 1000 seems good enough.
530+
peerMaxHeaderListSize: 0xffffffffffffffff, // "infinite", per spec. Use 2^64-1 instead.
531+
streams: make(map[uint32]*clientStream),
532+
singleUse: singleUse,
533+
wantSettingsAck: true,
534+
pings: make(map[[8]byte]chan struct{}),
533535
}
534536
if d := t.idleConnTimeout(); d != 0 {
535537
cc.idleTimeout = d
@@ -1085,8 +1087,13 @@ func (cs *clientStream) writeRequestBody(body io.Reader, bodyCloser io.Closer) (
10851087
var trls []byte
10861088
if hasTrailers {
10871089
cc.mu.Lock()
1088-
defer cc.mu.Unlock()
1089-
trls = cc.encodeTrailers(req)
1090+
trls, err = cc.encodeTrailers(req)
1091+
cc.mu.Unlock()
1092+
if err != nil {
1093+
cc.writeStreamReset(cs.ID, ErrCodeInternal, err)
1094+
cc.forgetStreamID(cs.ID)
1095+
return err
1096+
}
10901097
}
10911098

10921099
cc.wmu.Lock()
@@ -1189,62 +1196,86 @@ func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trail
11891196
}
11901197
}
11911198

1192-
// 8.1.2.3 Request Pseudo-Header Fields
1193-
// The :path pseudo-header field includes the path and query parts of the
1194-
// target URI (the path-absolute production and optionally a '?' character
1195-
// followed by the query production (see Sections 3.3 and 3.4 of
1196-
// [RFC3986]).
1197-
cc.writeHeader(":authority", host)
1198-
cc.writeHeader(":method", req.Method)
1199-
if req.Method != "CONNECT" {
1200-
cc.writeHeader(":path", path)
1201-
cc.writeHeader(":scheme", req.URL.Scheme)
1202-
}
1203-
if trailers != "" {
1204-
cc.writeHeader("trailer", trailers)
1205-
}
1199+
enumerateHeaders := func(f func(name, value string)) {
1200+
// 8.1.2.3 Request Pseudo-Header Fields
1201+
// The :path pseudo-header field includes the path and query parts of the
1202+
// target URI (the path-absolute production and optionally a '?' character
1203+
// followed by the query production (see Sections 3.3 and 3.4 of
1204+
// [RFC3986]).
1205+
f(":authority", host)
1206+
f(":method", req.Method)
1207+
if req.Method != "CONNECT" {
1208+
f(":path", path)
1209+
f(":scheme", req.URL.Scheme)
1210+
}
1211+
if trailers != "" {
1212+
f("trailer", trailers)
1213+
}
12061214

1207-
var didUA bool
1208-
for k, vv := range req.Header {
1209-
lowKey := strings.ToLower(k)
1210-
switch lowKey {
1211-
case "host", "content-length":
1212-
// Host is :authority, already sent.
1213-
// Content-Length is automatic, set below.
1214-
continue
1215-
case "connection", "proxy-connection", "transfer-encoding", "upgrade", "keep-alive":
1216-
// Per 8.1.2.2 Connection-Specific Header
1217-
// Fields, don't send connection-specific
1218-
// fields. We have already checked if any
1219-
// are error-worthy so just ignore the rest.
1220-
continue
1221-
case "user-agent":
1222-
// Match Go's http1 behavior: at most one
1223-
// User-Agent. If set to nil or empty string,
1224-
// then omit it. Otherwise if not mentioned,
1225-
// include the default (below).
1226-
didUA = true
1227-
if len(vv) < 1 {
1215+
var didUA bool
1216+
for k, vv := range req.Header {
1217+
if strings.EqualFold(k, "host") || strings.EqualFold(k, "content-length") {
1218+
// Host is :authority, already sent.
1219+
// Content-Length is automatic, set below.
12281220
continue
1229-
}
1230-
vv = vv[:1]
1231-
if vv[0] == "" {
1221+
} else if strings.EqualFold(k, "connection") || strings.EqualFold(k, "proxy-connection") ||
1222+
strings.EqualFold(k, "transfer-encoding") || strings.EqualFold(k, "upgrade") ||
1223+
strings.EqualFold(k, "keep-alive") {
1224+
// Per 8.1.2.2 Connection-Specific Header
1225+
// Fields, don't send connection-specific
1226+
// fields. We have already checked if any
1227+
// are error-worthy so just ignore the rest.
12321228
continue
1229+
} else if strings.EqualFold(k, "user-agent") {
1230+
// Match Go's http1 behavior: at most one
1231+
// User-Agent. If set to nil or empty string,
1232+
// then omit it. Otherwise if not mentioned,
1233+
// include the default (below).
1234+
didUA = true
1235+
if len(vv) < 1 {
1236+
continue
1237+
}
1238+
vv = vv[:1]
1239+
if vv[0] == "" {
1240+
continue
1241+
}
1242+
1243+
}
1244+
1245+
for _, v := range vv {
1246+
f(k, v)
12331247
}
12341248
}
1235-
for _, v := range vv {
1236-
cc.writeHeader(lowKey, v)
1249+
if shouldSendReqContentLength(req.Method, contentLength) {
1250+
f("content-length", strconv.FormatInt(contentLength, 10))
1251+
}
1252+
if addGzipHeader {
1253+
f("accept-encoding", "gzip")
1254+
}
1255+
if !didUA {
1256+
f("user-agent", defaultUserAgent)
12371257
}
12381258
}
1239-
if shouldSendReqContentLength(req.Method, contentLength) {
1240-
cc.writeHeader("content-length", strconv.FormatInt(contentLength, 10))
1241-
}
1242-
if addGzipHeader {
1243-
cc.writeHeader("accept-encoding", "gzip")
1244-
}
1245-
if !didUA {
1246-
cc.writeHeader("user-agent", defaultUserAgent)
1259+
1260+
// Do a first pass over the headers counting bytes to ensure
1261+
// we don't exceed cc.peerMaxHeaderListSize. This is done as a
1262+
// separate pass before encoding the headers to prevent
1263+
// modifying the hpack state.
1264+
hlSize := uint64(0)
1265+
enumerateHeaders(func(name, value string) {
1266+
hf := hpack.HeaderField{Name: name, Value: value}
1267+
hlSize += uint64(hf.Size())
1268+
})
1269+
1270+
if hlSize > cc.peerMaxHeaderListSize {
1271+
return nil, errRequestHeaderListSize
12471272
}
1273+
1274+
// Header list size is ok. Write the headers.
1275+
enumerateHeaders(func(name, value string) {
1276+
cc.writeHeader(strings.ToLower(name), value)
1277+
})
1278+
12481279
return cc.hbuf.Bytes(), nil
12491280
}
12501281

@@ -1271,17 +1302,29 @@ func shouldSendReqContentLength(method string, contentLength int64) bool {
12711302
}
12721303

12731304
// requires cc.mu be held.
1274-
func (cc *ClientConn) encodeTrailers(req *http.Request) []byte {
1305+
func (cc *ClientConn) encodeTrailers(req *http.Request) ([]byte, error) {
12751306
cc.hbuf.Reset()
1307+
1308+
hlSize := uint64(0)
1309+
for k, vv := range req.Trailer {
1310+
for _, v := range vv {
1311+
hf := hpack.HeaderField{Name: k, Value: v}
1312+
hlSize += uint64(hf.Size())
1313+
}
1314+
}
1315+
if hlSize > cc.peerMaxHeaderListSize {
1316+
return nil, errRequestHeaderListSize
1317+
}
1318+
12761319
for k, vv := range req.Trailer {
1277-
// Transfer-Encoding, etc.. have already been filter at the
1320+
// Transfer-Encoding, etc.. have already been filtered at the
12781321
// start of RoundTrip
12791322
lowKey := strings.ToLower(k)
12801323
for _, v := range vv {
12811324
cc.writeHeader(lowKey, v)
12821325
}
12831326
}
1284-
return cc.hbuf.Bytes()
1327+
return cc.hbuf.Bytes(), nil
12851328
}
12861329

12871330
func (cc *ClientConn) writeHeader(name, value string) {
@@ -1911,6 +1954,8 @@ func (rl *clientConnReadLoop) processSettings(f *SettingsFrame) error {
19111954
cc.maxFrameSize = s.Val
19121955
case SettingMaxConcurrentStreams:
19131956
cc.maxConcurrentStreams = s.Val
1957+
case SettingMaxHeaderListSize:
1958+
cc.peerMaxHeaderListSize = uint64(s.Val)
19141959
case SettingInitialWindowSize:
19151960
// Values above the maximum flow-control
19161961
// window size of 2^31-1 MUST be treated as a
@@ -2077,6 +2122,7 @@ func (cc *ClientConn) writeStreamReset(streamID uint32, code ErrCode, err error)
20772122

20782123
var (
20792124
errResponseHeaderListSize = errors.New("http2: response header list larger than advertised limit")
2125+
errRequestHeaderListSize = errors.New("http2: request header list larger than peer's advertised limit")
20802126
errPseudoTrailers = errors.New("http2: invalid pseudo header in trailers")
20812127
)
20822128

0 commit comments

Comments
 (0)