From 3c333c0c5288a7cf127e427ddda5b1b54020a2b4 Mon Sep 17 00:00:00 2001 From: Carlos Hernandez Date: Fri, 6 Sep 2024 00:19:28 +0000 Subject: [PATCH 1/6] route: fix address parsing of messages on Darwin sizeofSockaddrInet is 16, but first byte of sockaddr specifies actual size of sockaddr. Although, 16 works for most cases, it fails for Netmasks addresses. On Darwin only the significant bits of the netmask are in the msg. Take this route message as an example ``` // rt_msg_hdr 88 00 05 01 00 00 00 00 41 08 00 00 07 00 00 00 92 7b 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 // metrics 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 // SOCKADDRS - DST (100.113.0.0) 10 02 00 00 64 71 00 00 00 00 00 00 00 00 00 00 // GW utun4319 14 12 21 00 01 08 00 00 75 74 75 6e 34 33 31 39 00 00 00 00 // NETMASK 255.255.0.0 06 02 00 00 ff ff // NULL 00 00 ``` i.e. ipv4 ``` 06 02 00 00 ff ff ``` The above byte sequence is for a sockaddr that is 6 bytes long representing an ipv4 for address that is 255.255.0.0. i.e. ipv6 netmask ``` 0e 1e 00 00 00 00 00 00 ff ff ff ff ff ff 00 00 ``` The above is `/48` netmask that should also be parsed using `b[0]` of the sockaddr that contains the length. Confirmed by using `route monitor`. sources: https://github.com/apple/darwin-xnu/blob/main/bsd/net/route.h https://github.com/apple/darwin-xnu/blob/main/bsd/sys/socket.h#L603 Fixes golang/go#44740 Change-Id: I8153130d02d0a5e547fbf60a85762d3889e1d08c GitHub-Last-Rev: f7b9253061ab7daea8c04833f30bb60699c205f1 GitHub-Pull-Request: golang/net#220 Reviewed-on: https://go-review.googlesource.com/c/net/+/609577 LUCI-TryBot-Result: Go LUCI Reviewed-by: Ian Lance Taylor Reviewed-by: Dmitri Shuralyov Auto-Submit: Ian Lance Taylor --- route/address.go | 27 ++++++++++++++---- route/address_darwin_test.go | 54 ++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/route/address.go b/route/address.go index 5443d6722..bae63003f 100644 --- a/route/address.go +++ b/route/address.go @@ -170,20 +170,37 @@ func (a *Inet6Addr) marshal(b []byte) (int, error) { // parseInetAddr parses b as an internet address for IPv4 or IPv6. func parseInetAddr(af int, b []byte) (Addr, error) { + const ( + off4 = 4 // offset of in_addr + off6 = 8 // offset of in6_addr + ) switch af { case syscall.AF_INET: - if len(b) < sizeofSockaddrInet { + if len(b) < (off4+1) || len(b) < int(b[0]) { return nil, errInvalidAddr } + sockAddrLen := int(b[0]) a := &Inet4Addr{} - copy(a.IP[:], b[4:8]) + n := off4 + 4 + if sockAddrLen < n { + n = sockAddrLen + } + copy(a.IP[:], b[off4:n]) return a, nil case syscall.AF_INET6: - if len(b) < sizeofSockaddrInet6 { + if len(b) < (off6+1) || len(b) < int(b[0]) { return nil, errInvalidAddr } - a := &Inet6Addr{ZoneID: int(nativeEndian.Uint32(b[24:28]))} - copy(a.IP[:], b[8:24]) + sockAddrLen := int(b[0]) + n := off6 + 16 + if sockAddrLen < n { + n = sockAddrLen + } + a := &Inet6Addr{} + if sockAddrLen == sizeofSockaddrInet6 { + a.ZoneID = int(nativeEndian.Uint32(b[24:28])) + } + copy(a.IP[:], b[off6:n]) if a.IP[0] == 0xfe && a.IP[1]&0xc0 == 0x80 || a.IP[0] == 0xff && (a.IP[1]&0x0f == 0x01 || a.IP[1]&0x0f == 0x02) { // KAME based IPv6 protocol stack usually // embeds the interface index in the diff --git a/route/address_darwin_test.go b/route/address_darwin_test.go index b819183a7..0b5c72d1d 100644 --- a/route/address_darwin_test.go +++ b/route/address_darwin_test.go @@ -42,6 +42,60 @@ var parseAddrsOnDarwinLittleEndianTests = []parseAddrsOnDarwinTest{ nil, }, }, + { + syscall.RTA_DST | syscall.RTA_GATEWAY | syscall.RTA_NETMASK, + parseKernelInetAddr, + []byte{ + 0x10, 0x02, 0x00, 0x00, 0x64, 0x71, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + + 0x14, 0x12, 0x21, 0x00, 0x01, 0x08, 0x00, 0x00, + 0x75, 0x74, 0x75, 0x6e, 0x34, 0x33, 0x31, 0x39, + 0x00, 0x00, 0x00, 0x00, + + 0x06, 0x02, 0x00, 0x00, 0xff, 0xff, 0x00, 0x00, + }, + []Addr{ + &Inet4Addr{IP: [4]byte{100, 113, 0, 0}}, + &LinkAddr{Index: 33, Name: "utun4319"}, + &Inet4Addr{IP: [4]byte{255, 255, 0, 0}}, + nil, + nil, + nil, + nil, + nil, + }, + }, + // route -n add -inet6 fd84:1b4e:6281:: -prefixlen 48 fe80::f22f:4bff:fe09:3bff%utun4319 + // gw fe80:0000:0000:0000:f22f:4bff:fe09:3bff + { + syscall.RTA_DST | syscall.RTA_GATEWAY | syscall.RTA_NETMASK, + parseKernelInetAddr, + []byte{ + 0x1c, 0x1e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xfd, 0x84, 0x1b, 0x4e, 0x62, 0x81, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + 0x1c, 0x1e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xfe, 0x80, 0x00, 0x21, 0x00, 0x00, 0x00, 0x00, + 0xf2, 0x2f, 0x4b, 0xff, 0xfe, 0x09, 0x3b, 0xff, + 0x00, 0x00, 0x00, 0x00, + + 0x0e, 0x1e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, + }, + []Addr{ + &Inet6Addr{IP: [16]byte{ 0xfd, 0x84, 0x1b, 0x4e, 0x62, 0x81 }}, + &Inet6Addr{IP: [16]byte{ 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xf2, 0x2f, 0x4b, 0xff, 0xfe, 0x09, 0x3b, 0xff }, ZoneID: 33}, + &Inet6Addr{IP: [16]byte{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,}}, + nil, + nil, + nil, + nil, + nil, + }, + }, } func TestParseAddrsOnDarwin(t *testing.T) { From 541dbe58b6bc869fc1c7de361846682a34365325 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Thu, 9 May 2024 10:24:28 -0700 Subject: [PATCH 2/6] http2: add Server.WriteByteTimeout Transports support a WriteByteTimeout option which sets the maximum amount of time we can go without being able to write any bytes to a connection. Add an equivalent option to Server for consistency. Fixes golang/go#61777 Change-Id: Iaa8a69dfc403906eb224829320f901e5a6a5c429 Reviewed-on: https://go-review.googlesource.com/c/net/+/601496 LUCI-TryBot-Result: Go LUCI Reviewed-by: Carlos Amedee Reviewed-by: Brad Fitzpatrick --- http2/connframes_test.go | 5 ++-- http2/http2.go | 53 ++++++++++++++++++++++++++++++++++------ http2/server.go | 12 ++++++++- http2/server_test.go | 32 ++++++++++++++++++++++++ http2/transport.go | 24 ++++++------------ 5 files changed, 98 insertions(+), 28 deletions(-) diff --git a/http2/connframes_test.go b/http2/connframes_test.go index 7db8b74e2..1f7834c01 100644 --- a/http2/connframes_test.go +++ b/http2/connframes_test.go @@ -6,7 +6,6 @@ package http2 import ( "bytes" - "context" "io" "net/http" "os" @@ -295,7 +294,7 @@ func (tf *testConnFramer) wantClosed() { if err == nil { tf.t.Fatalf("got unexpected frame (want closed connection): %v", fr) } - if err == context.DeadlineExceeded { + if err == os.ErrDeadlineExceeded { tf.t.Fatalf("connection is not closed; want it to be") } } @@ -306,7 +305,7 @@ func (tf *testConnFramer) wantIdle() { if err == nil { tf.t.Fatalf("got unexpected frame (want idle connection): %v", fr) } - if err != context.DeadlineExceeded { + if err != os.ErrDeadlineExceeded { tf.t.Fatalf("got unexpected frame error (want idle connection): %v", err) } } diff --git a/http2/http2.go b/http2/http2.go index 003e649f3..7688c356b 100644 --- a/http2/http2.go +++ b/http2/http2.go @@ -19,8 +19,9 @@ import ( "bufio" "context" "crypto/tls" + "errors" "fmt" - "io" + "net" "net/http" "os" "sort" @@ -237,13 +238,19 @@ func (cw closeWaiter) Wait() { // Its buffered writer is lazily allocated as needed, to minimize // idle memory usage with many connections. type bufferedWriter struct { - _ incomparable - w io.Writer // immutable - bw *bufio.Writer // non-nil when data is buffered + _ incomparable + group synctestGroupInterface // immutable + conn net.Conn // immutable + bw *bufio.Writer // non-nil when data is buffered + byteTimeout time.Duration // immutable, WriteByteTimeout } -func newBufferedWriter(w io.Writer) *bufferedWriter { - return &bufferedWriter{w: w} +func newBufferedWriter(group synctestGroupInterface, conn net.Conn, timeout time.Duration) *bufferedWriter { + return &bufferedWriter{ + group: group, + conn: conn, + byteTimeout: timeout, + } } // bufWriterPoolBufferSize is the size of bufio.Writer's @@ -270,7 +277,7 @@ func (w *bufferedWriter) Available() int { func (w *bufferedWriter) Write(p []byte) (n int, err error) { if w.bw == nil { bw := bufWriterPool.Get().(*bufio.Writer) - bw.Reset(w.w) + bw.Reset((*bufferedWriterTimeoutWriter)(w)) w.bw = bw } return w.bw.Write(p) @@ -288,6 +295,38 @@ func (w *bufferedWriter) Flush() error { return err } +type bufferedWriterTimeoutWriter bufferedWriter + +func (w *bufferedWriterTimeoutWriter) Write(p []byte) (n int, err error) { + return writeWithByteTimeout(w.group, w.conn, w.byteTimeout, p) +} + +// writeWithByteTimeout writes to conn. +// If more than timeout passes without any bytes being written to the connection, +// the write fails. +func writeWithByteTimeout(group synctestGroupInterface, conn net.Conn, timeout time.Duration, p []byte) (n int, err error) { + if timeout <= 0 { + return conn.Write(p) + } + for { + var now time.Time + if group == nil { + now = time.Now() + } else { + now = group.Now() + } + conn.SetWriteDeadline(now.Add(timeout)) + nn, err := conn.Write(p[n:]) + n += nn + if n == len(p) || nn == 0 || !errors.Is(err, os.ErrDeadlineExceeded) { + // Either we finished the write, made no progress, or hit the deadline. + // Whichever it is, we're done now. + conn.SetWriteDeadline(time.Time{}) + return n, err + } + } +} + func mustUint31(v int32) uint32 { if v < 0 || v > 2147483647 { panic("out of range") diff --git a/http2/server.go b/http2/server.go index 6c349f3ec..b16173c57 100644 --- a/http2/server.go +++ b/http2/server.go @@ -127,6 +127,12 @@ type Server struct { // If zero or negative, there is no timeout. IdleTimeout time.Duration + // WriteByteTimeout is the timeout after which a connection will be + // closed if no data can be written to it. The timeout begins when data is + // available to write, and is extended whenever any bytes are written. + // If zero or negative, there is no timeout. + WriteByteTimeout time.Duration + // MaxUploadBufferPerConnection is the size of the initial flow // control window for each connections. The HTTP/2 spec does not // allow this to be smaller than 65535 or larger than 2^32-1. @@ -446,7 +452,7 @@ func (s *Server) serveConn(c net.Conn, opts *ServeConnOpts, newf func(*serverCon conn: c, baseCtx: baseCtx, remoteAddrStr: c.RemoteAddr().String(), - bw: newBufferedWriter(c), + bw: newBufferedWriter(s.group, c, s.WriteByteTimeout), handler: opts.handler(), streams: make(map[uint32]*stream), readFrameCh: make(chan readFrameResult), @@ -1320,6 +1326,10 @@ func (sc *serverConn) wroteFrame(res frameWriteResult) { sc.writingFrame = false sc.writingFrameAsync = false + if res.err != nil { + sc.conn.Close() + } + wr := res.wr if writeEndsStream(wr.write) { diff --git a/http2/server_test.go b/http2/server_test.go index 47c3c619c..ab53c2694 100644 --- a/http2/server_test.go +++ b/http2/server_test.go @@ -4674,3 +4674,35 @@ func TestServerSetReadWriteDeadlineRace(t *testing.T) { } resp.Body.Close() } + +func TestServerWriteByteTimeout(t *testing.T) { + const timeout = 1 * time.Second + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { + w.Write(make([]byte, 100)) + }, func(s *Server) { + s.WriteByteTimeout = timeout + }) + st.greet() + + st.cc.(*synctestNetConn).SetReadBufferSize(1) // write one byte at a time + st.writeHeaders(HeadersFrameParam{ + StreamID: 1, + BlockFragment: st.encodeHeader(), + EndStream: true, + EndHeaders: true, + }) + + // Read a few bytes, staying just under WriteByteTimeout. + for i := 0; i < 10; i++ { + st.advance(timeout - 1) + if n, err := st.cc.Read(make([]byte, 1)); n != 1 || err != nil { + t.Fatalf("read %v: %v, %v; want 1, nil", i, n, err) + } + } + + // Wait for WriteByteTimeout. + // The connection should close. + st.advance(1 * time.Second) // timeout after writing one byte + st.advance(1 * time.Second) // timeout after failing to write any more bytes + st.wantClosed() +} diff --git a/http2/transport.go b/http2/transport.go index 61f511f97..49fc792ad 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -25,7 +25,6 @@ import ( "net/http" "net/http/httptrace" "net/textproto" - "os" "sort" "strconv" "strings" @@ -499,6 +498,7 @@ func (cs *clientStream) closeReqBodyLocked() { } type stickyErrWriter struct { + group synctestGroupInterface conn net.Conn timeout time.Duration err *error @@ -508,22 +508,9 @@ func (sew stickyErrWriter) Write(p []byte) (n int, err error) { if *sew.err != nil { return 0, *sew.err } - for { - if sew.timeout != 0 { - sew.conn.SetWriteDeadline(time.Now().Add(sew.timeout)) - } - nn, err := sew.conn.Write(p[n:]) - n += nn - if n < len(p) && nn > 0 && errors.Is(err, os.ErrDeadlineExceeded) { - // Keep extending the deadline so long as we're making progress. - continue - } - if sew.timeout != 0 { - sew.conn.SetWriteDeadline(time.Time{}) - } - *sew.err = err - return n, err - } + n, err = writeWithByteTimeout(sew.group, sew.conn, sew.timeout, p) + *sew.err = err + return n, err } // noCachedConnError is the concrete type of ErrNoCachedConn, which @@ -792,10 +779,12 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro pings: make(map[[8]byte]chan struct{}), reqHeaderMu: make(chan struct{}, 1), } + var group synctestGroupInterface if t.transportTestHooks != nil { t.markNewGoroutine() t.transportTestHooks.newclientconn(cc) c = cc.tconn + group = t.group } if VerboseLogs { t.vlogf("http2: Transport creating client conn %p to %v", cc, c.RemoteAddr()) @@ -807,6 +796,7 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro // TODO: adjust this writer size to account for frame size + // MTU + crypto/tls record padding. cc.bw = bufio.NewWriter(stickyErrWriter{ + group: group, conn: c, timeout: t.WriteByteTimeout, err: &cc.werr, From 4790dc7047441aed4889873cdd30e1e6adf49735 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 28 May 2024 14:11:26 -0700 Subject: [PATCH 3/6] http2: add support for server-originated pings Add configurable support for health-checking idle connections accepted by the HTTP/2 server, following the same configuration as the Transport. Fixes golang/go#67812 Change-Id: Ia4014e691546b2c29db8dad3af5f39966d0ceb93 Reviewed-on: https://go-review.googlesource.com/c/net/+/601497 Reviewed-by: Carlos Amedee Reviewed-by: Brad Fitzpatrick LUCI-TryBot-Result: Go LUCI --- http2/server.go | 60 ++++++++++++++++++++++++++++++++++++++++++++ http2/server_test.go | 43 +++++++++++++++++++++++++++++++ http2/write.go | 10 ++++++++ 3 files changed, 113 insertions(+) diff --git a/http2/server.go b/http2/server.go index b16173c57..645c8da69 100644 --- a/http2/server.go +++ b/http2/server.go @@ -29,6 +29,7 @@ import ( "bufio" "bytes" "context" + "crypto/rand" "crypto/tls" "errors" "fmt" @@ -127,6 +128,16 @@ type Server struct { // If zero or negative, there is no timeout. IdleTimeout time.Duration + // ReadIdleTimeout is the timeout after which a health check using a ping + // frame will be carried out if no frame is received on the connection. + // If zero, no health check is performed. + ReadIdleTimeout time.Duration + + // PingTimeout is the timeout after which the connection will be closed + // if a response to a ping is not received. + // If zero, a default of 15 seconds is used. + PingTimeout time.Duration + // WriteByteTimeout is the timeout after which a connection will be // closed if no data can be written to it. The timeout begins when data is // available to write, and is extended whenever any bytes are written. @@ -644,9 +655,12 @@ type serverConn struct { inGoAway bool // we've started to or sent GOAWAY inFrameScheduleLoop bool // whether we're in the scheduleFrameWrite loop needToSendGoAway bool // we need to schedule a GOAWAY frame write + pingSent bool + sentPingData [8]byte goAwayCode ErrCode shutdownTimer timer // nil until used idleTimer timer // nil if unused + readIdleTimer timer // nil if unused // Owned by the writeFrameAsync goroutine: headerWriteBuf bytes.Buffer @@ -974,11 +988,17 @@ func (sc *serverConn) serve() { defer sc.idleTimer.Stop() } + if sc.srv.ReadIdleTimeout > 0 { + sc.readIdleTimer = sc.srv.afterFunc(sc.srv.ReadIdleTimeout, sc.onReadIdleTimer) + defer sc.readIdleTimer.Stop() + } + go sc.readFrames() // closed by defer sc.conn.Close above settingsTimer := sc.srv.afterFunc(firstSettingsTimeout, sc.onSettingsTimer) defer settingsTimer.Stop() + lastFrameTime := sc.srv.now() loopNum := 0 for { loopNum++ @@ -992,6 +1012,7 @@ func (sc *serverConn) serve() { case res := <-sc.wroteFrameCh: sc.wroteFrame(res) case res := <-sc.readFrameCh: + lastFrameTime = sc.srv.now() // Process any written frames before reading new frames from the client since a // written frame could have triggered a new stream to be started. if sc.writingFrameAsync { @@ -1023,6 +1044,8 @@ func (sc *serverConn) serve() { case idleTimerMsg: sc.vlogf("connection is idle") sc.goAway(ErrCodeNo) + case readIdleTimerMsg: + sc.handlePingTimer(lastFrameTime) case shutdownTimerMsg: sc.vlogf("GOAWAY close timer fired; closing conn from %v", sc.conn.RemoteAddr()) return @@ -1061,12 +1084,43 @@ func (sc *serverConn) serve() { } } +func (sc *serverConn) handlePingTimer(lastFrameReadTime time.Time) { + if sc.pingSent { + sc.vlogf("timeout waiting for PING response") + sc.conn.Close() + return + } + + pingAt := lastFrameReadTime.Add(sc.srv.ReadIdleTimeout) + now := sc.srv.now() + if pingAt.After(now) { + // We received frames since arming the ping timer. + // Reset it for the next possible timeout. + sc.readIdleTimer.Reset(pingAt.Sub(now)) + return + } + + sc.pingSent = true + // Ignore crypto/rand.Read errors: It generally can't fail, and worse case if it does + // is we send a PING frame containing 0s. + _, _ = rand.Read(sc.sentPingData[:]) + sc.writeFrame(FrameWriteRequest{ + write: &writePing{data: sc.sentPingData}, + }) + pingTimeout := sc.srv.PingTimeout + if pingTimeout <= 0 { + pingTimeout = 15 * time.Second + } + sc.readIdleTimer.Reset(pingTimeout) +} + type serverMessage int // Message values sent to serveMsgCh. var ( settingsTimerMsg = new(serverMessage) idleTimerMsg = new(serverMessage) + readIdleTimerMsg = new(serverMessage) shutdownTimerMsg = new(serverMessage) gracefulShutdownMsg = new(serverMessage) handlerDoneMsg = new(serverMessage) @@ -1074,6 +1128,7 @@ var ( func (sc *serverConn) onSettingsTimer() { sc.sendServeMsg(settingsTimerMsg) } func (sc *serverConn) onIdleTimer() { sc.sendServeMsg(idleTimerMsg) } +func (sc *serverConn) onReadIdleTimer() { sc.sendServeMsg(readIdleTimerMsg) } func (sc *serverConn) onShutdownTimer() { sc.sendServeMsg(shutdownTimerMsg) } func (sc *serverConn) sendServeMsg(msg interface{}) { @@ -1604,6 +1659,11 @@ func (sc *serverConn) processFrame(f Frame) error { func (sc *serverConn) processPing(f *PingFrame) error { sc.serveG.check() if f.IsAck() { + if sc.pingSent && sc.sentPingData == f.Data { + // This is a response to a PING we sent. + sc.pingSent = false + sc.readIdleTimer.Reset(sc.srv.ReadIdleTimeout) + } // 6.7 PING: " An endpoint MUST NOT respond to PING frames // containing this flag." return nil diff --git a/http2/server_test.go b/http2/server_test.go index ab53c2694..ec7ae043c 100644 --- a/http2/server_test.go +++ b/http2/server_test.go @@ -4706,3 +4706,46 @@ func TestServerWriteByteTimeout(t *testing.T) { st.advance(1 * time.Second) // timeout after failing to write any more bytes st.wantClosed() } + +func TestServerPingSent(t *testing.T) { + const readIdleTimeout = 15 * time.Second + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { + }, func(s *Server) { + s.ReadIdleTimeout = readIdleTimeout + }) + st.greet() + + st.wantIdle() + + st.advance(readIdleTimeout) + _ = readFrame[*PingFrame](t, st) + st.wantIdle() + + st.advance(14 * time.Second) + st.wantIdle() + st.advance(1 * time.Second) + st.wantClosed() +} + +func TestServerPingResponded(t *testing.T) { + const readIdleTimeout = 15 * time.Second + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { + }, func(s *Server) { + s.ReadIdleTimeout = readIdleTimeout + }) + st.greet() + + st.wantIdle() + + st.advance(readIdleTimeout) + pf := readFrame[*PingFrame](t, st) + st.wantIdle() + + st.advance(14 * time.Second) + st.wantIdle() + + st.writePing(true, pf.Data) + + st.advance(2 * time.Second) + st.wantIdle() +} diff --git a/http2/write.go b/http2/write.go index 33f61398a..6ff6bee7e 100644 --- a/http2/write.go +++ b/http2/write.go @@ -131,6 +131,16 @@ func (se StreamError) writeFrame(ctx writeContext) error { func (se StreamError) staysWithinBuffer(max int) bool { return frameHeaderLen+4 <= max } +type writePing struct { + data [8]byte +} + +func (w writePing) writeFrame(ctx writeContext) error { + return ctx.Framer().WritePing(false, w.data) +} + +func (w writePing) staysWithinBuffer(max int) bool { return frameHeaderLen+len(w.data) <= max } + type writePingAck struct{ pf *PingFrame } func (w writePingAck) writeFrame(ctx writeContext) error { From 7191757bc637cf79a7ece0546e33f903bf5e9709 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Mon, 12 Aug 2024 12:52:08 -0700 Subject: [PATCH 4/6] http2: add support for net/http HTTP2 config field For golang/go#67813 Change-Id: I6b7f857d6ed250ba8b09649730980a91b3e8d7e9 Reviewed-on: https://go-review.googlesource.com/c/net/+/607255 Reviewed-by: Brad Fitzpatrick LUCI-TryBot-Result: Go LUCI Reviewed-by: Jonathan Amsterdam --- http2/clientconn_test.go | 14 ++++- http2/config.go | 122 +++++++++++++++++++++++++++++++++++ http2/config_go124.go | 61 ++++++++++++++++++ http2/config_pre_go124.go | 16 +++++ http2/config_test.go | 95 ++++++++++++++++++++++++++++ http2/connframes_test.go | 18 ++++++ http2/server.go | 129 +++++++++++++------------------------- http2/server_test.go | 8 ++- http2/transport.go | 117 ++++++++++++++-------------------- 9 files changed, 416 insertions(+), 164 deletions(-) create mode 100644 http2/config.go create mode 100644 http2/config_go124.go create mode 100644 http2/config_pre_go124.go create mode 100644 http2/config_test.go diff --git a/http2/clientconn_test.go b/http2/clientconn_test.go index 553379099..92d630514 100644 --- a/http2/clientconn_test.go +++ b/http2/clientconn_test.go @@ -148,7 +148,7 @@ func (tc *testClientConn) readClientPreface() { } } -func newTestClientConn(t *testing.T, opts ...func(*Transport)) *testClientConn { +func newTestClientConn(t *testing.T, opts ...any) *testClientConn { t.Helper() tt := newTestTransport(t, opts...) @@ -486,7 +486,7 @@ type testTransport struct { ccs []*testClientConn } -func newTestTransport(t *testing.T, opts ...func(*Transport)) *testTransport { +func newTestTransport(t *testing.T, opts ...any) *testTransport { tt := &testTransport{ t: t, group: newSynctest(time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC)), @@ -495,7 +495,15 @@ func newTestTransport(t *testing.T, opts ...func(*Transport)) *testTransport { tr := &Transport{} for _, o := range opts { - o(tr) + switch o := o.(type) { + case func(*http.Transport): + if tr.t1 == nil { + tr.t1 = &http.Transport{} + } + o(tr.t1) + case func(*Transport): + o(tr) + } } tt.tr = tr diff --git a/http2/config.go b/http2/config.go new file mode 100644 index 000000000..de58dfb8d --- /dev/null +++ b/http2/config.go @@ -0,0 +1,122 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package http2 + +import ( + "math" + "net/http" + "time" +) + +// http2Config is a package-internal version of net/http.HTTP2Config. +// +// http.HTTP2Config was added in Go 1.24. +// When running with a version of net/http that includes HTTP2Config, +// we merge the configuration with the fields in Transport or Server +// to produce an http2Config. +// +// Zero valued fields in http2Config are interpreted as in the +// net/http.HTTPConfig documentation. +// +// Precedence order for reconciling configurations is: +// +// - Use the net/http.{Server,Transport}.HTTP2Config value, when non-zero. +// - Otherwise use the http2.{Server.Transport} value. +// - If the resulting value is zero or out of range, use a default. +type http2Config struct { + MaxConcurrentStreams uint32 + MaxDecoderHeaderTableSize uint32 + MaxEncoderHeaderTableSize uint32 + MaxReadFrameSize uint32 + MaxUploadBufferPerConnection int32 + MaxUploadBufferPerStream int32 + SendPingTimeout time.Duration + PingTimeout time.Duration + WriteByteTimeout time.Duration + PermitProhibitedCipherSuites bool + CountError func(errType string) +} + +// configFromServer merges configuration settings from +// net/http.Server.HTTP2Config and http2.Server. +func configFromServer(h1 *http.Server, h2 *Server) http2Config { + conf := http2Config{ + MaxConcurrentStreams: h2.MaxConcurrentStreams, + MaxEncoderHeaderTableSize: h2.MaxEncoderHeaderTableSize, + MaxDecoderHeaderTableSize: h2.MaxDecoderHeaderTableSize, + MaxReadFrameSize: h2.MaxReadFrameSize, + MaxUploadBufferPerConnection: h2.MaxUploadBufferPerConnection, + MaxUploadBufferPerStream: h2.MaxUploadBufferPerStream, + SendPingTimeout: h2.ReadIdleTimeout, + PingTimeout: h2.PingTimeout, + WriteByteTimeout: h2.WriteByteTimeout, + PermitProhibitedCipherSuites: h2.PermitProhibitedCipherSuites, + CountError: h2.CountError, + } + fillNetHTTPServerConfig(&conf, h1) + setConfigDefaults(&conf, true) + return conf +} + +// configFromServer merges configuration settings from h2 and h2.t1.HTTP2 +// (the net/http Transport). +func configFromTransport(h2 *Transport) http2Config { + conf := http2Config{ + MaxEncoderHeaderTableSize: h2.MaxEncoderHeaderTableSize, + MaxDecoderHeaderTableSize: h2.MaxDecoderHeaderTableSize, + MaxReadFrameSize: h2.MaxReadFrameSize, + SendPingTimeout: h2.ReadIdleTimeout, + PingTimeout: h2.PingTimeout, + WriteByteTimeout: h2.WriteByteTimeout, + } + + // Unlike most config fields, where out-of-range values revert to the default, + // Transport.MaxReadFrameSize clips. + if conf.MaxReadFrameSize < minMaxFrameSize { + conf.MaxReadFrameSize = minMaxFrameSize + } else if conf.MaxReadFrameSize > maxFrameSize { + conf.MaxReadFrameSize = maxFrameSize + } + + if h2.t1 != nil { + fillNetHTTPTransportConfig(&conf, h2.t1) + } + setConfigDefaults(&conf, false) + return conf +} + +func setDefault[T ~int | ~int32 | ~uint32 | ~int64](v *T, minval, maxval, defval T) { + if *v < minval || *v > maxval { + *v = defval + } +} + +func setConfigDefaults(conf *http2Config, server bool) { + setDefault(&conf.MaxConcurrentStreams, 1, math.MaxUint32, defaultMaxStreams) + setDefault(&conf.MaxEncoderHeaderTableSize, 1, math.MaxUint32, initialHeaderTableSize) + setDefault(&conf.MaxDecoderHeaderTableSize, 1, math.MaxUint32, initialHeaderTableSize) + if server { + setDefault(&conf.MaxUploadBufferPerConnection, initialWindowSize, math.MaxInt32, 1<<20) + } else { + setDefault(&conf.MaxUploadBufferPerConnection, initialWindowSize, math.MaxInt32, transportDefaultConnFlow) + } + if server { + setDefault(&conf.MaxUploadBufferPerStream, 1, math.MaxInt32, 1<<20) + } else { + setDefault(&conf.MaxUploadBufferPerStream, 1, math.MaxInt32, transportDefaultStreamFlow) + } + setDefault(&conf.MaxReadFrameSize, minMaxFrameSize, maxFrameSize, defaultMaxReadFrameSize) + setDefault(&conf.PingTimeout, 1, math.MaxInt64, 15*time.Second) +} + +// adjustHTTP1MaxHeaderSize converts a limit in bytes on the size of an HTTP/1 header +// to an HTTP/2 MAX_HEADER_LIST_SIZE value. +func adjustHTTP1MaxHeaderSize(n int64) int64 { + // http2's count is in a slightly different unit and includes 32 bytes per pair. + // So, take the net/http.Server value and pad it up a bit, assuming 10 headers. + const perFieldOverhead = 32 // per http2 spec + const typicalHeaders = 10 // conservative + return n + typicalHeaders*perFieldOverhead +} diff --git a/http2/config_go124.go b/http2/config_go124.go new file mode 100644 index 000000000..e3784123c --- /dev/null +++ b/http2/config_go124.go @@ -0,0 +1,61 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build go1.24 + +package http2 + +import "net/http" + +// fillNetHTTPServerConfig sets fields in conf from srv.HTTP2. +func fillNetHTTPServerConfig(conf *http2Config, srv *http.Server) { + fillNetHTTPConfig(conf, srv.HTTP2) +} + +// fillNetHTTPServerConfig sets fields in conf from tr.HTTP2. +func fillNetHTTPTransportConfig(conf *http2Config, tr *http.Transport) { + fillNetHTTPConfig(conf, tr.HTTP2) +} + +func fillNetHTTPConfig(conf *http2Config, h2 *http.HTTP2Config) { + if h2 == nil { + return + } + if h2.MaxConcurrentStreams != 0 { + conf.MaxConcurrentStreams = uint32(h2.MaxConcurrentStreams) + } + if h2.MaxEncoderHeaderTableSize != 0 { + conf.MaxEncoderHeaderTableSize = uint32(h2.MaxEncoderHeaderTableSize) + } + if h2.MaxDecoderHeaderTableSize != 0 { + conf.MaxDecoderHeaderTableSize = uint32(h2.MaxDecoderHeaderTableSize) + } + if h2.MaxConcurrentStreams != 0 { + conf.MaxConcurrentStreams = uint32(h2.MaxConcurrentStreams) + } + if h2.MaxReadFrameSize != 0 { + conf.MaxReadFrameSize = uint32(h2.MaxReadFrameSize) + } + if h2.MaxReceiveBufferPerConnection != 0 { + conf.MaxUploadBufferPerConnection = int32(h2.MaxReceiveBufferPerConnection) + } + if h2.MaxReceiveBufferPerStream != 0 { + conf.MaxUploadBufferPerStream = int32(h2.MaxReceiveBufferPerStream) + } + if h2.SendPingTimeout != 0 { + conf.SendPingTimeout = h2.SendPingTimeout + } + if h2.PingTimeout != 0 { + conf.PingTimeout = h2.PingTimeout + } + if h2.WriteByteTimeout != 0 { + conf.WriteByteTimeout = h2.WriteByteTimeout + } + if h2.PermitProhibitedCipherSuites { + conf.PermitProhibitedCipherSuites = true + } + if h2.CountError != nil { + conf.CountError = h2.CountError + } +} diff --git a/http2/config_pre_go124.go b/http2/config_pre_go124.go new file mode 100644 index 000000000..060fd6c64 --- /dev/null +++ b/http2/config_pre_go124.go @@ -0,0 +1,16 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build !go1.24 + +package http2 + +import "net/http" + +// Pre-Go 1.24 fallback. +// The Server.HTTP2 and Transport.HTTP2 config fields were added in Go 1.24. + +func fillNetHTTPServerConfig(conf *http2Config, srv *http.Server) {} + +func fillNetHTTPTransportConfig(conf *http2Config, tr *http.Transport) {} diff --git a/http2/config_test.go b/http2/config_test.go new file mode 100644 index 000000000..b8e7a7b04 --- /dev/null +++ b/http2/config_test.go @@ -0,0 +1,95 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build go1.24 + +package http2 + +import ( + "net/http" + "testing" + "time" +) + +func TestConfigServerSettings(t *testing.T) { + config := &http.HTTP2Config{ + MaxConcurrentStreams: 1, + MaxDecoderHeaderTableSize: 1<<20 + 2, + MaxEncoderHeaderTableSize: 1<<20 + 3, + MaxReadFrameSize: 1<<20 + 4, + MaxReceiveBufferPerConnection: 64<<10 + 5, + MaxReceiveBufferPerStream: 64<<10 + 6, + } + const maxHeaderBytes = 4096 + 7 + st := newServerTester(t, nil, func(s *http.Server) { + s.MaxHeaderBytes = maxHeaderBytes + s.HTTP2 = config + }) + st.writePreface() + st.writeSettings() + st.wantSettings(map[SettingID]uint32{ + SettingMaxConcurrentStreams: uint32(config.MaxConcurrentStreams), + SettingHeaderTableSize: uint32(config.MaxDecoderHeaderTableSize), + SettingInitialWindowSize: uint32(config.MaxReceiveBufferPerStream), + SettingMaxFrameSize: uint32(config.MaxReadFrameSize), + SettingMaxHeaderListSize: maxHeaderBytes + (32 * 10), + }) +} + +func TestConfigTransportSettings(t *testing.T) { + config := &http.HTTP2Config{ + MaxConcurrentStreams: 1, // ignored by Transport + MaxDecoderHeaderTableSize: 1<<20 + 2, + MaxEncoderHeaderTableSize: 1<<20 + 3, + MaxReadFrameSize: 1<<20 + 4, + MaxReceiveBufferPerConnection: 64<<10 + 5, + MaxReceiveBufferPerStream: 64<<10 + 6, + } + const maxHeaderBytes = 4096 + 7 + tc := newTestClientConn(t, func(tr *http.Transport) { + tr.HTTP2 = config + tr.MaxResponseHeaderBytes = maxHeaderBytes + }) + tc.wantSettings(map[SettingID]uint32{ + SettingHeaderTableSize: uint32(config.MaxDecoderHeaderTableSize), + SettingInitialWindowSize: uint32(config.MaxReceiveBufferPerStream), + SettingMaxFrameSize: uint32(config.MaxReadFrameSize), + SettingMaxHeaderListSize: maxHeaderBytes + (32 * 10), + }) + tc.wantWindowUpdate(0, uint32(config.MaxReceiveBufferPerConnection)) +} + +func TestConfigPingTimeoutServer(t *testing.T) { + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { + }, func(s *Server) { + s.ReadIdleTimeout = 2 * time.Second + s.PingTimeout = 3 * time.Second + }) + st.greet() + + st.advance(2 * time.Second) + _ = readFrame[*PingFrame](t, st) + st.advance(3 * time.Second) + st.wantClosed() +} + +func TestConfigPingTimeoutTransport(t *testing.T) { + tc := newTestClientConn(t, func(tr *Transport) { + tr.ReadIdleTimeout = 2 * time.Second + tr.PingTimeout = 3 * time.Second + }) + tc.greet() + + req, _ := http.NewRequest("GET", "https://dummy.tld/", nil) + rt := tc.roundTrip(req) + tc.wantFrameType(FrameHeaders) + + tc.advance(2 * time.Second) + tc.wantFrameType(FramePing) + tc.advance(3 * time.Second) + err := rt.err() + if err == nil { + t.Fatalf("expected connection to close") + } +} diff --git a/http2/connframes_test.go b/http2/connframes_test.go index 1f7834c01..2c4532571 100644 --- a/http2/connframes_test.go +++ b/http2/connframes_test.go @@ -261,6 +261,24 @@ func (tf *testConnFramer) wantRSTStream(streamID uint32, code ErrCode) { } } +func (tf *testConnFramer) wantSettings(want map[SettingID]uint32) { + fr := readFrame[*SettingsFrame](tf.t, tf) + if fr.Header().Flags.Has(FlagSettingsAck) { + tf.t.Errorf("got SETTINGS frame with ACK set, want no ACK") + } + for wantID, wantVal := range want { + gotVal, ok := fr.Value(wantID) + if !ok { + tf.t.Errorf("SETTINGS: %v is not set, want %v", wantID, wantVal) + } else if gotVal != wantVal { + tf.t.Errorf("SETTINGS: %v is %v, want %v", wantID, gotVal, wantVal) + } + } + if tf.t.Failed() { + tf.t.Fatalf("%v", fr) + } +} + func (tf *testConnFramer) wantSettingsAck() { tf.t.Helper() fr := readFrame[*SettingsFrame](tf.t, tf) diff --git a/http2/server.go b/http2/server.go index 645c8da69..617b4a476 100644 --- a/http2/server.go +++ b/http2/server.go @@ -53,10 +53,14 @@ import ( ) const ( - prefaceTimeout = 10 * time.Second - firstSettingsTimeout = 2 * time.Second // should be in-flight with preface anyway - handlerChunkWriteSize = 4 << 10 - defaultMaxStreams = 250 // TODO: make this 100 as the GFE seems to? + prefaceTimeout = 10 * time.Second + firstSettingsTimeout = 2 * time.Second // should be in-flight with preface anyway + handlerChunkWriteSize = 4 << 10 + defaultMaxStreams = 250 // TODO: make this 100 as the GFE seems to? + + // 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. maxQueuedControlFrames = 10000 ) @@ -206,57 +210,6 @@ func (s *Server) afterFunc(d time.Duration, f func()) timer { return timeTimer{time.AfterFunc(d, f)} } -func (s *Server) initialConnRecvWindowSize() int32 { - if s.MaxUploadBufferPerConnection >= initialWindowSize { - return s.MaxUploadBufferPerConnection - } - return 1 << 20 -} - -func (s *Server) initialStreamRecvWindowSize() int32 { - if s.MaxUploadBufferPerStream > 0 { - return s.MaxUploadBufferPerStream - } - return 1 << 20 -} - -func (s *Server) maxReadFrameSize() uint32 { - if v := s.MaxReadFrameSize; v >= minMaxFrameSize && v <= maxFrameSize { - return v - } - return defaultMaxReadFrameSize -} - -func (s *Server) maxConcurrentStreams() uint32 { - if v := s.MaxConcurrentStreams; v > 0 { - return v - } - 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. -func (s *Server) maxQueuedControlFrames() int { - // TODO: if anybody asks, add a Server field, and remember to define the - // behavior of negative values. - return maxQueuedControlFrames -} - type serverInternalState struct { mu sync.Mutex activeConns map[*serverConn]struct{} @@ -457,13 +410,15 @@ func (s *Server) serveConn(c net.Conn, opts *ServeConnOpts, newf func(*serverCon baseCtx, cancel := serverConnBaseContext(c, opts) defer cancel() + http1srv := opts.baseConfig() + conf := configFromServer(http1srv, s) sc := &serverConn{ srv: s, - hs: opts.baseConfig(), + hs: http1srv, conn: c, baseCtx: baseCtx, remoteAddrStr: c.RemoteAddr().String(), - bw: newBufferedWriter(s.group, c, s.WriteByteTimeout), + bw: newBufferedWriter(s.group, c, conf.WriteByteTimeout), handler: opts.handler(), streams: make(map[uint32]*stream), readFrameCh: make(chan readFrameResult), @@ -473,9 +428,12 @@ func (s *Server) serveConn(c net.Conn, opts *ServeConnOpts, newf func(*serverCon bodyReadCh: make(chan bodyReadMsg), // buffering doesn't matter either way doneServing: make(chan struct{}), clientMaxStreams: math.MaxUint32, // Section 6.5.2: "Initially, there is no limit to this value" - advMaxStreams: s.maxConcurrentStreams(), + advMaxStreams: conf.MaxConcurrentStreams, initialStreamSendWindowSize: initialWindowSize, + initialStreamRecvWindowSize: conf.MaxUploadBufferPerStream, maxFrameSize: initialMaxFrameSize, + pingTimeout: conf.PingTimeout, + countErrorFunc: conf.CountError, serveG: newGoroutineLock(), pushEnabled: true, sawClientPreface: opts.SawClientPreface, @@ -508,15 +466,15 @@ func (s *Server) serveConn(c net.Conn, opts *ServeConnOpts, newf func(*serverCon sc.flow.add(initialWindowSize) sc.inflow.init(initialWindowSize) sc.hpackEncoder = hpack.NewEncoder(&sc.headerWriteBuf) - sc.hpackEncoder.SetMaxDynamicTableSizeLimit(s.maxEncoderHeaderTableSize()) + sc.hpackEncoder.SetMaxDynamicTableSizeLimit(conf.MaxEncoderHeaderTableSize) fr := NewFramer(sc.bw, c) - if s.CountError != nil { - fr.countError = s.CountError + if conf.CountError != nil { + fr.countError = conf.CountError } - fr.ReadMetaHeaders = hpack.NewDecoder(s.maxDecoderHeaderTableSize(), nil) + fr.ReadMetaHeaders = hpack.NewDecoder(conf.MaxDecoderHeaderTableSize, nil) fr.MaxHeaderListSize = sc.maxHeaderListSize() - fr.SetMaxReadFrameSize(s.maxReadFrameSize()) + fr.SetMaxReadFrameSize(conf.MaxReadFrameSize) sc.framer = fr if tc, ok := c.(connectionStater); ok { @@ -549,7 +507,7 @@ func (s *Server) serveConn(c net.Conn, opts *ServeConnOpts, newf func(*serverCon // So for now, do nothing here again. } - if !s.PermitProhibitedCipherSuites && isBadCipher(sc.tlsState.CipherSuite) { + if !conf.PermitProhibitedCipherSuites && isBadCipher(sc.tlsState.CipherSuite) { // "Endpoints MAY choose to generate a connection error // (Section 5.4.1) of type INADEQUATE_SECURITY if one of // the prohibited cipher suites are negotiated." @@ -586,7 +544,7 @@ func (s *Server) serveConn(c net.Conn, opts *ServeConnOpts, newf func(*serverCon opts.UpgradeRequest = nil } - sc.serve() + sc.serve(conf) } func serverConnBaseContext(c net.Conn, opts *ServeConnOpts) (ctx context.Context, cancel func()) { @@ -626,6 +584,7 @@ type serverConn struct { tlsState *tls.ConnectionState // shared by all handlers, like net/http remoteAddrStr string writeSched WriteScheduler + countErrorFunc func(errType string) // Everything following is owned by the serve loop; use serveG.check(): serveG goroutineLock // used to verify funcs are on serve() @@ -645,6 +604,7 @@ type serverConn struct { streams map[uint32]*stream unstartedHandlers []unstartedHandler initialStreamSendWindowSize int32 + initialStreamRecvWindowSize int32 maxFrameSize int32 peerMaxHeaderListSize uint32 // zero means unknown (default) canonHeader map[string]string // http2-lower-case -> Go-Canonical-Case @@ -660,6 +620,8 @@ type serverConn struct { goAwayCode ErrCode shutdownTimer timer // nil until used idleTimer timer // nil if unused + readIdleTimeout time.Duration + pingTimeout time.Duration readIdleTimer timer // nil if unused // Owned by the writeFrameAsync goroutine: @@ -675,11 +637,7 @@ func (sc *serverConn) maxHeaderListSize() uint32 { if n <= 0 { n = http.DefaultMaxHeaderBytes } - // http2's count is in a slightly different unit and includes 32 bytes per pair. - // So, take the net/http.Server value and pad it up a bit, assuming 10 headers. - const perFieldOverhead = 32 // per http2 spec - const typicalHeaders = 10 // conservative - return uint32(n + typicalHeaders*perFieldOverhead) + return uint32(adjustHTTP1MaxHeaderSize(int64(n))) } func (sc *serverConn) curOpenStreams() uint32 { @@ -943,7 +901,7 @@ func (sc *serverConn) notePanic() { } } -func (sc *serverConn) serve() { +func (sc *serverConn) serve(conf http2Config) { sc.serveG.check() defer sc.notePanic() defer sc.conn.Close() @@ -957,18 +915,18 @@ func (sc *serverConn) serve() { sc.writeFrame(FrameWriteRequest{ write: writeSettings{ - {SettingMaxFrameSize, sc.srv.maxReadFrameSize()}, + {SettingMaxFrameSize, conf.MaxReadFrameSize}, {SettingMaxConcurrentStreams, sc.advMaxStreams}, {SettingMaxHeaderListSize, sc.maxHeaderListSize()}, - {SettingHeaderTableSize, sc.srv.maxDecoderHeaderTableSize()}, - {SettingInitialWindowSize, uint32(sc.srv.initialStreamRecvWindowSize())}, + {SettingHeaderTableSize, conf.MaxDecoderHeaderTableSize}, + {SettingInitialWindowSize, uint32(sc.initialStreamRecvWindowSize)}, }, }) sc.unackedSettings++ // Each connection starts with initialWindowSize inflow tokens. // If a higher value is configured, we add more tokens. - if diff := sc.srv.initialConnRecvWindowSize() - initialWindowSize; diff > 0 { + if diff := conf.MaxUploadBufferPerConnection - initialWindowSize; diff > 0 { sc.sendWindowUpdate(nil, int(diff)) } @@ -988,8 +946,9 @@ func (sc *serverConn) serve() { defer sc.idleTimer.Stop() } - if sc.srv.ReadIdleTimeout > 0 { - sc.readIdleTimer = sc.srv.afterFunc(sc.srv.ReadIdleTimeout, sc.onReadIdleTimer) + if conf.SendPingTimeout > 0 { + sc.readIdleTimeout = conf.SendPingTimeout + sc.readIdleTimer = sc.srv.afterFunc(conf.SendPingTimeout, sc.onReadIdleTimer) defer sc.readIdleTimer.Stop() } @@ -1068,7 +1027,7 @@ func (sc *serverConn) serve() { // If the peer is causing us to generate a lot of control frames, // but not reading them from us, assume they are trying to make us // run out of memory. - if sc.queuedControlFrames > sc.srv.maxQueuedControlFrames() { + if sc.queuedControlFrames > maxQueuedControlFrames { sc.vlogf("http2: too many control frames in send queue, closing connection") return } @@ -1091,7 +1050,7 @@ func (sc *serverConn) handlePingTimer(lastFrameReadTime time.Time) { return } - pingAt := lastFrameReadTime.Add(sc.srv.ReadIdleTimeout) + pingAt := lastFrameReadTime.Add(sc.readIdleTimeout) now := sc.srv.now() if pingAt.After(now) { // We received frames since arming the ping timer. @@ -1107,11 +1066,7 @@ func (sc *serverConn) handlePingTimer(lastFrameReadTime time.Time) { sc.writeFrame(FrameWriteRequest{ write: &writePing{data: sc.sentPingData}, }) - pingTimeout := sc.srv.PingTimeout - if pingTimeout <= 0 { - pingTimeout = 15 * time.Second - } - sc.readIdleTimer.Reset(pingTimeout) + sc.readIdleTimer.Reset(sc.pingTimeout) } type serverMessage int @@ -1662,7 +1617,7 @@ func (sc *serverConn) processPing(f *PingFrame) error { if sc.pingSent && sc.sentPingData == f.Data { // This is a response to a PING we sent. sc.pingSent = false - sc.readIdleTimer.Reset(sc.srv.ReadIdleTimeout) + sc.readIdleTimer.Reset(sc.readIdleTimeout) } // 6.7 PING: " An endpoint MUST NOT respond to PING frames // containing this flag." @@ -2230,7 +2185,7 @@ func (sc *serverConn) newStream(id, pusherID uint32, state streamState) *stream st.cw.Init() st.flow.conn = &sc.flow // link to conn-level counter st.flow.add(sc.initialStreamSendWindowSize) - st.inflow.init(sc.srv.initialStreamRecvWindowSize()) + st.inflow.init(sc.initialStreamRecvWindowSize) if sc.hs.WriteTimeout > 0 { st.writeDeadline = sc.srv.afterFunc(sc.hs.WriteTimeout, st.onWriteTimeout) } @@ -3371,7 +3326,7 @@ func (sc *serverConn) countError(name string, err error) error { if sc == nil || sc.srv == nil { return err } - f := sc.srv.CountError + f := sc.countErrorFunc if f == nil { return err } diff --git a/http2/server_test.go b/http2/server_test.go index ec7ae043c..77de1be20 100644 --- a/http2/server_test.go +++ b/http2/server_test.go @@ -461,7 +461,8 @@ func (st *serverTester) greetAndCheckSettings(checkSetting func(s Setting) error if f.FrameHeader.StreamID != 0 { st.t.Fatalf("WindowUpdate StreamID = %d; want 0", f.FrameHeader.StreamID) } - incr := uint32(st.sc.srv.initialConnRecvWindowSize() - initialWindowSize) + conf := configFromServer(st.sc.hs, st.sc.srv) + incr := uint32(conf.MaxUploadBufferPerConnection - initialWindowSize) if f.Increment != incr { st.t.Fatalf("WindowUpdate increment = %d; want %d", f.Increment, incr) } @@ -589,11 +590,12 @@ func (st *serverTester) bodylessReq1(headers ...string) { } func (st *serverTester) wantFlowControlConsumed(streamID, consumed int32) { + conf := configFromServer(st.sc.hs, st.sc.srv) var initial int32 if streamID == 0 { - initial = st.sc.srv.initialConnRecvWindowSize() + initial = conf.MaxUploadBufferPerConnection } else { - initial = st.sc.srv.initialStreamRecvWindowSize() + initial = conf.MaxUploadBufferPerStream } donec := make(chan struct{}) st.sc.sendServeMsg(func(sc *serverConn) { diff --git a/http2/transport.go b/http2/transport.go index 49fc792ad..0c5f64aa8 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -226,40 +226,26 @@ func (t *Transport) contextWithTimeout(ctx context.Context, d time.Duration) (co } func (t *Transport) maxHeaderListSize() uint32 { - if t.MaxHeaderListSize == 0 { + n := int64(t.MaxHeaderListSize) + if t.t1 != nil && t.t1.MaxResponseHeaderBytes != 0 { + n = t.t1.MaxResponseHeaderBytes + if n > 0 { + n = adjustHTTP1MaxHeaderSize(n) + } + } + if n <= 0 { return 10 << 20 } - if t.MaxHeaderListSize == 0xffffffff { + if n >= 0xffffffff { return 0 } - return t.MaxHeaderListSize -} - -func (t *Transport) maxFrameReadSize() uint32 { - if t.MaxReadFrameSize == 0 { - return 0 // use the default provided by the peer - } - if t.MaxReadFrameSize < minMaxFrameSize { - return minMaxFrameSize - } - if t.MaxReadFrameSize > maxFrameSize { - return maxFrameSize - } - return t.MaxReadFrameSize + return uint32(n) } func (t *Transport) disableCompression() bool { return t.DisableCompression || (t.t1 != nil && t.t1.DisableCompression) } -func (t *Transport) pingTimeout() time.Duration { - if t.PingTimeout == 0 { - return 15 * time.Second - } - return t.PingTimeout - -} - // ConfigureTransport configures a net/http HTTP/1 Transport to use HTTP/2. // It returns an error if t1 has already been HTTP/2-enabled. // @@ -369,11 +355,14 @@ type ClientConn struct { lastActive time.Time lastIdle time.Time // time last idle // Settings from peer: (also guarded by wmu) - maxFrameSize uint32 - maxConcurrentStreams uint32 - peerMaxHeaderListSize uint64 - peerMaxHeaderTableSize uint32 - initialWindowSize uint32 + maxFrameSize uint32 + maxConcurrentStreams uint32 + peerMaxHeaderListSize uint64 + peerMaxHeaderTableSize uint32 + initialWindowSize uint32 + initialStreamRecvWindowSize int32 + readIdleTimeout time.Duration + pingTimeout time.Duration // reqHeaderMu is a 1-element semaphore channel controlling access to sending new requests. // Write to reqHeaderMu to lock it, read from it to unlock. @@ -745,39 +734,29 @@ func (t *Transport) expectContinueTimeout() time.Duration { 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()) } func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, error) { + conf := configFromTransport(t) cc := &ClientConn{ - t: t, - tconn: c, - readerDone: make(chan struct{}), - nextStreamID: 1, - maxFrameSize: 16 << 10, // spec default - initialWindowSize: 65535, // spec default - maxConcurrentStreams: initialMaxConcurrentStreams, // "infinite", per spec. Use a smaller value until we have received server settings. - peerMaxHeaderListSize: 0xffffffffffffffff, // "infinite", per spec. Use 2^64-1 instead. - streams: make(map[uint32]*clientStream), - singleUse: singleUse, - wantSettingsAck: true, - pings: make(map[[8]byte]chan struct{}), - reqHeaderMu: make(chan struct{}, 1), + t: t, + tconn: c, + readerDone: make(chan struct{}), + nextStreamID: 1, + maxFrameSize: 16 << 10, // spec default + initialWindowSize: 65535, // spec default + initialStreamRecvWindowSize: conf.MaxUploadBufferPerStream, + maxConcurrentStreams: initialMaxConcurrentStreams, // "infinite", per spec. Use a smaller value until we have received server settings. + peerMaxHeaderListSize: 0xffffffffffffffff, // "infinite", per spec. Use 2^64-1 instead. + streams: make(map[uint32]*clientStream), + singleUse: singleUse, + wantSettingsAck: true, + readIdleTimeout: conf.SendPingTimeout, + pingTimeout: conf.PingTimeout, + pings: make(map[[8]byte]chan struct{}), + reqHeaderMu: make(chan struct{}, 1), } var group synctestGroupInterface if t.transportTestHooks != nil { @@ -798,23 +777,21 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro cc.bw = bufio.NewWriter(stickyErrWriter{ group: group, conn: c, - timeout: t.WriteByteTimeout, + timeout: conf.WriteByteTimeout, err: &cc.werr, }) cc.br = bufio.NewReader(c) cc.fr = NewFramer(cc.bw, cc.br) - if t.maxFrameReadSize() != 0 { - cc.fr.SetMaxReadFrameSize(t.maxFrameReadSize()) - } + cc.fr.SetMaxReadFrameSize(conf.MaxReadFrameSize) if t.CountError != nil { cc.fr.countError = t.CountError } - maxHeaderTableSize := t.maxDecoderHeaderTableSize() + maxHeaderTableSize := conf.MaxDecoderHeaderTableSize cc.fr.ReadMetaHeaders = hpack.NewDecoder(maxHeaderTableSize, nil) cc.fr.MaxHeaderListSize = t.maxHeaderListSize() cc.henc = hpack.NewEncoder(&cc.hbuf) - cc.henc.SetMaxDynamicTableSizeLimit(t.maxEncoderHeaderTableSize()) + cc.henc.SetMaxDynamicTableSizeLimit(conf.MaxEncoderHeaderTableSize) cc.peerMaxHeaderTableSize = initialHeaderTableSize if cs, ok := c.(connectionStater); ok { @@ -824,11 +801,9 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro initialSettings := []Setting{ {ID: SettingEnablePush, Val: 0}, - {ID: SettingInitialWindowSize, Val: transportDefaultStreamFlow}, - } - if max := t.maxFrameReadSize(); max != 0 { - initialSettings = append(initialSettings, Setting{ID: SettingMaxFrameSize, Val: max}) + {ID: SettingInitialWindowSize, Val: uint32(cc.initialStreamRecvWindowSize)}, } + initialSettings = append(initialSettings, Setting{ID: SettingMaxFrameSize, Val: conf.MaxReadFrameSize}) if max := t.maxHeaderListSize(); max != 0 { initialSettings = append(initialSettings, Setting{ID: SettingMaxHeaderListSize, Val: max}) } @@ -838,8 +813,8 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro cc.bw.Write(clientPreface) cc.fr.WriteSettings(initialSettings...) - cc.fr.WriteWindowUpdate(0, transportDefaultConnFlow) - cc.inflow.init(transportDefaultConnFlow + initialWindowSize) + cc.fr.WriteWindowUpdate(0, uint32(conf.MaxUploadBufferPerConnection)) + cc.inflow.init(conf.MaxUploadBufferPerConnection + initialWindowSize) cc.bw.Flush() if cc.werr != nil { cc.Close() @@ -857,7 +832,7 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro } func (cc *ClientConn) healthCheck() { - pingTimeout := cc.t.pingTimeout() + pingTimeout := cc.pingTimeout // We don't need to periodically ping in the health check, because the readLoop of ClientConn will // trigger the healthCheck again if there is no frame received. ctx, cancel := cc.t.contextWithTimeout(context.Background(), pingTimeout) @@ -2189,7 +2164,7 @@ type resAndError struct { func (cc *ClientConn) addStreamLocked(cs *clientStream) { cs.flow.add(int32(cc.initialWindowSize)) cs.flow.setConnFlow(&cc.flow) - cs.inflow.init(transportDefaultStreamFlow) + cs.inflow.init(cc.initialStreamRecvWindowSize) cs.ID = cc.nextStreamID cc.nextStreamID += 2 cc.streams[cs.ID] = cs @@ -2335,7 +2310,7 @@ func (cc *ClientConn) countReadFrameError(err error) { func (rl *clientConnReadLoop) run() error { cc := rl.cc gotSettings := false - readIdleTimeout := cc.t.ReadIdleTimeout + readIdleTimeout := cc.readIdleTimeout var t timer if readIdleTimeout != 0 { t = cc.t.afterFunc(readIdleTimeout, cc.healthCheck) From f88258d67e0f0f144c79964ca05bb81d51ee8411 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 25 Sep 2024 19:42:35 +0000 Subject: [PATCH 5/6] websocket: update nhooyr.io/websocket to github.com/coder/websocket Maintenance of nhooyr.io/websocket has moved to github.com/coder/websocket. Read more about the transition at https://coder.com/blog/websocket Updates golang/go#18152 Change-Id: Ia2b11c9a57ad7ded775b50a5bbb7ea91562d39b5 GitHub-Last-Rev: 59cea5e101430586ca445f6e119e25cae5242254 GitHub-Pull-Request: golang/net#222 Reviewed-on: https://go-review.googlesource.com/c/net/+/614075 Reviewed-by: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil Auto-Submit: Damien Neil --- websocket/websocket.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/websocket/websocket.go b/websocket/websocket.go index 923a5780e..ac76165ce 100644 --- a/websocket/websocket.go +++ b/websocket/websocket.go @@ -8,7 +8,7 @@ // This package currently lacks some features found in an alternative // and more actively maintained WebSocket package: // -// https://pkg.go.dev/nhooyr.io/websocket +// https://pkg.go.dev/github.com/coder/websocket package websocket // import "golang.org/x/net/websocket" import ( From 6cc5ac4e9a03d73b331eb1d6db98a02e558243b7 Mon Sep 17 00:00:00 2001 From: Gopher Robot Date: Fri, 4 Oct 2024 16:00:50 +0000 Subject: [PATCH 6/6] go.mod: update golang.org/x dependencies Update golang.org/x dependencies to their latest tagged versions. Change-Id: I57613519f6e5795e60d258865e81f6954d672606 Reviewed-on: https://go-review.googlesource.com/c/net/+/617959 Reviewed-by: David Chase Auto-Submit: Gopher Robot Reviewed-by: Dmitri Shuralyov LUCI-TryBot-Result: Go LUCI --- go.mod | 8 ++++---- go.sum | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index ffda816a8..77935d3f2 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module golang.org/x/net go 1.18 require ( - golang.org/x/crypto v0.27.0 - golang.org/x/sys v0.25.0 - golang.org/x/term v0.24.0 - golang.org/x/text v0.18.0 + golang.org/x/crypto v0.28.0 + golang.org/x/sys v0.26.0 + golang.org/x/term v0.25.0 + golang.org/x/text v0.19.0 ) diff --git a/go.sum b/go.sum index a1a56bf3d..1b8c61d60 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,8 @@ -golang.org/x/crypto v0.27.0 h1:GXm2NjJrPaiv/h1tb2UH8QfgC/hOf/+z0p6PT8o1w7A= -golang.org/x/crypto v0.27.0/go.mod h1:1Xngt8kV6Dvbssa53Ziq6Eqn0HqbZi5Z6R0ZpwQzt70= -golang.org/x/sys v0.25.0 h1:r+8e+loiHxRqhXVl6ML1nO3l1+oFoWbnlu2Ehimmi34= -golang.org/x/sys v0.25.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/term v0.24.0 h1:Mh5cbb+Zk2hqqXNO7S1iTjEphVL+jb8ZWaqh/g+JWkM= -golang.org/x/term v0.24.0/go.mod h1:lOBK/LVxemqiMij05LGJ0tzNr8xlmwBRJ81PX6wVLH8= -golang.org/x/text v0.18.0 h1:XvMDiNzPAl0jr17s6W9lcaIhGUfUORdGCNsuLmPG224= -golang.org/x/text v0.18.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= +golang.org/x/crypto v0.28.0 h1:GBDwsMXVQi34v5CCYUm2jkJvu4cbtru2U4TN2PSyQnw= +golang.org/x/crypto v0.28.0/go.mod h1:rmgy+3RHxRZMyY0jjAJShp2zgEdOqj2AO7U0pYmeQ7U= +golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo= +golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.25.0 h1:WtHI/ltw4NvSUig5KARz9h521QvRC8RmF/cuYqifU24= +golang.org/x/term v0.25.0/go.mod h1:RPyXicDX+6vLxogjjRxjgD2TKtmAO6NZBsBRfrOLu7M= +golang.org/x/text v0.19.0 h1:kTxAhCbGbxhK0IwgSKiMO5awPoDQ0RpfiVYBfK860YM= +golang.org/x/text v0.19.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY=