[net] http2: add SETTINGS_HEADER_TABLE_SIZE support
Gopher Robot (Gerrit)
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.
To view, visit change 435899. To unsubscribe, or for help writing mail filters, visit settings.
Eli (Gerrit)
Eli has uploaded this change for review.
http2: add SETTINGS_HEADER_TABLE_SIZE support
Add support for handling of SETTINGS_HEADER_TABLESIZE in SETTINGS
frames.
Add http2.Transport.MaxDecoderHeaderTableSize to set the advertised
table size for new client connections. Add
http2.Transport.MaxEncoderHeaderTableSize to cap the accepted size for
new client connections.
Add http2.Server.MaxDecoderHeaderTableSize and MaxEncoderHeaderTableSize
to do the same on the server.
Fixes golang/go#29356
Change-Id: I16ae0f84b8527dc1e09dfce081e9f408fd514513
---
M http2/hpack/encode.go
M http2/server.go
M http2/server_test.go
M http2/transport.go
M http2/transport_test.go
5 files changed, 280 insertions(+), 12 deletions(-)
diff --git a/http2/hpack/encode.go b/http2/hpack/encode.go
index 6886dc1..46219da 100644
--- a/http2/hpack/encode.go
+++ b/http2/hpack/encode.go
@@ -116,6 +116,11 @@
e.dynTab.setMaxSize(v)
}
+// MaxDynamicTableSize returns the current dynamic header table size.
+func (e *Encoder) MaxDynamicTableSize() (v uint32) {
+ return e.dynTab.maxSize
+}
+
// SetMaxDynamicTableSizeLimit changes the maximum value that can be
// specified in SetMaxDynamicTableSize to v. By default, it is set to
// 4096, which is the same size of the default dynamic header table
diff --git a/http2/server.go b/http2/server.go
index 43cc2a3..cdc4582 100644
--- a/http2/server.go
+++ b/http2/server.go
@@ -98,6 +98,19 @@
// the HTTP/2 spec's recommendations.
MaxConcurrentStreams uint32
+ // MaxDecoderHeaderTableSize optionally specifies the http2
+ // SETTINGS_HEADER_TABLE_SIZE to send in the initial settings frame. It
+ // informs the remote endpoint of the maximum size of the header compression
+ // table used to decode header blocks, in octets. If zero, the default value
+ // of 4096 is used.
+ MaxDecoderHeaderTableSize uint32
+
+ // MaxEncoderHeaderTableSize optionally specifies an upper limit for the
+ // header compression table used for encoding request headers. Received
+ // SETTINGS_HEADER_TABLE_SIZE settings are capped at this limit. If zero,
+ // the default value of 4096 is used.
+ MaxEncoderHeaderTableSize uint32
+
// MaxReadFrameSize optionally specifies the largest frame
// this server is willing to read. A valid value is between
// 16k and 16M, inclusive. If zero or otherwise invalid, a
@@ -170,6 +183,20 @@
return defaultMaxStreams
}
+func (s *Server) maxDecoderHeaderTableSize() uint32 {
+ if v := s.MaxDecoderHeaderTableSize; v > 0 {
+ return v
+ }
+ return initialHeaderTableSize
+}
+
+func (s *Server) maxEncoderHeaderTableSize() uint32 {
+ if v := s.MaxEncoderHeaderTableSize; v > 0 {
+ return v
+ }
+ return initialHeaderTableSize
+}
+
// maxQueuedControlFrames is the maximum number of control frames like
// SETTINGS, PING and RST_STREAM that will be queued for writing before
// the connection is closed to prevent memory exhaustion attacks.
@@ -394,7 +421,6 @@
advMaxStreams: s.maxConcurrentStreams(),
initialStreamSendWindowSize: initialWindowSize,
maxFrameSize: initialMaxFrameSize,
- headerTableSize: initialHeaderTableSize,
serveG: newGoroutineLock(),
pushEnabled: true,
sawClientPreface: opts.SawClientPreface,
@@ -424,12 +450,13 @@
sc.flow.add(initialWindowSize)
sc.inflow.add(initialWindowSize)
sc.hpackEncoder = hpack.NewEncoder(&sc.headerWriteBuf)
+ sc.hpackEncoder.SetMaxDynamicTableSizeLimit(s.maxEncoderHeaderTableSize())
fr := NewFramer(sc.bw, c)
if s.CountError != nil {
fr.countError = s.CountError
}
- fr.ReadMetaHeaders = hpack.NewDecoder(initialHeaderTableSize, nil)
+ fr.ReadMetaHeaders = hpack.NewDecoder(s.maxDecoderHeaderTableSize(), nil)
fr.MaxHeaderListSize = sc.maxHeaderListSize()
fr.SetMaxReadFrameSize(s.maxReadFrameSize())
sc.framer = fr
@@ -559,7 +586,6 @@
streams map[uint32]*stream
initialStreamSendWindowSize int32
maxFrameSize int32
- headerTableSize uint32
peerMaxHeaderListSize uint32 // zero means unknown (default)
canonHeader map[string]string // http2-lower-case -> Go-Canonical-Case
writingFrame bool // started writing a frame (on serve goroutine or separate)
@@ -862,6 +888,7 @@
{SettingMaxFrameSize, sc.srv.maxReadFrameSize()},
{SettingMaxConcurrentStreams, sc.advMaxStreams},
{SettingMaxHeaderListSize, sc.maxHeaderListSize()},
+ {SettingHeaderTableSize, sc.srv.maxDecoderHeaderTableSize()},
{SettingInitialWindowSize, uint32(sc.srv.initialStreamRecvWindowSize())},
},
})
@@ -1632,7 +1659,6 @@
}
switch s.ID {
case SettingHeaderTableSize:
- sc.headerTableSize = s.Val
sc.hpackEncoder.SetMaxDynamicTableSize(s.Val)
case SettingEnablePush:
sc.pushEnabled = s.Val != 0
diff --git a/http2/server_test.go b/http2/server_test.go
index 808b3d1..2b9fad5 100644
--- a/http2/server_test.go
+++ b/http2/server_test.go
@@ -2754,6 +2754,43 @@
}
}
+func TestServer_MaxDecoderHeaderTableSize(t *testing.T) {
+ wantHeaderTableSize := uint32(initialHeaderTableSize * 2)
+ st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {}, func(s *Server) {
+ s.MaxDecoderHeaderTableSize = wantHeaderTableSize
+ })
+ defer st.Close()
+
+ var advHeaderTableSize *uint32
+ st.greetAndCheckSettings(func(s Setting) error {
+ switch s.ID {
+ case SettingHeaderTableSize:
+ advHeaderTableSize = &s.Val
+ }
+ return nil
+ })
+
+ if advHeaderTableSize == nil {
+ t.Errorf("server didn't advertise a header table size")
+ } else if got, want := *advHeaderTableSize, wantHeaderTableSize; got != want {
+ t.Errorf("server advertised a header table size of %d, want %d", got, want)
+ }
+}
+
+func TestServer_MaxEncoderHeaderTableSize(t *testing.T) {
+ wantHeaderTableSize := uint32(initialHeaderTableSize / 2)
+ st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {}, func(s *Server) {
+ s.MaxEncoderHeaderTableSize = wantHeaderTableSize
+ })
+ defer st.Close()
+
+ st.greet()
+
+ if got, want := st.sc.hpackEncoder.MaxDynamicTableSize(), wantHeaderTableSize; got != want {
+ t.Errorf("server encoder is using a header table size of %d, want %d", got, want)
+ }
+}
+
// Issue 12843
func TestServerDoS_MaxHeaderListSize(t *testing.T) {
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {})
diff --git a/http2/transport.go b/http2/transport.go
index 9a874f7..dc27f9b 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -117,6 +117,19 @@
// to mean no limit.
MaxHeaderListSize uint32
+ // MaxDecoderHeaderTableSize optionally specifies the http2
+ // SETTINGS_HEADER_TABLE_SIZE to send in the initial settings frame. It
+ // informs the remote endpoint of the maximum size of the header compression
+ // table used to decode header blocks, in octets. If zero, the default value
+ // of 4096 is used.
+ MaxDecoderHeaderTableSize uint32
+
+ // MaxEncoderHeaderTableSize optionally specifies an upper limit for the
+ // header compression table used for encoding request headers. Received
+ // SETTINGS_HEADER_TABLE_SIZE settings are capped at this limit. If zero,
+ // the default value of 4096 is used.
+ MaxEncoderHeaderTableSize uint32
+
// StrictMaxConcurrentStreams controls whether the server's
// SETTINGS_MAX_CONCURRENT_STREAMS should be respected
// globally. If false, new TCP connections are created to the
@@ -292,10 +305,11 @@
lastActive time.Time
lastIdle time.Time // time last idle
// Settings from peer: (also guarded by wmu)
- maxFrameSize uint32
- maxConcurrentStreams uint32
- peerMaxHeaderListSize uint64
- initialWindowSize uint32
+ maxFrameSize uint32
+ maxConcurrentStreams uint32
+ peerMaxHeaderListSize uint64
+ peerMaxHeaderTableSize uint32
+ initialWindowSize uint32
// reqHeaderMu is a 1-element semaphore channel controlling access to sending new requests.
// Write to reqHeaderMu to lock it, read from it to unlock.
@@ -666,6 +680,20 @@
return t.t1.ExpectContinueTimeout
}
+func (t *Transport) maxDecoderHeaderTableSize() uint32 {
+ if v := t.MaxDecoderHeaderTableSize; v > 0 {
+ return v
+ }
+ return initialHeaderTableSize
+}
+
+func (t *Transport) maxEncoderHeaderTableSize() uint32 {
+ if v := t.MaxEncoderHeaderTableSize; v > 0 {
+ return v
+ }
+ return initialHeaderTableSize
+}
+
func (t *Transport) NewClientConn(c net.Conn) (*ClientConn, error) {
return t.newClientConn(c, t.disableKeepAlives())
}
@@ -709,12 +737,13 @@
if t.CountError != nil {
cc.fr.countError = t.CountError
}
- cc.fr.ReadMetaHeaders = hpack.NewDecoder(initialHeaderTableSize, nil)
+ maxHeaderTableSize := t.maxDecoderHeaderTableSize()
+ cc.fr.ReadMetaHeaders = hpack.NewDecoder(maxHeaderTableSize, nil)
cc.fr.MaxHeaderListSize = t.maxHeaderListSize()
- // TODO: SetMaxDynamicTableSize, SetMaxDynamicTableSizeLimit on
- // henc in response to SETTINGS frames?
cc.henc = hpack.NewEncoder(&cc.hbuf)
+ cc.henc.SetMaxDynamicTableSizeLimit(t.maxEncoderHeaderTableSize())
+ cc.peerMaxHeaderTableSize = initialHeaderTableSize
if t.AllowHTTP {
cc.nextStreamID = 3
@@ -732,6 +761,9 @@
if max := t.maxHeaderListSize(); max != 0 {
initialSettings = append(initialSettings, Setting{ID: SettingMaxHeaderListSize, Val: max})
}
+ if maxHeaderTableSize != initialHeaderTableSize {
+ initialSettings = append(initialSettings, Setting{ID: SettingHeaderTableSize, Val: maxHeaderTableSize})
+ }
cc.bw.Write(clientPreface)
cc.fr.WriteSettings(initialSettings...)
@@ -2774,8 +2806,10 @@
cc.cond.Broadcast()
cc.initialWindowSize = s.Val
+ case SettingHeaderTableSize:
+ cc.henc.SetMaxDynamicTableSize(s.Val)
+ cc.peerMaxHeaderTableSize = s.Val
default:
- // TODO(bradfitz): handle more settings? SETTINGS_HEADER_TABLE_SIZE probably.
cc.vlogf("Unhandled Setting: %v", s)
}
return nil
diff --git a/http2/transport_test.go b/http2/transport_test.go
index 67afb57..fbd1bfa 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -4193,6 +4193,150 @@
ct.run()
}
+func TestTransportMaxDecoderHeaderTableSize(t *testing.T) {
+ ct := newClientTester(t)
+ var reqSize, resSize uint32 = 8192, 16384
+ ct.tr.MaxDecoderHeaderTableSize = reqSize
+ ct.client = func() error {
+ req, _ := http.NewRequest("GET", "https://dummy.tld/", nil)
+ cc, err := ct.tr.NewClientConn(ct.cc)
+ if err != nil {
+ return err
+ }
+ _, err = cc.RoundTrip(req)
+ if err != nil {
+ return err
+ }
+ if got, want := cc.peerMaxHeaderTableSize, resSize; got != want {
+ return fmt.Errorf("peerHeaderTableSize = %d, want %d", got, want)
+ }
+ return nil
+ }
+ ct.server = func() error {
+ buf := make([]byte, len(ClientPreface))
+ _, err := io.ReadFull(ct.sc, buf)
+ if err != nil {
+ return fmt.Errorf("reading client preface: %v", err)
+ }
+ f, err := ct.fr.ReadFrame()
+ if err != nil {
+ return err
+ }
+ sf, ok := f.(*SettingsFrame)
+ if !ok {
+ ct.t.Fatalf("wanted client settings frame; got %v", f)
+ _ = sf // stash it away?
+ }
+ var found bool
+ err = sf.ForeachSetting(func(s Setting) error {
+ if s.ID == SettingHeaderTableSize {
+ found = true
+ if got, want := s.Val, reqSize; got != want {
+ return fmt.Errorf("received SETTINGS_HEADER_TABLE_SIZE = %d, want %d", got, want)
+ }
+ }
+ return nil
+ })
+ if err != nil {
+ return err
+ }
+ if !found {
+ return fmt.Errorf("missing SETTINGS_HEADER_TABLE_SIZE setting")
+ }
+ if err := ct.fr.WriteSettings(Setting{SettingHeaderTableSize, resSize}); err != nil {
+ ct.t.Fatal(err)
+ }
+ if err := ct.fr.WriteSettingsAck(); err != nil {
+ ct.t.Fatal(err)
+ }
+
+ for {
+ f, err := ct.fr.ReadFrame()
+ if err != nil {
+ return err
+ }
+ switch f := f.(type) {
+ case *HeadersFrame:
+ var buf bytes.Buffer
+ enc := hpack.NewEncoder(&buf)
+ enc.WriteField(hpack.HeaderField{Name: ":status", Value: "200"})
+ ct.fr.WriteHeaders(HeadersFrameParam{
+ StreamID: f.StreamID,
+ EndHeaders: true,
+ EndStream: true,
+ BlockFragment: buf.Bytes(),
+ })
+ return nil
+ }
+ }
+ }
+ ct.run()
+}
+
+func TestTransportMaxEncoderHeaderTableSize(t *testing.T) {
+ ct := newClientTester(t)
+ var peerAdvertisedMaxHeaderTableSize uint32 = 16384
+ ct.tr.MaxEncoderHeaderTableSize = 8192
+ ct.client = func() error {
+ req, _ := http.NewRequest("GET", "https://dummy.tld/", nil)
+ cc, err := ct.tr.NewClientConn(ct.cc)
+ if err != nil {
+ return err
+ }
+ _, err = cc.RoundTrip(req)
+ if err != nil {
+ return err
+ }
+ if got, want := cc.henc.MaxDynamicTableSize(), ct.tr.MaxEncoderHeaderTableSize; got != want {
+ return fmt.Errorf("henc.MaxDynamicTableSize() = %d, want %d", got, want)
+ }
+ return nil
+ }
+ ct.server = func() error {
+ buf := make([]byte, len(ClientPreface))
+ _, err := io.ReadFull(ct.sc, buf)
+ if err != nil {
+ return fmt.Errorf("reading client preface: %v", err)
+ }
+ f, err := ct.fr.ReadFrame()
+ if err != nil {
+ return err
+ }
+ sf, ok := f.(*SettingsFrame)
+ if !ok {
+ ct.t.Fatalf("wanted client settings frame; got %v", f)
+ _ = sf // stash it away?
+ }
+ if err := ct.fr.WriteSettings(Setting{SettingHeaderTableSize, peerAdvertisedMaxHeaderTableSize}); err != nil {
+ ct.t.Fatal(err)
+ }
+ if err := ct.fr.WriteSettingsAck(); err != nil {
+ ct.t.Fatal(err)
+ }
+
+ for {
+ f, err := ct.fr.ReadFrame()
+ if err != nil {
+ return err
+ }
+ switch f := f.(type) {
+ case *HeadersFrame:
+ var buf bytes.Buffer
+ enc := hpack.NewEncoder(&buf)
+ enc.WriteField(hpack.HeaderField{Name: ":status", Value: "200"})
+ ct.fr.WriteHeaders(HeadersFrameParam{
+ StreamID: f.StreamID,
+ EndHeaders: true,
+ EndStream: true,
+ BlockFragment: buf.Bytes(),
+ })
+ return nil
+ }
+ }
+ }
+ ct.run()
+}
+
func TestAuthorityAddr(t *testing.T) {
tests := []struct {
scheme, authority string
To view, visit change 435899. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Attention is currently required from: Eli, Tom Bergan.
2 comments:
Patchset:
Thanks for the change.
File http2/server.go:
Patch Set #1, Line 112:
MaxEncoderHeaderTableSize uint32I see that Brad pushed for splitting this into two knobs in https://go.dev/cl/155877, but I'm not really convinced this is necessary. Probably fine.
Do we need support for disabling the dynamic header table (SETTINGS_HEADER_TABLE_SIZE = 0)? If we want it in the future, how will we fit it in here?
What's the use case for permitting the user to customize the table size? This seems like a very fiddly setting; are we just exposing it because we can, or are there real-world cases where you want to set this?
Also, since this is an addition to the public API of a golang.org/x repo, it needs an accepted proposal: https://github.com/golang/proposal (Doesn't have to be a very large proposal, new http/2 knobs aren't very controversial.)
To view, visit change 435899. To unsubscribe, or for help writing mail filters, visit settings.
Eli (Gerrit)
Attention is currently required from: Damien Neil, Tom Bergan.
1 comment:
File http2/server.go:
Patch Set #1, Line 112:
MaxEncoderHeaderTableSize uint32
but I'm not really convinced this is necessary. Probably fine.
I think it could easily go either way, and I've gone back and forth with no strong opinion. Splitting mirrors the underlying protocol and makes each knob easier to explain, but I've always seen both of these set to the same value in practice.
Do we need support for disabling the dynamic header table (SETTINGS_HEADER_TABLE_SIZE = 0)? If we want it in the future, how will we fit it in here?
I'm inclined to never support it. Going below the default 4096 hits interoperability issues (see note in updated rfc https://www.rfc-editor.org/rfc/rfc9113#name-compression-state), and is obscure/fraught enough that while technically supported by the protocol, doesn't seem worth making the API more complex. If we do want it in the future, a "disable dynamic table" bool isn't elegant, but would make sense alongside the MaxHeaderTableSize settings.
There's a similar situation today with MaxConcurrentStreams, which technically does support a setting of 0, but that's not often seen and may cause issues with the stack at the other end of the connection, so not allowing it is no great loss.
(Interestingly QPACK defaults the dynamic table to 0, nicely sidestepping this problem of stacks not supporting shrinking below the default.)
What's the use case for permitting the user to customize the table size? This seems like a very fiddly setting; are we just exposing it because we can, or are there real-world cases where you want to set this?
The default 4096 works well for individual client-facing connections. The main real-world case where having this configurable matters is in reverse proxies/load balancers where disparate client requests with varied headers are being multiplexed onto a single http2 connection. In that scenario, 4096 is small, large headers will cause it to churn, and hpack compression/request latency ends up suffering. For this reason we've been running ~4 years with this patched to allow bumping both the encoder/decoder table size on our proxy's upstream connection pool to the max allowed by the protocol.
Also, since this is an addition to the public API of a golang.org/x repo, it needs an accepted proposal: https://github.com/golang/proposal (Doesn't have to be a very large proposal, new http/2 knobs aren't very controversial.)
Will do! I'll write that up tomorrow.
To view, visit change 435899. To unsubscribe, or for help writing mail filters, visit settings.
Eli (Gerrit)
File http2/server.go:
Patch Set #1, Line 112:
MaxEncoderHeaderTableSize uint32
> but I'm not really convinced this is necessary. Probably fine. […]
To view, visit change 435899. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Eli, Tom Bergan.
Patch set 1:Run-TryBot +1
1 comment:
File http2/server.go:
Patch Set #1, Line 112:
MaxEncoderHeaderTableSize uint32
Proposal https://github. […]
Thanks! Change looks good to me.
I just considered filing a proposal to preemptively approve all future knobs here configuring protocol settings, but I think this is actually the last setting we don't have a knob for (after https://go.dev/issue/54850 is approved) so probably not necessary.
To view, visit change 435899. To unsubscribe, or for help writing mail filters, visit settings.
Eli (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Damien Neil, Tom Bergan.
1 comment:
Patchset:
Accompany proposal has been accepted, so this change is now ready for review/open to feedback. 🙂
To view, visit change 435899. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Eli, Tom Bergan.
Patch set 1:Code-Review +2
2 comments:
Commit Message:
Patch Set #1, Line 20:
Fixes golang/go#29356Also:
Fixes golang/go#56054
Patchset:
Thanks!
To view, visit change 435899. To unsubscribe, or for help writing mail filters, visit settings.
Eli (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Damien Neil, Eli, Tom Bergan.
Eli uploaded patch set #2 to this change.
http2: add SETTINGS_HEADER_TABLE_SIZE support
Add support for handling of SETTINGS_HEADER_TABLESIZE in SETTINGS
frames.
Add http2.Transport.MaxDecoderHeaderTableSize to set the advertised
table size for new client connections. Add
http2.Transport.MaxEncoderHeaderTableSize to cap the accepted size for
new client connections.
Add http2.Server.MaxDecoderHeaderTableSize and MaxEncoderHeaderTableSize
to do the same on the server.
Fixes golang/go#29356
Fixes golang/go#56054
Change-Id: I16ae0f84b8527dc1e09dfce081e9f408fd514513
---
M http2/hpack/encode.go
M http2/server.go
M http2/server_test.go
M http2/transport.go
M http2/transport_test.go
5 files changed, 281 insertions(+), 12 deletions(-)
To view, visit change 435899. To unsubscribe, or for help writing mail filters, visit settings.
Eli (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Damien Neil, Tom Bergan.
1 comment:
Commit Message:
Patch Set #1, Line 20:
Fixes golang/go#29356
Also: […]
Thanks for catching!
To view, visit change 435899. To unsubscribe, or for help writing mail filters, visit settings.
Joedian Reid (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Damien Neil, Eli, Tom Bergan.
Patch set 2:Code-Review +1
Damien Neil (Gerrit)
Damien Neil submitted this change.
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
http2: add SETTINGS_HEADER_TABLE_SIZE support
Add support for handling of SETTINGS_HEADER_TABLESIZE in SETTINGS
frames.
Add http2.Transport.MaxDecoderHeaderTableSize to set the advertised
table size for new client connections. Add
http2.Transport.MaxEncoderHeaderTableSize to cap the accepted size for
new client connections.
Add http2.Server.MaxDecoderHeaderTableSize and MaxEncoderHeaderTableSize
to do the same on the server.
Fixes golang/go#29356
Fixes golang/go#56054
Change-Id: I16ae0f84b8527dc1e09dfce081e9f408fd514513
Reviewed-on: https://go-review.googlesource.com/c/net/+/435899
Reviewed-by: Damien Neil <dn...@google.com>
Reviewed-by: Joedian Reid <joe...@golang.org>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
---
M http2/hpack/encode.go
M http2/server.go
M http2/server_test.go
M http2/transport.go
M http2/transport_test.go
5 files changed, 286 insertions(+), 12 deletions(-)
diff --git a/http2/hpack/encode.go b/http2/hpack/encode.go
index 6886dc1..46219da 100644
--- a/http2/hpack/encode.go
+++ b/http2/hpack/encode.go
@@ -116,6 +116,11 @@
e.dynTab.setMaxSize(v)
}
+// MaxDynamicTableSize returns the current dynamic header table size.
+func (e *Encoder) MaxDynamicTableSize() (v uint32) {
+ return e.dynTab.maxSize
+}
+
// SetMaxDynamicTableSizeLimit changes the maximum value that can be
// specified in SetMaxDynamicTableSize to v. By default, it is set to
// 4096, which is the same size of the default dynamic header table
diff --git a/http2/server.go b/http2/server.go
index d8a17aa..e35a76c 100644@@ -864,6 +890,7 @@
{SettingMaxFrameSize, sc.srv.maxReadFrameSize()},
{SettingMaxConcurrentStreams, sc.advMaxStreams},
{SettingMaxHeaderListSize, sc.maxHeaderListSize()},
+ {SettingHeaderTableSize, sc.srv.maxDecoderHeaderTableSize()},
{SettingInitialWindowSize, uint32(sc.srv.initialStreamRecvWindowSize())},
},
})
@@ -1661,7 +1688,6 @@
}
switch s.ID {
case SettingHeaderTableSize:
- sc.headerTableSize = s.Val
sc.hpackEncoder.SetMaxDynamicTableSize(s.Val)
case SettingEnablePush:
sc.pushEnabled = s.Val != 0
diff --git a/http2/server_test.go b/http2/server_test.go
index 757bd29..3760871 100644
--- a/http2/server_test.go
+++ b/http2/server_test.go
@@ -2736,6 +2736,43 @@
index 46dda4d..91f4370 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -118,6 +118,19 @@
// to mean no limit.
MaxHeaderListSize uint32
+ // MaxDecoderHeaderTableSize optionally specifies the http2
+ // SETTINGS_HEADER_TABLE_SIZE to send in the initial settings frame. It
+ // informs the remote endpoint of the maximum size of the header compression
+ // table used to decode header blocks, in octets. If zero, the default value
+ // of 4096 is used.
+ MaxDecoderHeaderTableSize uint32
+
+ // MaxEncoderHeaderTableSize optionally specifies an upper limit for the
+ // header compression table used for encoding request headers. Received
+ // SETTINGS_HEADER_TABLE_SIZE settings are capped at this limit. If zero,
+ // the default value of 4096 is used.
+ MaxEncoderHeaderTableSize uint32
+
// StrictMaxConcurrentStreams controls whether the server's
// SETTINGS_MAX_CONCURRENT_STREAMS should be respected
// globally. If false, new TCP connections are created to the
@@ -293,10 +306,11 @@
lastActive time.Time
lastIdle time.Time // time last idle
// Settings from peer: (also guarded by wmu)
- maxFrameSize uint32
- maxConcurrentStreams uint32
- peerMaxHeaderListSize uint64
- initialWindowSize uint32
+ maxFrameSize uint32
+ maxConcurrentStreams uint32
+ peerMaxHeaderListSize uint64
+ peerMaxHeaderTableSize uint32
+ initialWindowSize uint32
// reqHeaderMu is a 1-element semaphore channel controlling access to sending new requests.
// Write to reqHeaderMu to lock it, read from it to unlock.
@@ -681,6 +695,20 @@
return t.t1.ExpectContinueTimeout
}
+func (t *Transport) maxDecoderHeaderTableSize() uint32 {
+ if v := t.MaxDecoderHeaderTableSize; v > 0 {
+ return v
+ }
+ return initialHeaderTableSize
+}
+
+func (t *Transport) maxEncoderHeaderTableSize() uint32 {
+ if v := t.MaxEncoderHeaderTableSize; v > 0 {
+ return v
+ }
+ return initialHeaderTableSize
+}
+
func (t *Transport) NewClientConn(c net.Conn) (*ClientConn, error) {
return t.newClientConn(c, t.disableKeepAlives())
}
@@ -724,12 +752,13 @@
if t.CountError != nil {
cc.fr.countError = t.CountError
}
- cc.fr.ReadMetaHeaders = hpack.NewDecoder(initialHeaderTableSize, nil)
+ maxHeaderTableSize := t.maxDecoderHeaderTableSize()
+ cc.fr.ReadMetaHeaders = hpack.NewDecoder(maxHeaderTableSize, nil)
cc.fr.MaxHeaderListSize = t.maxHeaderListSize()
- // TODO: SetMaxDynamicTableSize, SetMaxDynamicTableSizeLimit on
- // henc in response to SETTINGS frames?
cc.henc = hpack.NewEncoder(&cc.hbuf)
+ cc.henc.SetMaxDynamicTableSizeLimit(t.maxEncoderHeaderTableSize())
+ cc.peerMaxHeaderTableSize = initialHeaderTableSize
if t.AllowHTTP {
cc.nextStreamID = 3
@@ -747,6 +776,9 @@
if max := t.maxHeaderListSize(); max != 0 {
initialSettings = append(initialSettings, Setting{ID: SettingMaxHeaderListSize, Val: max})
}
+ if maxHeaderTableSize != initialHeaderTableSize {
+ initialSettings = append(initialSettings, Setting{ID: SettingHeaderTableSize, Val: maxHeaderTableSize})
+ }
cc.bw.Write(clientPreface)
cc.fr.WriteSettings(initialSettings...)
@@ -2773,8 +2805,10 @@
cc.cond.Broadcast()
cc.initialWindowSize = s.Val
+ case SettingHeaderTableSize:
+ cc.henc.SetMaxDynamicTableSize(s.Val)
+ cc.peerMaxHeaderTableSize = s.Val
default:
- // TODO(bradfitz): handle more settings? SETTINGS_HEADER_TABLE_SIZE probably.
cc.vlogf("Unhandled Setting: %v", s)
}
return nil
diff --git a/http2/transport_test.go b/http2/transport_test.go
index 9eaf7bf..ee852b6 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -4223,6 +4223,150 @@
To view, visit change 435899. To unsubscribe, or for help writing mail filters, visit settings.