src/net/http/internal/http2/server.go | 20 +++++++++++++++++++- src/net/http/internal/http2/server_test.go | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++ diff --git a/src/net/http/internal/http2/server.go b/src/net/http/internal/http2/server.go index c51bc590897f43f9a08e16f9cb29b2c083e69326..6a0311adbc5984b08f9f2b92a6b2a5271d36e8bd 100644 --- a/src/net/http/internal/http2/server.go +++ b/src/net/http/internal/http2/server.go @@ -431,6 +431,9 @@ sawClientPreface bool // preface has already been read, used in h2c upgrade sawFirstSettings bool // got the initial SETTINGS frame after the preface needToSendSettingsAck bool unackedSettings int // how many SETTINGS have we sent without ACKs? + pendingEncoderTableSize bool // peer changed SETTINGS_HEADER_TABLE_SIZE; apply to hpackEncoder before the next frame write + encoderTableSizeMin uint32 // smallest SETTINGS_HEADER_TABLE_SIZE since the last apply + encoderTableSize uint32 // latest SETTINGS_HEADER_TABLE_SIZE queuedControlFrames int // control frames in the writeSched queue clientMaxStreams uint32 // SETTINGS_MAX_CONCURRENT_STREAMS from client (our PUSH_PROMISE limit) advMaxStreams uint32 // our SETTINGS_MAX_CONCURRENT_STREAMS advertised the client @@ -1160,6 +1163,16 @@ } sc.writingFrame = true sc.needsFrameFlush = true + if sc.pendingEncoderTableSize { + // hpackEncoder may be in use by writeFrameAsync, so SETTINGS + // changes to it are deferred until no frame is being written. + // Replaying the smallest size before the latest one keeps the + // encoder's view identical to having applied every change + // (RFC 7541, Section 4.2). + sc.pendingEncoderTableSize = false + sc.hpackEncoder.SetMaxDynamicTableSize(sc.encoderTableSizeMin) + sc.hpackEncoder.SetMaxDynamicTableSize(sc.encoderTableSize) + } if wr.write.staysWithinBuffer(sc.bw.Available()) { sc.writingFrameAsync = false err := wr.write.writeFrame(sc) @@ -1638,7 +1651,12 @@ sc.vlogf("http2: server processing setting %v", s) } switch s.ID { case SettingHeaderTableSize: - sc.hpackEncoder.SetMaxDynamicTableSize(s.Val) + // Applied by startFrameWrite; see comment there. + if !sc.pendingEncoderTableSize || s.Val < sc.encoderTableSizeMin { + sc.encoderTableSizeMin = s.Val + } + sc.encoderTableSize = s.Val + sc.pendingEncoderTableSize = true case SettingEnablePush: sc.pushEnabled = s.Val != 0 case SettingMaxConcurrentStreams: diff --git a/src/net/http/internal/http2/server_test.go b/src/net/http/internal/http2/server_test.go index fb2b84e04663bd0422006d1e5dbe425f7907df04..18bab6a938ab3d46b21b15178cd3c3c87f3ff983 100644 --- a/src/net/http/internal/http2/server_test.go +++ b/src/net/http/internal/http2/server_test.go @@ -2817,6 +2817,64 @@ t.Errorf("server encoder is using a header table size of %d, want %d", got, want) } } +// TestServer_HeaderTableSizeDuringWrite tests that a +// SETTINGS_HEADER_TABLE_SIZE change from the client is not applied to the +// server's HPACK encoder while a frame write, which may be using the encoder +// on another goroutine, is in progress. +func TestServer_HeaderTableSizeDuringWrite(t *testing.T) { + synctest.Test(t, testServer_HeaderTableSizeDuringWrite) +} +func testServer_HeaderTableSizeDuringWrite(t *testing.T) { + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {}) + defer st.Close() + st.greet() + enc := st.sc.TestHPACKEncoder() + + // Leave the response HEADERS write for stream 1 in progress while the + // client changes SETTINGS_HEADER_TABLE_SIZE twice. + st.cc.(*tls.Conn).NetConn().(*synctestNetConn).SetReadBufferSize(0) + st.bodylessReq1() + synctest.Wait() + st.writeSettings(Setting{SettingHeaderTableSize, 0}) + st.writeSettings(Setting{SettingHeaderTableSize, 2048}) + synctest.Wait() + // Table size should not be changed immediately. To avoid concurrent use of + // the encoder, we apply the size change right before we use the encoder to + // write frames. + if got, want := enc.MaxDynamicTableSize(), uint32(InitialHeaderTableSize); got != want { + t.Errorf("during frame write: encoder header table size = %d, want %d", got, want) + } + st.cc.(*tls.Conn).NetConn().(*synctestNetConn).SetReadBufferSize(math.MaxInt) + st.wantHeaders(wantHeader{streamID: 1, endStream: true}) + st.wantSettingsAck() + + st.writeHeaders(HeadersFrameParam{ + StreamID: 3, + BlockFragment: st.encodeHeader(), + EndStream: true, + EndHeaders: true, + }) + synctest.Wait() + hf := readFrame[*HeadersFrame](t, st) + if hf.StreamID != 3 { + t.Fatalf("got HEADERS for stream %d, want stream 3", hf.StreamID) + } + // The response must signal both the smallest (0) and the final (2048) + // table size (RFC 7541, Section 4.2), just like an encoder that saw both + // size changes directly. + var want bytes.Buffer + wantEnc := hpack.NewEncoder(&want) + wantEnc.SetMaxDynamicTableSize(0) + wantEnc.SetMaxDynamicTableSize(2048) + wantEnc.WriteField(hpack.HeaderField{Name: ":status", Value: "200"}) + if got := hf.HeaderBlockFragment(); !bytes.HasPrefix(got, want.Bytes()) { + t.Errorf("stream 3 header block = %x, want prefix %x", got, want.Bytes()) + } + if got, want := enc.MaxDynamicTableSize(), uint32(2048); got != want { + t.Errorf("after frame write: encoder header table size = %d, want %d", got, want) + } +} + // Issue 12843 func TestServerDoS_MaxHeaderListSize(t *testing.T) { synctest.Test(t, testServerDoS_MaxHeaderListSize) } func testServerDoS_MaxHeaderListSize(t *testing.T) {