diff --git a/http2/server.go b/http2/server.go index dff9604fd1..407ca3cff3 100644 --- a/http2/server.go +++ b/http2/server.go @@ -528,6 +528,7 @@ type serverConn struct { headerTableSize uint32 peerMaxHeaderListSize uint32 // zero means unknown (default) canonHeader map[string]string // http2-lower-case -> Go-Canonical-Case + canonHeaderKeysSize int // canonHeader keys size in bytes writingFrame bool // started writing a frame (on serve goroutine or separate) writingFrameAsync bool // started a frame on its own goroutine but haven't heard back on wroteFrameCh needsFrameFlush bool // last frame write wasn't a flush @@ -704,6 +705,13 @@ func (sc *serverConn) condlogf(err error, format string, args ...interface{}) { } } +// maxCachedCanonicalHeadersKeysSize is an arbitrarily-chosen limit on the size +// of the entries in the canonHeader cache. +// This should be larger than the size of unique, uncommon header keys likely to +// be sent by the peer, while not so high as to permit unreasonable memory usage +// if the peer sends an unbounded number of unique header keys. +const maxCachedCanonicalHeadersKeysSize = 2048 + func (sc *serverConn) canonicalHeader(v string) string { sc.serveG.check() buildCommonHeaderMapsOnce() @@ -719,14 +727,10 @@ func (sc *serverConn) canonicalHeader(v string) string { sc.canonHeader = make(map[string]string) } cv = http.CanonicalHeaderKey(v) - // maxCachedCanonicalHeaders is an arbitrarily-chosen limit on the number of - // entries in the canonHeader cache. This should be larger than the number - // of unique, uncommon header keys likely to be sent by the peer, while not - // so high as to permit unreaasonable memory usage if the peer sends an unbounded - // number of unique header keys. - const maxCachedCanonicalHeaders = 32 - if len(sc.canonHeader) < maxCachedCanonicalHeaders { + size := 100 + len(v)*2 // 100 bytes of map overhead + key + value + if sc.canonHeaderKeysSize+size <= maxCachedCanonicalHeadersKeysSize { sc.canonHeader[v] = cv + sc.canonHeaderKeysSize += size } return cv } diff --git a/http2/server_test.go b/http2/server_test.go index 9421518a8b..6c54d7dec8 100644 --- a/http2/server_test.go +++ b/http2/server_test.go @@ -4411,3 +4411,29 @@ func TestProtocolErrorAfterGoAway(t *testing.T) { } } } + +// TestCanonicalHeaderCacheGrowth verifies that the canonical header cache +// size is capped to a reasonable level. +func TestCanonicalHeaderCacheGrowth(t *testing.T) { + for _, size := range []int{1, (1 << 20) - 10} { + base := strings.Repeat("X", size) + sc := &serverConn{ + serveG: newGoroutineLock(), + } + const count = 1000 + for i := 0; i < count; i++ { + h := fmt.Sprintf("%v-%v", base, i) + c := sc.canonicalHeader(h) + if len(h) != len(c) { + t.Errorf("sc.canonicalHeader(%q) = %q, want same length", h, c) + } + } + total := 0 + for k, v := range sc.canonHeader { + total += len(k) + len(v) + 100 + } + if total > maxCachedCanonicalHeadersKeysSize { + t.Errorf("after adding %v ~%v-byte headers, canonHeader cache is ~%v bytes, want <%v", count, size, total, maxCachedCanonicalHeadersKeysSize) + } + } +}