From d1683ac987d8545d9020f3f863e4885a30cadb7a Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Mon, 9 Sep 2024 16:38:23 +0800 Subject: [PATCH 1/6] http2: server advertises SETTINGS_ENABLE_CONNECT_PROTOCOL support --- http2/frame.go | 2 +- http2/http2.go | 38 ++++++++++++++++++++++---------------- http2/server.go | 35 ++++++++++++++++++++++++++--------- 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/http2/frame.go b/http2/frame.go index 105c3b279..498163b05 100644 --- a/http2/frame.go +++ b/http2/frame.go @@ -1490,7 +1490,7 @@ func (mh *MetaHeadersFrame) checkPseudos() error { pf := mh.PseudoFields() for i, hf := range pf { switch hf.Name { - case ":method", ":path", ":scheme", ":authority": + case ":method", ":path", ":scheme", ":authority", ":protocol": isRequest = true case ":status": isResponse = true diff --git a/http2/http2.go b/http2/http2.go index 003e649f3..c590de684 100644 --- a/http2/http2.go +++ b/http2/http2.go @@ -33,10 +33,11 @@ import ( ) var ( - VerboseLogs bool - logFrameWrites bool - logFrameReads bool - inTests bool + VerboseLogs bool + logFrameWrites bool + logFrameReads bool + inTests bool + disableExtendedConnectProtocol bool ) func init() { @@ -49,6 +50,9 @@ func init() { logFrameWrites = true logFrameReads = true } + if strings.Contains(e, "http2xconnect=0") { + disableExtendedConnectProtocol = true + } } const ( @@ -149,21 +153,23 @@ func (s Setting) Valid() error { type SettingID uint16 const ( - SettingHeaderTableSize SettingID = 0x1 - SettingEnablePush SettingID = 0x2 - SettingMaxConcurrentStreams SettingID = 0x3 - SettingInitialWindowSize SettingID = 0x4 - SettingMaxFrameSize SettingID = 0x5 - SettingMaxHeaderListSize SettingID = 0x6 + SettingHeaderTableSize SettingID = 0x1 + SettingEnablePush SettingID = 0x2 + SettingMaxConcurrentStreams SettingID = 0x3 + SettingInitialWindowSize SettingID = 0x4 + SettingMaxFrameSize SettingID = 0x5 + SettingMaxHeaderListSize SettingID = 0x6 + SettingEnableConnectProtocol SettingID = 0x8 ) var settingName = map[SettingID]string{ - SettingHeaderTableSize: "HEADER_TABLE_SIZE", - SettingEnablePush: "ENABLE_PUSH", - SettingMaxConcurrentStreams: "MAX_CONCURRENT_STREAMS", - SettingInitialWindowSize: "INITIAL_WINDOW_SIZE", - SettingMaxFrameSize: "MAX_FRAME_SIZE", - SettingMaxHeaderListSize: "MAX_HEADER_LIST_SIZE", + SettingHeaderTableSize: "HEADER_TABLE_SIZE", + SettingEnablePush: "ENABLE_PUSH", + SettingMaxConcurrentStreams: "MAX_CONCURRENT_STREAMS", + SettingInitialWindowSize: "INITIAL_WINDOW_SIZE", + SettingMaxFrameSize: "MAX_FRAME_SIZE", + SettingMaxHeaderListSize: "MAX_HEADER_LIST_SIZE", + SettingEnableConnectProtocol: "ENABLE_CONNECT_PROTOCOL", } func (s SettingID) String() string { diff --git a/http2/server.go b/http2/server.go index 6c349f3ec..8f080f36a 100644 --- a/http2/server.go +++ b/http2/server.go @@ -935,14 +935,18 @@ func (sc *serverConn) serve() { sc.vlogf("http2: server connection from %v on %p", sc.conn.RemoteAddr(), sc.hs) } + settings := writeSettings{ + {SettingMaxFrameSize, sc.srv.maxReadFrameSize()}, + {SettingMaxConcurrentStreams, sc.advMaxStreams}, + {SettingMaxHeaderListSize, sc.maxHeaderListSize()}, + {SettingHeaderTableSize, sc.srv.maxDecoderHeaderTableSize()}, + {SettingInitialWindowSize, uint32(sc.srv.initialStreamRecvWindowSize())}, + } + if !disableExtendedConnectProtocol { + settings = append(settings, Setting{SettingEnableConnectProtocol, 1}) + } sc.writeFrame(FrameWriteRequest{ - write: writeSettings{ - {SettingMaxFrameSize, sc.srv.maxReadFrameSize()}, - {SettingMaxConcurrentStreams, sc.advMaxStreams}, - {SettingMaxHeaderListSize, sc.maxHeaderListSize()}, - {SettingHeaderTableSize, sc.srv.maxDecoderHeaderTableSize()}, - {SettingInitialWindowSize, uint32(sc.srv.initialStreamRecvWindowSize())}, - }, + write: settings, }) sc.unackedSettings++ @@ -1757,6 +1761,9 @@ func (sc *serverConn) processSetting(s Setting) error { sc.maxFrameSize = int32(s.Val) // the maximum valid s.Val is < 2^31 case SettingMaxHeaderListSize: sc.peerMaxHeaderListSize = s.Val + case SettingEnableConnectProtocol: + // Receipt of this parameter by a server does not + // have any impact default: // Unknown setting: "An endpoint that receives a SETTINGS // frame with any unknown or unsupported identifier MUST @@ -2187,11 +2194,17 @@ func (sc *serverConn) newWriterAndRequest(st *stream, f *MetaHeadersFrame) (*res scheme: f.PseudoValue("scheme"), authority: f.PseudoValue("authority"), path: f.PseudoValue("path"), + protocol: f.PseudoValue("protocol"), + } + + // extended connect is disabled, so we should not see :protocol + if disableExtendedConnectProtocol && rp.protocol != "" { + return nil, nil, sc.countError("bad_connect", streamError(f.StreamID, ErrCodeProtocol)) } isConnect := rp.method == "CONNECT" if isConnect { - if rp.path != "" || rp.scheme != "" || rp.authority == "" { + if rp.protocol == "" && (rp.path != "" || rp.scheme != "" || rp.authority == "") { return nil, nil, sc.countError("bad_connect", streamError(f.StreamID, ErrCodeProtocol)) } } else if rp.method == "" || rp.path == "" || (rp.scheme != "https" && rp.scheme != "http") { @@ -2215,6 +2228,9 @@ func (sc *serverConn) newWriterAndRequest(st *stream, f *MetaHeadersFrame) (*res if rp.authority == "" { rp.authority = rp.header.Get("Host") } + if rp.protocol != "" { + rp.header.Set(":protocol", rp.protocol) + } rw, req, err := sc.newWriterAndRequestNoBody(st, rp) if err != nil { @@ -2241,6 +2257,7 @@ func (sc *serverConn) newWriterAndRequest(st *stream, f *MetaHeadersFrame) (*res type requestParam struct { method string scheme, authority, path string + protocol string header http.Header } @@ -2282,7 +2299,7 @@ func (sc *serverConn) newWriterAndRequestNoBody(st *stream, rp requestParam) (*r var url_ *url.URL var requestURI string - if rp.method == "CONNECT" { + if rp.method == "CONNECT" && rp.protocol == "" { url_ = &url.URL{Host: rp.authority} requestURI = rp.authority // mimic HTTP/1 server behavior } else { From 77f622bf829dabf319b41a8fc8cf716436d33645 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Fri, 27 Sep 2024 09:16:01 +0800 Subject: [PATCH 2/6] http2: client side SETTINGS_ENABLE_CONNECT_PROTOCOL support --- http2/http2.go | 4 +++ http2/transport.go | 79 +++++++++++++++++++++++++++++------------ http2/transport_test.go | 59 ++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 22 deletions(-) diff --git a/http2/http2.go b/http2/http2.go index c590de684..a7dd5ff52 100644 --- a/http2/http2.go +++ b/http2/http2.go @@ -144,6 +144,10 @@ func (s Setting) Valid() error { if s.Val < 16384 || s.Val > 1<<24-1 { return ConnectionError(ErrCodeProtocol) } + case SettingEnableConnectProtocol: + if s.Val != 1 && s.Val != 0 { + return ConnectionError(ErrCodeProtocol) + } } return nil } diff --git a/http2/transport.go b/http2/transport.go index 61f511f97..ed1a23b58 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -350,31 +350,33 @@ type ClientConn struct { idleTimeout time.Duration // or 0 for never idleTimer timer - mu sync.Mutex // guards following - cond *sync.Cond // hold mu; broadcast on flow/closed changes - flow outflow // our conn-level flow control quota (cs.outflow is per stream) - inflow inflow // peer's conn-level flow control - doNotReuse bool // whether conn is marked to not be reused for any future requests - closing bool - closed bool - seenSettings bool // true if we've seen a settings frame, false otherwise - wantSettingsAck bool // we sent a SETTINGS frame and haven't heard back - goAway *GoAwayFrame // if non-nil, the GoAwayFrame we received - goAwayDebug string // goAway frame's debug data, retained as a string - streams map[uint32]*clientStream // client-initiated - streamsReserved int // incr by ReserveNewRequest; decr on RoundTrip - nextStreamID uint32 - pendingRequests int // requests blocked and waiting to be sent because len(streams) == maxConcurrentStreams - pings map[[8]byte]chan struct{} // in flight ping data to notification channel - br *bufio.Reader - lastActive time.Time - lastIdle time.Time // time last idle + mu sync.Mutex // guards following + cond *sync.Cond // hold mu; broadcast on flow/closed changes + flow outflow // our conn-level flow control quota (cs.outflow is per stream) + inflow inflow // peer's conn-level flow control + doNotReuse bool // whether conn is marked to not be reused for any future requests + closing bool + closed bool + seenSettings bool // true if we've seen a settings frame, false otherwise + seenSettingsChan chan struct{} // closed when seenSettings is true or frame reading fails + wantSettingsAck bool // we sent a SETTINGS frame and haven't heard back + goAway *GoAwayFrame // if non-nil, the GoAwayFrame we received + goAwayDebug string // goAway frame's debug data, retained as a string + streams map[uint32]*clientStream // client-initiated + streamsReserved int // incr by ReserveNewRequest; decr on RoundTrip + nextStreamID uint32 + pendingRequests int // requests blocked and waiting to be sent because len(streams) == maxConcurrentStreams + pings map[[8]byte]chan struct{} // in flight ping data to notification channel + br *bufio.Reader + 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 + extendedConnecAllowed bool // reqHeaderMu is a 1-element semaphore channel controlling access to sending new requests. // Write to reqHeaderMu to lock it, read from it to unlock. @@ -788,6 +790,7 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro peerMaxHeaderListSize: 0xffffffffffffffff, // "infinite", per spec. Use 2^64-1 instead. streams: make(map[uint32]*clientStream), singleUse: singleUse, + seenSettingsChan: make(chan struct{}), wantSettingsAck: true, pings: make(map[[8]byte]chan struct{}), reqHeaderMu: make(chan struct{}, 1), @@ -1411,6 +1414,8 @@ func (cs *clientStream) doRequest(req *http.Request, streamf func(*clientStream) cs.cleanupWriteRequest(err) } +var errExtendedConnectNotSupported = errors.New("net/http: extended connect not supported by peer") + // writeRequest sends a request. // // It returns nil after the request is written, the response read, @@ -1440,7 +1445,20 @@ func (cs *clientStream) writeRequest(req *http.Request, streamf func(*clientStre return ctx.Err() } + // wait for setting frames to be received, a server can change this value later, + // but we just wait for the first settings frame + var isExtendedConnect bool + if req.Method == "CONNECT" && req.Header.Get(":protocol") != "" { + isExtendedConnect = true + <-cc.seenSettingsChan + } + cc.mu.Lock() + if isExtendedConnect && !cc.extendedConnecAllowed { + cc.mu.Unlock() + <-cc.reqHeaderMu + return errExtendedConnectNotSupported + } if cc.idleTimer != nil { cc.idleTimer.Stop() } @@ -1945,7 +1963,7 @@ func (cs *clientStream) awaitFlowControl(maxBytes int) (taken int32, err error) func validateHeaders(hdrs http.Header) string { for k, vv := range hdrs { - if !httpguts.ValidHeaderFieldName(k) { + if !httpguts.ValidHeaderFieldName(k) && k != ":protocol" { return fmt.Sprintf("name %q", k) } for _, v := range vv { @@ -1961,6 +1979,10 @@ func validateHeaders(hdrs http.Header) string { var errNilRequestURL = errors.New("http2: Request.URI is nil") +func isNormalConnect(req *http.Request) bool { + return req.Method == "CONNECT" && req.Header.Get(":protocol") == "" +} + // requires cc.wmu be held. func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trailers string, contentLength int64) ([]byte, error) { cc.hbuf.Reset() @@ -1981,7 +2003,7 @@ func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trail } var path string - if req.Method != "CONNECT" { + if !isNormalConnect(req) { path = req.URL.RequestURI() if !validPseudoPath(path) { orig := path @@ -2018,7 +2040,7 @@ func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trail m = http.MethodGet } f(":method", m) - if req.Method != "CONNECT" { + if !isNormalConnect(req) { f(":path", path) f(":scheme", req.URL.Scheme) } @@ -2405,6 +2427,9 @@ func (rl *clientConnReadLoop) run() error { if VerboseLogs { cc.vlogf("http2: Transport conn %p received error from processing frame %v: %v", cc, summarizeFrame(f), err) } + if !cc.seenSettings { + close(cc.seenSettingsChan) + } return err } } @@ -2952,6 +2977,15 @@ func (rl *clientConnReadLoop) processSettingsNoWrite(f *SettingsFrame) error { case SettingHeaderTableSize: cc.henc.SetMaxDynamicTableSize(s.Val) cc.peerMaxHeaderTableSize = s.Val + case SettingEnableConnectProtocol: + if err := s.Valid(); err != nil { + return err + } + // RFC 8441 section, https://datatracker.ietf.org/doc/html/rfc8441#section-3 + if s.Val == 0 && cc.extendedConnecAllowed { + return ConnectionError(ErrCodeProtocol) + } + cc.extendedConnecAllowed = s.Val == 1 default: cc.vlogf("Unhandled Setting: %v", s) } @@ -2969,6 +3003,7 @@ func (rl *clientConnReadLoop) processSettingsNoWrite(f *SettingsFrame) error { // connection can establish to our default. cc.maxConcurrentStreams = defaultMaxConcurrentStreams } + close(cc.seenSettingsChan) cc.seenSettings = true } diff --git a/http2/transport_test.go b/http2/transport_test.go index 498e27932..5e2e2eb1e 100644 --- a/http2/transport_test.go +++ b/http2/transport_test.go @@ -5421,3 +5421,62 @@ func TestIssue67671(t *testing.T) { res.Body.Close() } } + +func TestExtendedConnectClientWithServerSupport(t *testing.T) { + disableExtendedConnectProtocol = false + ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + t.Log(io.Copy(w, r.Body)) + }) + tr := &Transport{ + TLSClientConfig: tlsConfigInsecure, + AllowHTTP: true, + } + defer tr.CloseIdleConnections() + pr, pw := io.Pipe() + pwDone := make(chan struct{}) + req, _ := http.NewRequest("CONNECT", ts.URL, pr) + req.Header.Set(":protocol", "extended-connect") + go func() { + pw.Write([]byte("hello, extended connect")) + pw.Close() + close(pwDone) + }() + + res, err := tr.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + body, err := io.ReadAll(res.Body) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(body, []byte("hello, extended connect")) { + t.Fatal("unexpected body received") + } +} + +func TestExtendedConnectClientWithoutServerSupport(t *testing.T) { + disableExtendedConnectProtocol = true + ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + io.Copy(w, r.Body) + }) + tr := &Transport{ + TLSClientConfig: tlsConfigInsecure, + AllowHTTP: true, + } + defer tr.CloseIdleConnections() + pr, pw := io.Pipe() + pwDone := make(chan struct{}) + req, _ := http.NewRequest("CONNECT", ts.URL, pr) + req.Header.Set(":protocol", "extended-connect") + go func() { + pw.Write([]byte("hello, extended connect")) + pw.Close() + close(pwDone) + }() + + _, err := tr.RoundTrip(req) + if !errors.Is(err, errExtendedConnectNotSupported) { + t.Fatalf("expected error errExtendedConnectNotSupported, got: %v", err) + } +} From f238a07580be5c4a8f7b5d1943aa98b0c2c065c4 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Mon, 9 Sep 2024 16:38:23 +0800 Subject: [PATCH 3/6] http2: server advertises SETTINGS_ENABLE_CONNECT_PROTOCOL support --- http2/frame.go | 2 +- http2/http2.go | 38 ++++++++++++++++++++++---------------- http2/server.go | 35 ++++++++++++++++++++++++++--------- 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/http2/frame.go b/http2/frame.go index 105c3b279..498163b05 100644 --- a/http2/frame.go +++ b/http2/frame.go @@ -1490,7 +1490,7 @@ func (mh *MetaHeadersFrame) checkPseudos() error { pf := mh.PseudoFields() for i, hf := range pf { switch hf.Name { - case ":method", ":path", ":scheme", ":authority": + case ":method", ":path", ":scheme", ":authority", ":protocol": isRequest = true case ":status": isResponse = true diff --git a/http2/http2.go b/http2/http2.go index 7688c356b..86403341a 100644 --- a/http2/http2.go +++ b/http2/http2.go @@ -34,10 +34,11 @@ import ( ) var ( - VerboseLogs bool - logFrameWrites bool - logFrameReads bool - inTests bool + VerboseLogs bool + logFrameWrites bool + logFrameReads bool + inTests bool + disableExtendedConnectProtocol bool ) func init() { @@ -50,6 +51,9 @@ func init() { logFrameWrites = true logFrameReads = true } + if strings.Contains(e, "http2xconnect=0") { + disableExtendedConnectProtocol = true + } } const ( @@ -150,21 +154,23 @@ func (s Setting) Valid() error { type SettingID uint16 const ( - SettingHeaderTableSize SettingID = 0x1 - SettingEnablePush SettingID = 0x2 - SettingMaxConcurrentStreams SettingID = 0x3 - SettingInitialWindowSize SettingID = 0x4 - SettingMaxFrameSize SettingID = 0x5 - SettingMaxHeaderListSize SettingID = 0x6 + SettingHeaderTableSize SettingID = 0x1 + SettingEnablePush SettingID = 0x2 + SettingMaxConcurrentStreams SettingID = 0x3 + SettingInitialWindowSize SettingID = 0x4 + SettingMaxFrameSize SettingID = 0x5 + SettingMaxHeaderListSize SettingID = 0x6 + SettingEnableConnectProtocol SettingID = 0x8 ) var settingName = map[SettingID]string{ - SettingHeaderTableSize: "HEADER_TABLE_SIZE", - SettingEnablePush: "ENABLE_PUSH", - SettingMaxConcurrentStreams: "MAX_CONCURRENT_STREAMS", - SettingInitialWindowSize: "INITIAL_WINDOW_SIZE", - SettingMaxFrameSize: "MAX_FRAME_SIZE", - SettingMaxHeaderListSize: "MAX_HEADER_LIST_SIZE", + SettingHeaderTableSize: "HEADER_TABLE_SIZE", + SettingEnablePush: "ENABLE_PUSH", + SettingMaxConcurrentStreams: "MAX_CONCURRENT_STREAMS", + SettingInitialWindowSize: "INITIAL_WINDOW_SIZE", + SettingMaxFrameSize: "MAX_FRAME_SIZE", + SettingMaxHeaderListSize: "MAX_HEADER_LIST_SIZE", + SettingEnableConnectProtocol: "ENABLE_CONNECT_PROTOCOL", } func (s SettingID) String() string { diff --git a/http2/server.go b/http2/server.go index 617b4a476..01a22c9c8 100644 --- a/http2/server.go +++ b/http2/server.go @@ -913,14 +913,18 @@ func (sc *serverConn) serve(conf http2Config) { sc.vlogf("http2: server connection from %v on %p", sc.conn.RemoteAddr(), sc.hs) } + settings := writeSettings{ + {SettingMaxFrameSize, conf.MaxReadFrameSize}, + {SettingMaxConcurrentStreams, sc.advMaxStreams}, + {SettingMaxHeaderListSize, sc.maxHeaderListSize()}, + {SettingHeaderTableSize, conf.MaxDecoderHeaderTableSize}, + {SettingInitialWindowSize, uint32(sc.initialStreamRecvWindowSize)}, + } + if !disableExtendedConnectProtocol { + settings = append(settings, Setting{SettingEnableConnectProtocol, 1}) + } sc.writeFrame(FrameWriteRequest{ - write: writeSettings{ - {SettingMaxFrameSize, conf.MaxReadFrameSize}, - {SettingMaxConcurrentStreams, sc.advMaxStreams}, - {SettingMaxHeaderListSize, sc.maxHeaderListSize()}, - {SettingHeaderTableSize, conf.MaxDecoderHeaderTableSize}, - {SettingInitialWindowSize, uint32(sc.initialStreamRecvWindowSize)}, - }, + write: settings, }) sc.unackedSettings++ @@ -1782,6 +1786,9 @@ func (sc *serverConn) processSetting(s Setting) error { sc.maxFrameSize = int32(s.Val) // the maximum valid s.Val is < 2^31 case SettingMaxHeaderListSize: sc.peerMaxHeaderListSize = s.Val + case SettingEnableConnectProtocol: + // Receipt of this parameter by a server does not + // have any impact default: // Unknown setting: "An endpoint that receives a SETTINGS // frame with any unknown or unsupported identifier MUST @@ -2212,11 +2219,17 @@ func (sc *serverConn) newWriterAndRequest(st *stream, f *MetaHeadersFrame) (*res scheme: f.PseudoValue("scheme"), authority: f.PseudoValue("authority"), path: f.PseudoValue("path"), + protocol: f.PseudoValue("protocol"), + } + + // extended connect is disabled, so we should not see :protocol + if disableExtendedConnectProtocol && rp.protocol != "" { + return nil, nil, sc.countError("bad_connect", streamError(f.StreamID, ErrCodeProtocol)) } isConnect := rp.method == "CONNECT" if isConnect { - if rp.path != "" || rp.scheme != "" || rp.authority == "" { + if rp.protocol == "" && (rp.path != "" || rp.scheme != "" || rp.authority == "") { return nil, nil, sc.countError("bad_connect", streamError(f.StreamID, ErrCodeProtocol)) } } else if rp.method == "" || rp.path == "" || (rp.scheme != "https" && rp.scheme != "http") { @@ -2240,6 +2253,9 @@ func (sc *serverConn) newWriterAndRequest(st *stream, f *MetaHeadersFrame) (*res if rp.authority == "" { rp.authority = rp.header.Get("Host") } + if rp.protocol != "" { + rp.header.Set(":protocol", rp.protocol) + } rw, req, err := sc.newWriterAndRequestNoBody(st, rp) if err != nil { @@ -2266,6 +2282,7 @@ func (sc *serverConn) newWriterAndRequest(st *stream, f *MetaHeadersFrame) (*res type requestParam struct { method string scheme, authority, path string + protocol string header http.Header } @@ -2307,7 +2324,7 @@ func (sc *serverConn) newWriterAndRequestNoBody(st *stream, rp requestParam) (*r var url_ *url.URL var requestURI string - if rp.method == "CONNECT" { + if rp.method == "CONNECT" && rp.protocol == "" { url_ = &url.URL{Host: rp.authority} requestURI = rp.authority // mimic HTTP/1 server behavior } else { From 4e0170fe1f5a5900d0df08ca5d33d29b02a6c4f0 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Fri, 27 Sep 2024 09:16:01 +0800 Subject: [PATCH 4/6] http2: client side SETTINGS_ENABLE_CONNECT_PROTOCOL support --- http2/http2.go | 4 +++ http2/transport.go | 79 +++++++++++++++++++++++++++++------------ http2/transport_test.go | 59 ++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 22 deletions(-) diff --git a/http2/http2.go b/http2/http2.go index 86403341a..c7601c909 100644 --- a/http2/http2.go +++ b/http2/http2.go @@ -145,6 +145,10 @@ func (s Setting) Valid() error { if s.Val < 16384 || s.Val > 1<<24-1 { return ConnectionError(ErrCodeProtocol) } + case SettingEnableConnectProtocol: + if s.Val != 1 && s.Val != 0 { + return ConnectionError(ErrCodeProtocol) + } } return nil } diff --git a/http2/transport.go b/http2/transport.go index 0c5f64aa8..27c47ef2a 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -335,25 +335,26 @@ type ClientConn struct { idleTimeout time.Duration // or 0 for never idleTimer timer - mu sync.Mutex // guards following - cond *sync.Cond // hold mu; broadcast on flow/closed changes - flow outflow // our conn-level flow control quota (cs.outflow is per stream) - inflow inflow // peer's conn-level flow control - doNotReuse bool // whether conn is marked to not be reused for any future requests - closing bool - closed bool - seenSettings bool // true if we've seen a settings frame, false otherwise - wantSettingsAck bool // we sent a SETTINGS frame and haven't heard back - goAway *GoAwayFrame // if non-nil, the GoAwayFrame we received - goAwayDebug string // goAway frame's debug data, retained as a string - streams map[uint32]*clientStream // client-initiated - streamsReserved int // incr by ReserveNewRequest; decr on RoundTrip - nextStreamID uint32 - pendingRequests int // requests blocked and waiting to be sent because len(streams) == maxConcurrentStreams - pings map[[8]byte]chan struct{} // in flight ping data to notification channel - br *bufio.Reader - lastActive time.Time - lastIdle time.Time // time last idle + mu sync.Mutex // guards following + cond *sync.Cond // hold mu; broadcast on flow/closed changes + flow outflow // our conn-level flow control quota (cs.outflow is per stream) + inflow inflow // peer's conn-level flow control + doNotReuse bool // whether conn is marked to not be reused for any future requests + closing bool + closed bool + seenSettings bool // true if we've seen a settings frame, false otherwise + seenSettingsChan chan struct{} // closed when seenSettings is true or frame reading fails + wantSettingsAck bool // we sent a SETTINGS frame and haven't heard back + goAway *GoAwayFrame // if non-nil, the GoAwayFrame we received + goAwayDebug string // goAway frame's debug data, retained as a string + streams map[uint32]*clientStream // client-initiated + streamsReserved int // incr by ReserveNewRequest; decr on RoundTrip + nextStreamID uint32 + pendingRequests int // requests blocked and waiting to be sent because len(streams) == maxConcurrentStreams + pings map[[8]byte]chan struct{} // in flight ping data to notification channel + br *bufio.Reader + lastActive time.Time + lastIdle time.Time // time last idle // Settings from peer: (also guarded by wmu) maxFrameSize uint32 maxConcurrentStreams uint32 @@ -363,6 +364,7 @@ type ClientConn struct { initialStreamRecvWindowSize int32 readIdleTimeout time.Duration pingTimeout time.Duration + extendedConnecAllowed bool // reqHeaderMu is a 1-element semaphore channel controlling access to sending new requests. // Write to reqHeaderMu to lock it, read from it to unlock. @@ -752,6 +754,7 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro peerMaxHeaderListSize: 0xffffffffffffffff, // "infinite", per spec. Use 2^64-1 instead. streams: make(map[uint32]*clientStream), singleUse: singleUse, + seenSettingsChan: make(chan struct{}), wantSettingsAck: true, readIdleTimeout: conf.SendPingTimeout, pingTimeout: conf.PingTimeout, @@ -1376,6 +1379,8 @@ func (cs *clientStream) doRequest(req *http.Request, streamf func(*clientStream) cs.cleanupWriteRequest(err) } +var errExtendedConnectNotSupported = errors.New("net/http: extended connect not supported by peer") + // writeRequest sends a request. // // It returns nil after the request is written, the response read, @@ -1405,7 +1410,20 @@ func (cs *clientStream) writeRequest(req *http.Request, streamf func(*clientStre return ctx.Err() } + // wait for setting frames to be received, a server can change this value later, + // but we just wait for the first settings frame + var isExtendedConnect bool + if req.Method == "CONNECT" && req.Header.Get(":protocol") != "" { + isExtendedConnect = true + <-cc.seenSettingsChan + } + cc.mu.Lock() + if isExtendedConnect && !cc.extendedConnecAllowed { + cc.mu.Unlock() + <-cc.reqHeaderMu + return errExtendedConnectNotSupported + } if cc.idleTimer != nil { cc.idleTimer.Stop() } @@ -1910,7 +1928,7 @@ func (cs *clientStream) awaitFlowControl(maxBytes int) (taken int32, err error) func validateHeaders(hdrs http.Header) string { for k, vv := range hdrs { - if !httpguts.ValidHeaderFieldName(k) { + if !httpguts.ValidHeaderFieldName(k) && k != ":protocol" { return fmt.Sprintf("name %q", k) } for _, v := range vv { @@ -1926,6 +1944,10 @@ func validateHeaders(hdrs http.Header) string { var errNilRequestURL = errors.New("http2: Request.URI is nil") +func isNormalConnect(req *http.Request) bool { + return req.Method == "CONNECT" && req.Header.Get(":protocol") == "" +} + // requires cc.wmu be held. func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trailers string, contentLength int64) ([]byte, error) { cc.hbuf.Reset() @@ -1946,7 +1968,7 @@ func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trail } var path string - if req.Method != "CONNECT" { + if !isNormalConnect(req) { path = req.URL.RequestURI() if !validPseudoPath(path) { orig := path @@ -1983,7 +2005,7 @@ func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trail m = http.MethodGet } f(":method", m) - if req.Method != "CONNECT" { + if !isNormalConnect(req) { f(":path", path) f(":scheme", req.URL.Scheme) } @@ -2370,6 +2392,9 @@ func (rl *clientConnReadLoop) run() error { if VerboseLogs { cc.vlogf("http2: Transport conn %p received error from processing frame %v: %v", cc, summarizeFrame(f), err) } + if !cc.seenSettings { + close(cc.seenSettingsChan) + } return err } } @@ -2917,6 +2942,15 @@ func (rl *clientConnReadLoop) processSettingsNoWrite(f *SettingsFrame) error { case SettingHeaderTableSize: cc.henc.SetMaxDynamicTableSize(s.Val) cc.peerMaxHeaderTableSize = s.Val + case SettingEnableConnectProtocol: + if err := s.Valid(); err != nil { + return err + } + // RFC 8441 section, https://datatracker.ietf.org/doc/html/rfc8441#section-3 + if s.Val == 0 && cc.extendedConnecAllowed { + return ConnectionError(ErrCodeProtocol) + } + cc.extendedConnecAllowed = s.Val == 1 default: cc.vlogf("Unhandled Setting: %v", s) } @@ -2934,6 +2968,7 @@ func (rl *clientConnReadLoop) processSettingsNoWrite(f *SettingsFrame) error { // connection can establish to our default. cc.maxConcurrentStreams = defaultMaxConcurrentStreams } + close(cc.seenSettingsChan) cc.seenSettings = true } diff --git a/http2/transport_test.go b/http2/transport_test.go index 498e27932..5e2e2eb1e 100644 --- a/http2/transport_test.go +++ b/http2/transport_test.go @@ -5421,3 +5421,62 @@ func TestIssue67671(t *testing.T) { res.Body.Close() } } + +func TestExtendedConnectClientWithServerSupport(t *testing.T) { + disableExtendedConnectProtocol = false + ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + t.Log(io.Copy(w, r.Body)) + }) + tr := &Transport{ + TLSClientConfig: tlsConfigInsecure, + AllowHTTP: true, + } + defer tr.CloseIdleConnections() + pr, pw := io.Pipe() + pwDone := make(chan struct{}) + req, _ := http.NewRequest("CONNECT", ts.URL, pr) + req.Header.Set(":protocol", "extended-connect") + go func() { + pw.Write([]byte("hello, extended connect")) + pw.Close() + close(pwDone) + }() + + res, err := tr.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + body, err := io.ReadAll(res.Body) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(body, []byte("hello, extended connect")) { + t.Fatal("unexpected body received") + } +} + +func TestExtendedConnectClientWithoutServerSupport(t *testing.T) { + disableExtendedConnectProtocol = true + ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + io.Copy(w, r.Body) + }) + tr := &Transport{ + TLSClientConfig: tlsConfigInsecure, + AllowHTTP: true, + } + defer tr.CloseIdleConnections() + pr, pw := io.Pipe() + pwDone := make(chan struct{}) + req, _ := http.NewRequest("CONNECT", ts.URL, pr) + req.Header.Set(":protocol", "extended-connect") + go func() { + pw.Write([]byte("hello, extended connect")) + pw.Close() + close(pwDone) + }() + + _, err := tr.RoundTrip(req) + if !errors.Is(err, errExtendedConnectNotSupported) { + t.Fatalf("expected error errExtendedConnectNotSupported, got: %v", err) + } +} From 4aac6770497d64ee256e1557eabb04f0b5c43fe4 Mon Sep 17 00:00:00 2001 From: Weidi Deng Date: Wed, 2 Oct 2024 14:33:28 +0800 Subject: [PATCH 5/6] update --- http2/frame.go | 2 +- http2/transport.go | 62 +++++++++++++++++++++++++---------------- http2/transport_test.go | 3 ++ 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/http2/frame.go b/http2/frame.go index 498163b05..81faec7e7 100644 --- a/http2/frame.go +++ b/http2/frame.go @@ -1498,7 +1498,7 @@ func (mh *MetaHeadersFrame) checkPseudos() error { return pseudoHeaderError(hf.Name) } // Check for duplicates. - // This would be a bad algorithm, but N is 4. + // This would be a bad algorithm, but N is 5. // And this doesn't allocate. for _, hf2 := range pf[:i] { if hf.Name == hf2.Name { diff --git a/http2/transport.go b/http2/transport.go index 7a52e3821..c16a9ffd0 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -364,7 +364,7 @@ type ClientConn struct { initialStreamRecvWindowSize int32 readIdleTimeout time.Duration pingTimeout time.Duration - extendedConnecAllowed bool + extendedConnectAllowed bool // reqHeaderMu is a 1-element semaphore channel controlling access to sending new requests. // Write to reqHeaderMu to lock it, read from it to unlock. @@ -1396,34 +1396,42 @@ func (cs *clientStream) writeRequest(req *http.Request, streamf func(*clientStre return err } + // wait for setting frames to be received, a server can change this value later, + // but we just wait for the first settings frame + var isExtendedConnect bool + if req.Method == "CONNECT" && req.Header.Get(":protocol") != "" { + isExtendedConnect = true + } + // Acquire the new-request lock by writing to reqHeaderMu. // This lock guards the critical section covering allocating a new stream ID // (requires mu) and creating the stream (requires wmu). if cc.reqHeaderMu == nil { panic("RoundTrip on uninitialized ClientConn") // for tests } - select { - case cc.reqHeaderMu <- struct{}{}: - case <-cs.reqCancel: - return errRequestCanceled - case <-ctx.Done(): - return ctx.Err() - } - - // wait for setting frames to be received, a server can change this value later, - // but we just wait for the first settings frame - var isExtendedConnect bool - if req.Method == "CONNECT" && req.Header.Get(":protocol") != "" { - isExtendedConnect = true - <-cc.seenSettingsChan + if isExtendedConnect { + select { + case cc.reqHeaderMu <- struct{}{}: + case <-cs.reqCancel: + return errRequestCanceled + case <-ctx.Done(): + return ctx.Err() + case <-cc.seenSettingsChan: + if !cc.extendedConnectAllowed { + return errExtendedConnectNotSupported + } + } + } else { + select { + case cc.reqHeaderMu <- struct{}{}: + case <-cs.reqCancel: + return errRequestCanceled + case <-ctx.Done(): + return ctx.Err() + } } cc.mu.Lock() - if isExtendedConnect && !cc.extendedConnecAllowed { - cc.mu.Unlock() - <-cc.reqHeaderMu - return errExtendedConnectNotSupported - } if cc.idleTimer != nil { cc.idleTimer.Stop() } @@ -2946,11 +2954,17 @@ func (rl *clientConnReadLoop) processSettingsNoWrite(f *SettingsFrame) error { if err := s.Valid(); err != nil { return err } - // RFC 8441 section, https://datatracker.ietf.org/doc/html/rfc8441#section-3 - if s.Val == 0 && cc.extendedConnecAllowed { - return ConnectionError(ErrCodeProtocol) + // If the peer wants to send us SETTINGS_ENABLE_CONNECT_PROTOCOL, + // we require that it do so in the first SETTINGS frame. + // + // When we attempt to use extended CONNECT, we wait for the first + // SETTINGS frame to see if the server supports it. If we let the + // server enable the feature with a later SETTINGS frame, then + // users will see inconsistent results depending on whether we've + // seen that frame or not. + if !cc.seenSettings { + cc.extendedConnectAllowed = s.Val == 1 } - cc.extendedConnecAllowed = s.Val == 1 default: cc.vlogf("Unhandled Setting: %v", s) } diff --git a/http2/transport_test.go b/http2/transport_test.go index 5e2e2eb1e..83bf262ae 100644 --- a/http2/transport_test.go +++ b/http2/transport_test.go @@ -5425,6 +5425,9 @@ func TestIssue67671(t *testing.T) { func TestExtendedConnectClientWithServerSupport(t *testing.T) { disableExtendedConnectProtocol = false ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get(":protocol") != "extended-connect" { + t.Fatalf("unexpected :protocol header received") + } t.Log(io.Copy(w, r.Body)) }) tr := &Transport{ From 23eded455f9b85f416e88c54c69c3120f2550ee8 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Fri, 22 Nov 2024 08:18:27 +0800 Subject: [PATCH 6/6] don't try to send to reqHeaderMu when waiting for extended connect --- http2/transport.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/http2/transport.go b/http2/transport.go index c16a9ffd0..3a9dbfd24 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -1411,7 +1411,6 @@ func (cs *clientStream) writeRequest(req *http.Request, streamf func(*clientStre } if isExtendedConnect { select { - case cc.reqHeaderMu <- struct{}{}: case <-cs.reqCancel: return errRequestCanceled case <-ctx.Done(): @@ -1421,14 +1420,13 @@ func (cs *clientStream) writeRequest(req *http.Request, streamf func(*clientStre return errExtendedConnectNotSupported } } - } else { - select { - case cc.reqHeaderMu <- struct{}{}: - case <-cs.reqCancel: - return errRequestCanceled - case <-ctx.Done(): - return ctx.Err() - } + } + select { + case cc.reqHeaderMu <- struct{}{}: + case <-cs.reqCancel: + return errRequestCanceled + case <-ctx.Done(): + return ctx.Err() } cc.mu.Lock()