Skip to content

Commit 2f8c3d1

Browse files
robahogopherbot
authored andcommitted
http2: add Transport.MaxReadFrameSize configuration setting
For golang/go#47840. Fixes golang/go#54850. Change-Id: I44efec8d1f223b3c2be82a2e11752fbbe8bf2cbf Reviewed-on: https://go-review.googlesource.com/c/net/+/362834 Run-TryBot: Damien Neil <dneil@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Joedian Reid <joedian@golang.org>
1 parent 0e478a2 commit 2f8c3d1

File tree

2 files changed

+141
-0
lines changed

2 files changed

+141
-0
lines changed

http2/transport.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,15 @@ type Transport struct {
118118
// to mean no limit.
119119
MaxHeaderListSize uint32
120120

121+
// MaxReadFrameSize is the http2 SETTINGS_MAX_FRAME_SIZE to send in the
122+
// initial settings frame. It is the size in bytes of the largest frame
123+
// payload that the sender is willing to receive. If 0, no setting is
124+
// sent, and the value is provided by the peer, which should be 16384
125+
// according to the spec:
126+
// https://datatracker.ietf.org/doc/html/rfc7540#section-6.5.2.
127+
// Values are bounded in the range 16k to 16M.
128+
MaxReadFrameSize uint32
129+
121130
// MaxDecoderHeaderTableSize optionally specifies the http2
122131
// SETTINGS_HEADER_TABLE_SIZE to send in the initial settings frame. It
123132
// informs the remote endpoint of the maximum size of the header compression
@@ -184,6 +193,19 @@ func (t *Transport) maxHeaderListSize() uint32 {
184193
return t.MaxHeaderListSize
185194
}
186195

196+
func (t *Transport) maxFrameReadSize() uint32 {
197+
if t.MaxReadFrameSize == 0 {
198+
return 0 // use the default provided by the peer
199+
}
200+
if t.MaxReadFrameSize < minMaxFrameSize {
201+
return minMaxFrameSize
202+
}
203+
if t.MaxReadFrameSize > maxFrameSize {
204+
return maxFrameSize
205+
}
206+
return t.MaxReadFrameSize
207+
}
208+
187209
func (t *Transport) disableCompression() bool {
188210
return t.DisableCompression || (t.t1 != nil && t.t1.DisableCompression)
189211
}
@@ -749,6 +771,9 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro
749771
})
750772
cc.br = bufio.NewReader(c)
751773
cc.fr = NewFramer(cc.bw, cc.br)
774+
if t.maxFrameReadSize() != 0 {
775+
cc.fr.SetMaxReadFrameSize(t.maxFrameReadSize())
776+
}
752777
if t.CountError != nil {
753778
cc.fr.countError = t.CountError
754779
}
@@ -773,6 +798,9 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro
773798
{ID: SettingEnablePush, Val: 0},
774799
{ID: SettingInitialWindowSize, Val: transportDefaultStreamFlow},
775800
}
801+
if max := t.maxFrameReadSize(); max != 0 {
802+
initialSettings = append(initialSettings, Setting{ID: SettingMaxFrameSize, Val: max})
803+
}
776804
if max := t.maxHeaderListSize(); max != 0 {
777805
initialSettings = append(initialSettings, Setting{ID: SettingMaxHeaderListSize, Val: max})
778806
}

http2/transport_test.go

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3998,6 +3998,64 @@ func TestTransportResponseDataBeforeHeaders(t *testing.T) {
39983998
ct.run()
39993999
}
40004000

4001+
// Test that the server received values are in range.
4002+
func TestTransportMaxFrameReadSize(t *testing.T) {
4003+
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
4004+
}, func(s *Server) {
4005+
s.MaxConcurrentStreams = 1
4006+
})
4007+
defer st.Close()
4008+
tr := &Transport{
4009+
TLSClientConfig: tlsConfigInsecure,
4010+
MaxReadFrameSize: 64000,
4011+
}
4012+
defer tr.CloseIdleConnections()
4013+
4014+
req, err := http.NewRequest("GET", st.ts.URL, nil)
4015+
if err != nil {
4016+
t.Fatal(err)
4017+
}
4018+
res, err := tr.RoundTrip(req)
4019+
if err != nil {
4020+
t.Fatal(err)
4021+
}
4022+
if got, want := res.StatusCode, 200; got != want {
4023+
t.Errorf("StatusCode = %v; want %v", got, want)
4024+
}
4025+
if res != nil && res.Body != nil {
4026+
res.Body.Close()
4027+
}
4028+
// Test that maxFrameReadSize() matches the requested size.
4029+
if got, want := tr.maxFrameReadSize(), uint32(64000); got != want {
4030+
t.Errorf("maxFrameReadSize = %v; want %v", got, want)
4031+
}
4032+
// Test that server receives the requested size.
4033+
if got, want := st.sc.maxFrameSize, int32(64000); got != want {
4034+
t.Errorf("maxFrameReadSize = %v; want %v", got, want)
4035+
}
4036+
4037+
// test for minimum frame read size
4038+
tr = &Transport{
4039+
TLSClientConfig: tlsConfigInsecure,
4040+
MaxReadFrameSize: 1024,
4041+
}
4042+
4043+
if got, want := tr.maxFrameReadSize(), uint32(minMaxFrameSize); got != want {
4044+
t.Errorf("maxFrameReadSize = %v; want %v", got, want)
4045+
}
4046+
4047+
// test for maximum frame size
4048+
tr = &Transport{
4049+
TLSClientConfig: tlsConfigInsecure,
4050+
MaxReadFrameSize: 1024 * 1024 * 16,
4051+
}
4052+
4053+
if got, want := tr.maxFrameReadSize(), uint32(maxFrameSize); got != want {
4054+
t.Errorf("maxFrameReadSize = %v; want %v", got, want)
4055+
}
4056+
4057+
}
4058+
40014059
func TestTransportRequestsLowServerLimit(t *testing.T) {
40024060
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
40034061
}, optOnlyServer, func(s *Server) {
@@ -4608,6 +4666,61 @@ func BenchmarkClientResponseHeaders(b *testing.B) {
46084666
b.Run("1000 Headers", func(b *testing.B) { benchSimpleRoundTrip(b, 0, 1000) })
46094667
}
46104668

4669+
func BenchmarkDownloadFrameSize(b *testing.B) {
4670+
b.Run(" 16k Frame", func(b *testing.B) { benchLargeDownloadRoundTrip(b, 16*1024) })
4671+
b.Run(" 64k Frame", func(b *testing.B) { benchLargeDownloadRoundTrip(b, 64*1024) })
4672+
b.Run("128k Frame", func(b *testing.B) { benchLargeDownloadRoundTrip(b, 128*1024) })
4673+
b.Run("256k Frame", func(b *testing.B) { benchLargeDownloadRoundTrip(b, 256*1024) })
4674+
b.Run("512k Frame", func(b *testing.B) { benchLargeDownloadRoundTrip(b, 512*1024) })
4675+
}
4676+
func benchLargeDownloadRoundTrip(b *testing.B, frameSize uint32) {
4677+
defer disableGoroutineTracking()()
4678+
const transferSize = 1024 * 1024 * 1024 // must be multiple of 1M
4679+
b.ReportAllocs()
4680+
st := newServerTester(b,
4681+
func(w http.ResponseWriter, r *http.Request) {
4682+
// test 1GB transfer
4683+
w.Header().Set("Content-Length", strconv.Itoa(transferSize))
4684+
w.Header().Set("Content-Transfer-Encoding", "binary")
4685+
var data [1024 * 1024]byte
4686+
for i := 0; i < transferSize/(1024*1024); i++ {
4687+
w.Write(data[:])
4688+
}
4689+
}, optQuiet,
4690+
)
4691+
defer st.Close()
4692+
4693+
tr := &Transport{TLSClientConfig: tlsConfigInsecure, MaxReadFrameSize: frameSize}
4694+
defer tr.CloseIdleConnections()
4695+
4696+
req, err := http.NewRequest("GET", st.ts.URL, nil)
4697+
if err != nil {
4698+
b.Fatal(err)
4699+
}
4700+
4701+
b.N = 3
4702+
b.SetBytes(transferSize)
4703+
b.ResetTimer()
4704+
4705+
for i := 0; i < b.N; i++ {
4706+
res, err := tr.RoundTrip(req)
4707+
if err != nil {
4708+
if res != nil {
4709+
res.Body.Close()
4710+
}
4711+
b.Fatalf("RoundTrip err = %v; want nil", err)
4712+
}
4713+
data, _ := io.ReadAll(res.Body)
4714+
if len(data) != transferSize {
4715+
b.Fatalf("Response length invalid")
4716+
}
4717+
res.Body.Close()
4718+
if res.StatusCode != http.StatusOK {
4719+
b.Fatalf("Response code = %v; want %v", res.StatusCode, http.StatusOK)
4720+
}
4721+
}
4722+
}
4723+
46114724
func activeStreams(cc *ClientConn) int {
46124725
count := 0
46134726
cc.mu.Lock()

0 commit comments

Comments
 (0)