diff --git a/go.mod b/go.mod index 7f512d7033..36207106dc 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.19.0 - golang.org/x/sys v0.17.0 - golang.org/x/term v0.17.0 + golang.org/x/crypto v0.21.0 + golang.org/x/sys v0.18.0 + golang.org/x/term v0.18.0 golang.org/x/text v0.14.0 ) diff --git a/go.sum b/go.sum index 683b469d6f..69fb104980 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,8 @@ -golang.org/x/crypto v0.19.0 h1:ENy+Az/9Y1vSrlrvBSyna3PITt4tiZLf7sgCjZBX7Wo= -golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/term v0.17.0 h1:mkTF7LCd6WGJNL3K1Ad7kwxNfYAW6a8a8QqtMblp/4U= -golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= +golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA= +golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= +golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= +golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8= +golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= diff --git a/http2/transport.go b/http2/transport.go index df578b86c6..c2a5b44b3d 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -2911,6 +2911,15 @@ func (rl *clientConnReadLoop) processWindowUpdate(f *WindowUpdateFrame) error { fl = &cs.flow } if !fl.add(int32(f.Increment)) { + // For stream, the sender sends RST_STREAM with an error code of FLOW_CONTROL_ERROR + if cs != nil { + rl.endStreamError(cs, StreamError{ + StreamID: f.StreamID, + Code: ErrCodeFlowControl, + }) + return nil + } + return ConnectionError(ErrCodeFlowControl) } cc.cond.Broadcast() diff --git a/internal/quic/cmd/interop/main.go b/internal/quic/cmd/interop/main.go index 20f737b525..5b652a2b15 100644 --- a/internal/quic/cmd/interop/main.go +++ b/internal/quic/cmd/interop/main.go @@ -25,8 +25,8 @@ import ( "path/filepath" "sync" - "golang.org/x/net/internal/quic" - "golang.org/x/net/internal/quic/qlog" + "golang.org/x/net/quic" + "golang.org/x/net/quic/qlog" ) var ( @@ -148,7 +148,7 @@ func basicTest(ctx context.Context, config *quic.Config, urls []string) { g.Add(1) go func() { defer g.Done() - fetchFrom(ctx, l, addr, u) + fetchFrom(ctx, config, l, addr, u) }() } @@ -221,8 +221,8 @@ func parseURL(s string) (u *url.URL, authority string, err error) { return u, authority, nil } -func fetchFrom(ctx context.Context, l *quic.Endpoint, addr string, urls []*url.URL) { - conn, err := l.Dial(ctx, "udp", addr) +func fetchFrom(ctx context.Context, config *quic.Config, l *quic.Endpoint, addr string, urls []*url.URL) { + conn, err := l.Dial(ctx, "udp", addr, config) if err != nil { log.Printf("%v: %v", addr, err) return diff --git a/internal/quic/doc.go b/internal/quic/doc.go deleted file mode 100644 index 2fe17fe226..0000000000 --- a/internal/quic/doc.go +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright 2023 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 quic is an experimental, incomplete implementation of the QUIC protocol. -// This package is a work in progress, and is not ready for use at this time. -// -// This package implements (or will implement) RFC 9000, RFC 9001, and RFC 9002. -package quic diff --git a/internal/quic/ack_delay.go b/quic/ack_delay.go similarity index 100% rename from internal/quic/ack_delay.go rename to quic/ack_delay.go diff --git a/internal/quic/ack_delay_test.go b/quic/ack_delay_test.go similarity index 100% rename from internal/quic/ack_delay_test.go rename to quic/ack_delay_test.go diff --git a/internal/quic/acks.go b/quic/acks.go similarity index 91% rename from internal/quic/acks.go rename to quic/acks.go index ba860efb2b..039b7b46e6 100644 --- a/internal/quic/acks.go +++ b/quic/acks.go @@ -130,12 +130,19 @@ func (acks *ackState) mustAckImmediately(space numberSpace, num packetNumber) bo // there are no gaps. If it does not, there must be a gap. return true } - if acks.unackedAckEliciting >= 2 { - // "[...] after receiving at least two ack-eliciting packets." - // https://www.rfc-editor.org/rfc/rfc9000.html#section-13.2.2 - return true + // "[...] SHOULD send an ACK frame after receiving at least two ack-eliciting packets." + // https://www.rfc-editor.org/rfc/rfc9000.html#section-13.2.2 + // + // This ack frequency takes a substantial toll on performance, however. + // Follow the behavior of Google QUICHE: + // Ack every other packet for the first 100 packets, and then ack every 10th packet. + // This keeps ack frequency high during the beginning of slow start when CWND is + // increasing rapidly. + packetsBeforeAck := 2 + if acks.seen.max() > 100 { + packetsBeforeAck = 10 } - return false + return acks.unackedAckEliciting >= packetsBeforeAck } // shouldSendAck reports whether the connection should send an ACK frame at this time, diff --git a/internal/quic/acks_test.go b/quic/acks_test.go similarity index 94% rename from internal/quic/acks_test.go rename to quic/acks_test.go index 4f1032910f..d10f917ad9 100644 --- a/internal/quic/acks_test.go +++ b/quic/acks_test.go @@ -7,6 +7,7 @@ package quic import ( + "slices" "testing" "time" ) @@ -198,7 +199,7 @@ func TestAcksSent(t *testing.T) { if len(gotNums) == 0 { wantDelay = 0 } - if !slicesEqual(gotNums, test.wantAcks) || gotDelay != wantDelay { + if !slices.Equal(gotNums, test.wantAcks) || gotDelay != wantDelay { t.Errorf("acks.acksToSend(T+%v) = %v, %v; want %v, %v", delay, gotNums, gotDelay, test.wantAcks, wantDelay) } } @@ -206,20 +207,6 @@ func TestAcksSent(t *testing.T) { } } -// slicesEqual reports whether two slices are equal. -// Replace this with slices.Equal once the module go.mod is go1.17 or newer. -func slicesEqual[E comparable](s1, s2 []E) bool { - if len(s1) != len(s2) { - return false - } - for i := range s1 { - if s1[i] != s2[i] { - return false - } - } - return true -} - func TestAcksDiscardAfterAck(t *testing.T) { acks := ackState{} now := time.Now() diff --git a/internal/quic/atomic_bits.go b/quic/atomic_bits.go similarity index 100% rename from internal/quic/atomic_bits.go rename to quic/atomic_bits.go diff --git a/quic/bench_test.go b/quic/bench_test.go new file mode 100644 index 0000000000..636b71327e --- /dev/null +++ b/quic/bench_test.go @@ -0,0 +1,170 @@ +// Copyright 2023 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.21 + +package quic + +import ( + "context" + "fmt" + "io" + "math" + "sync" + "testing" +) + +// BenchmarkThroughput is based on the crypto/tls benchmark of the same name. +func BenchmarkThroughput(b *testing.B) { + for size := 1; size <= 64; size <<= 1 { + name := fmt.Sprintf("%dMiB", size) + b.Run(name, func(b *testing.B) { + throughput(b, int64(size<<20)) + }) + } +} + +func throughput(b *testing.B, totalBytes int64) { + // Same buffer size as crypto/tls's BenchmarkThroughput, for consistency. + const bufsize = 32 << 10 + + cli, srv := newLocalConnPair(b, &Config{}, &Config{}) + + go func() { + buf := make([]byte, bufsize) + for i := 0; i < b.N; i++ { + sconn, err := srv.AcceptStream(context.Background()) + if err != nil { + panic(fmt.Errorf("AcceptStream: %v", err)) + } + if _, err := io.CopyBuffer(sconn, sconn, buf); err != nil { + panic(fmt.Errorf("CopyBuffer: %v", err)) + } + sconn.Close() + } + }() + + b.SetBytes(totalBytes) + buf := make([]byte, bufsize) + chunks := int(math.Ceil(float64(totalBytes) / float64(len(buf)))) + for i := 0; i < b.N; i++ { + cconn, err := cli.NewStream(context.Background()) + if err != nil { + b.Fatalf("NewStream: %v", err) + } + closec := make(chan struct{}) + go func() { + defer close(closec) + buf := make([]byte, bufsize) + if _, err := io.CopyBuffer(io.Discard, cconn, buf); err != nil { + panic(fmt.Errorf("Discard: %v", err)) + } + }() + for j := 0; j < chunks; j++ { + _, err := cconn.Write(buf) + if err != nil { + b.Fatalf("Write: %v", err) + } + } + cconn.CloseWrite() + <-closec + cconn.Close() + } +} + +func BenchmarkReadByte(b *testing.B) { + cli, srv := newLocalConnPair(b, &Config{}, &Config{}) + + var wg sync.WaitGroup + defer wg.Wait() + + wg.Add(1) + go func() { + defer wg.Done() + buf := make([]byte, 1<<20) + sconn, err := srv.AcceptStream(context.Background()) + if err != nil { + panic(fmt.Errorf("AcceptStream: %v", err)) + } + for { + if _, err := sconn.Write(buf); err != nil { + break + } + sconn.Flush() + } + }() + + b.SetBytes(1) + cconn, err := cli.NewStream(context.Background()) + if err != nil { + b.Fatalf("NewStream: %v", err) + } + cconn.Flush() + for i := 0; i < b.N; i++ { + _, err := cconn.ReadByte() + if err != nil { + b.Fatalf("ReadByte: %v", err) + } + } + cconn.Close() +} + +func BenchmarkWriteByte(b *testing.B) { + cli, srv := newLocalConnPair(b, &Config{}, &Config{}) + + var wg sync.WaitGroup + defer wg.Wait() + + wg.Add(1) + go func() { + defer wg.Done() + sconn, err := srv.AcceptStream(context.Background()) + if err != nil { + panic(fmt.Errorf("AcceptStream: %v", err)) + } + n, err := io.Copy(io.Discard, sconn) + if n != int64(b.N) || err != nil { + b.Errorf("server io.Copy() = %v, %v; want %v, nil", n, err, b.N) + } + }() + + b.SetBytes(1) + cconn, err := cli.NewStream(context.Background()) + if err != nil { + b.Fatalf("NewStream: %v", err) + } + cconn.Flush() + for i := 0; i < b.N; i++ { + if err := cconn.WriteByte(0); err != nil { + b.Fatalf("WriteByte: %v", err) + } + } + cconn.Close() +} + +func BenchmarkStreamCreation(b *testing.B) { + cli, srv := newLocalConnPair(b, &Config{}, &Config{}) + + go func() { + for i := 0; i < b.N; i++ { + sconn, err := srv.AcceptStream(context.Background()) + if err != nil { + panic(fmt.Errorf("AcceptStream: %v", err)) + } + sconn.Close() + } + }() + + buf := make([]byte, 1) + for i := 0; i < b.N; i++ { + cconn, err := cli.NewStream(context.Background()) + if err != nil { + b.Fatalf("NewStream: %v", err) + } + cconn.Write(buf) + cconn.Flush() + cconn.Read(buf) + cconn.Close() + } +} diff --git a/internal/quic/config.go b/quic/config.go similarity index 96% rename from internal/quic/config.go rename to quic/config.go index b045b7b92c..5d420312bb 100644 --- a/internal/quic/config.go +++ b/quic/config.go @@ -107,6 +107,13 @@ type Config struct { QLogLogger *slog.Logger } +// Clone returns a shallow clone of c, or nil if c is nil. +// It is safe to clone a [Config] that is being used concurrently by a QUIC endpoint. +func (c *Config) Clone() *Config { + n := *c + return &n +} + func configDefault[T ~int64](v, def, limit T) T { switch { case v == 0: diff --git a/internal/quic/config_test.go b/quic/config_test.go similarity index 100% rename from internal/quic/config_test.go rename to quic/config_test.go diff --git a/internal/quic/congestion_reno.go b/quic/congestion_reno.go similarity index 83% rename from internal/quic/congestion_reno.go rename to quic/congestion_reno.go index 982cbf4bb4..a539835247 100644 --- a/internal/quic/congestion_reno.go +++ b/quic/congestion_reno.go @@ -7,6 +7,8 @@ package quic import ( + "context" + "log/slog" "math" "time" ) @@ -40,6 +42,9 @@ type ccReno struct { // true if we haven't sent that packet yet. sendOnePacketInRecovery bool + // inRecovery is set when we are in the recovery state. + inRecovery bool + // underutilized is set if the congestion window is underutilized // due to insufficient application data, flow control limits, or // anti-amplification limits. @@ -100,12 +105,19 @@ func (c *ccReno) canSend() bool { // congestion controller permits sending data, but no data is sent. // // https://www.rfc-editor.org/rfc/rfc9002#section-7.8 -func (c *ccReno) setUnderutilized(v bool) { +func (c *ccReno) setUnderutilized(log *slog.Logger, v bool) { + if c.underutilized == v { + return + } + oldState := c.state() c.underutilized = v + if logEnabled(log, QLogLevelPacket) { + logCongestionStateUpdated(log, oldState, c.state()) + } } // packetSent indicates that a packet has been sent. -func (c *ccReno) packetSent(now time.Time, space numberSpace, sent *sentPacket) { +func (c *ccReno) packetSent(now time.Time, log *slog.Logger, space numberSpace, sent *sentPacket) { if !sent.inFlight { return } @@ -185,7 +197,11 @@ func (c *ccReno) packetLost(now time.Time, space numberSpace, sent *sentPacket, } // packetBatchEnd is called at the end of processing a batch of acked or lost packets. -func (c *ccReno) packetBatchEnd(now time.Time, space numberSpace, rtt *rttState, maxAckDelay time.Duration) { +func (c *ccReno) packetBatchEnd(now time.Time, log *slog.Logger, space numberSpace, rtt *rttState, maxAckDelay time.Duration) { + if logEnabled(log, QLogLevelPacket) { + oldState := c.state() + defer func() { logCongestionStateUpdated(log, oldState, c.state()) }() + } if !c.ackLastLoss.IsZero() && !c.ackLastLoss.Before(c.recoveryStartTime) { // Enter the recovery state. // https://www.rfc-editor.org/rfc/rfc9002.html#section-7.3.2 @@ -196,8 +212,10 @@ func (c *ccReno) packetBatchEnd(now time.Time, space numberSpace, rtt *rttState, // Clear congestionPendingAcks to avoid increasing the congestion // window based on acks in a frame that sends us into recovery. c.congestionPendingAcks = 0 + c.inRecovery = true } else if c.congestionPendingAcks > 0 { // We are in slow start or congestion avoidance. + c.inRecovery = false if c.congestionWindow < c.slowStartThreshold { // When the congestion window is less than the slow start threshold, // we are in slow start and increase the window by the number of @@ -253,3 +271,38 @@ func (c *ccReno) minimumCongestionWindow() int { // https://www.rfc-editor.org/rfc/rfc9002.html#section-7.2-4 return 2 * c.maxDatagramSize } + +func logCongestionStateUpdated(log *slog.Logger, oldState, newState congestionState) { + if oldState == newState { + return + } + log.LogAttrs(context.Background(), QLogLevelPacket, + "recovery:congestion_state_updated", + slog.String("old", oldState.String()), + slog.String("new", newState.String()), + ) +} + +type congestionState string + +func (s congestionState) String() string { return string(s) } + +const ( + congestionSlowStart = congestionState("slow_start") + congestionCongestionAvoidance = congestionState("congestion_avoidance") + congestionApplicationLimited = congestionState("application_limited") + congestionRecovery = congestionState("recovery") +) + +func (c *ccReno) state() congestionState { + switch { + case c.inRecovery: + return congestionRecovery + case c.underutilized: + return congestionApplicationLimited + case c.congestionWindow < c.slowStartThreshold: + return congestionSlowStart + default: + return congestionCongestionAvoidance + } +} diff --git a/internal/quic/congestion_reno_test.go b/quic/congestion_reno_test.go similarity index 99% rename from internal/quic/congestion_reno_test.go rename to quic/congestion_reno_test.go index e9af6452ca..cda7a90a80 100644 --- a/internal/quic/congestion_reno_test.go +++ b/quic/congestion_reno_test.go @@ -470,7 +470,7 @@ func (c *ccTest) setRTT(smoothedRTT, rttvar time.Duration) { func (c *ccTest) setUnderutilized(v bool) { c.t.Helper() c.t.Logf("set underutilized = %v", v) - c.cc.setUnderutilized(v) + c.cc.setUnderutilized(nil, v) } func (c *ccTest) packetSent(space numberSpace, size int, fns ...func(*sentPacket)) *sentPacket { @@ -488,7 +488,7 @@ func (c *ccTest) packetSent(space numberSpace, size int, fns ...func(*sentPacket f(sent) } c.t.Logf("packet sent: num=%v.%v, size=%v", space, sent.num, sent.size) - c.cc.packetSent(c.now, space, sent) + c.cc.packetSent(c.now, nil, space, sent) return sent } @@ -519,7 +519,7 @@ func (c *ccTest) packetDiscarded(space numberSpace, sent *sentPacket) { func (c *ccTest) packetBatchEnd(space numberSpace) { c.t.Helper() c.t.Logf("(end of batch)") - c.cc.packetBatchEnd(c.now, space, &c.rtt, c.maxAckDelay) + c.cc.packetBatchEnd(c.now, nil, space, &c.rtt, c.maxAckDelay) } func (c *ccTest) wantCanSend(want bool) { diff --git a/internal/quic/conn.go b/quic/conn.go similarity index 96% rename from internal/quic/conn.go rename to quic/conn.go index 6d79013eb4..38e8fe8f4e 100644 --- a/internal/quic/conn.go +++ b/quic/conn.go @@ -25,6 +25,7 @@ type Conn struct { config *Config testHooks connTestHooks peerAddr netip.AddrPort + localAddr netip.AddrPort msgc chan any donec chan struct{} // closed when conn loop exits @@ -36,6 +37,7 @@ type Conn struct { connIDState connIDState loss lossState streams streamsState + path pathState // Packet protection keys, CRYPTO streams, and TLS state. keysInitial fixedKeyPair @@ -92,12 +94,12 @@ type newServerConnIDs struct { retrySrcConnID []byte // source from server's Retry } -func newConn(now time.Time, side connSide, cids newServerConnIDs, peerAddr netip.AddrPort, config *Config, e *Endpoint) (conn *Conn, _ error) { +func newConn(now time.Time, side connSide, cids newServerConnIDs, peerHostname string, peerAddr netip.AddrPort, config *Config, e *Endpoint) (conn *Conn, _ error) { c := &Conn{ side: side, endpoint: e, config: config, - peerAddr: peerAddr, + peerAddr: unmapAddrPort(peerAddr), msgc: make(chan any, 1), donec: make(chan struct{}), peerAckDelayExponent: -1, @@ -144,7 +146,7 @@ func newConn(now time.Time, side connSide, cids newServerConnIDs, peerAddr netip c.lifetimeInit() c.restartIdleTimer(now) - if err := c.startTLS(now, initialConnID, transportParameters{ + if err := c.startTLS(now, initialConnID, peerHostname, transportParameters{ initialSrcConnID: c.connIDState.srcConnID(), originalDstConnID: cids.originalDstConnID, retrySrcConnID: cids.retrySrcConnID, @@ -210,7 +212,7 @@ func (c *Conn) discardKeys(now time.Time, space numberSpace) { case handshakeSpace: c.keysHandshake.discard() } - c.loss.discardKeys(now, space) + c.loss.discardKeys(now, c.log, space) } // receiveTransportParameters applies transport parameters sent by the peer. @@ -317,7 +319,11 @@ func (c *Conn) loop(now time.Time) { } switch m := m.(type) { case *datagram: - c.handleDatagram(now, m) + if !c.handleDatagram(now, m) { + if c.logEnabled(QLogLevelPacket) { + c.logPacketDropped(m) + } + } m.recycle() case timerEvent: // A connection timer has expired. diff --git a/internal/quic/conn_async_test.go b/quic/conn_async_test.go similarity index 100% rename from internal/quic/conn_async_test.go rename to quic/conn_async_test.go diff --git a/internal/quic/conn_close.go b/quic/conn_close.go similarity index 100% rename from internal/quic/conn_close.go rename to quic/conn_close.go diff --git a/internal/quic/conn_close_test.go b/quic/conn_close_test.go similarity index 98% rename from internal/quic/conn_close_test.go rename to quic/conn_close_test.go index 63d4911e8a..2139750119 100644 --- a/internal/quic/conn_close_test.go +++ b/quic/conn_close_test.go @@ -249,8 +249,9 @@ func TestConnCloseUnblocksNewStream(t *testing.T) { func TestConnCloseUnblocksStreamRead(t *testing.T) { testConnCloseUnblocks(t, func(ctx context.Context, tc *testConn) error { s := newLocalStream(t, tc, bidiStream) + s.SetReadContext(ctx) buf := make([]byte, 16) - _, err := s.ReadContext(ctx, buf) + _, err := s.Read(buf) return err }, permissiveTransportParameters) } @@ -258,8 +259,9 @@ func TestConnCloseUnblocksStreamRead(t *testing.T) { func TestConnCloseUnblocksStreamWrite(t *testing.T) { testConnCloseUnblocks(t, func(ctx context.Context, tc *testConn) error { s := newLocalStream(t, tc, bidiStream) + s.SetWriteContext(ctx) buf := make([]byte, 32) - _, err := s.WriteContext(ctx, buf) + _, err := s.Write(buf) return err }, permissiveTransportParameters, func(c *Config) { c.MaxStreamWriteBufferSize = 16 @@ -269,11 +271,12 @@ func TestConnCloseUnblocksStreamWrite(t *testing.T) { func TestConnCloseUnblocksStreamClose(t *testing.T) { testConnCloseUnblocks(t, func(ctx context.Context, tc *testConn) error { s := newLocalStream(t, tc, bidiStream) + s.SetWriteContext(ctx) buf := make([]byte, 16) - _, err := s.WriteContext(ctx, buf) + _, err := s.Write(buf) if err != nil { return err } - return s.CloseContext(ctx) + return s.Close() }, permissiveTransportParameters) } diff --git a/internal/quic/conn_flow.go b/quic/conn_flow.go similarity index 100% rename from internal/quic/conn_flow.go rename to quic/conn_flow.go diff --git a/internal/quic/conn_flow_test.go b/quic/conn_flow_test.go similarity index 90% rename from internal/quic/conn_flow_test.go rename to quic/conn_flow_test.go index 39c879346c..260684bdbc 100644 --- a/internal/quic/conn_flow_test.go +++ b/quic/conn_flow_test.go @@ -12,39 +12,34 @@ import ( ) func TestConnInflowReturnOnRead(t *testing.T) { - ctx := canceledContext() tc, s := newTestConnAndRemoteStream(t, serverSide, uniStream, func(c *Config) { c.MaxConnReadBufferSize = 64 }) tc.writeFrames(packetType1RTT, debugFrameStream{ id: s.id, - data: make([]byte, 64), + data: make([]byte, 8), }) - const readSize = 8 - if n, err := s.ReadContext(ctx, make([]byte, readSize)); n != readSize || err != nil { - t.Fatalf("s.Read() = %v, %v; want %v, nil", n, err, readSize) - } - tc.wantFrame("available window increases, send a MAX_DATA", - packetType1RTT, debugFrameMaxData{ - max: 64 + readSize, - }) - if n, err := s.ReadContext(ctx, make([]byte, 64)); n != 64-readSize || err != nil { - t.Fatalf("s.Read() = %v, %v; want %v, nil", n, err, 64-readSize) + if n, err := s.Read(make([]byte, 8)); n != 8 || err != nil { + t.Fatalf("s.Read() = %v, %v; want %v, nil", n, err, 8) } tc.wantFrame("available window increases, send a MAX_DATA", packetType1RTT, debugFrameMaxData{ - max: 128, + max: 64 + 8, }) // Peer can write up to the new limit. tc.writeFrames(packetType1RTT, debugFrameStream{ id: s.id, - off: 64, + off: 8, data: make([]byte, 64), }) - tc.wantIdle("connection is idle") - if n, err := s.ReadContext(ctx, make([]byte, 64)); n != 64 || err != nil { - t.Fatalf("offset 64: s.Read() = %v, %v; want %v, nil", n, err, 64) + if n, err := s.Read(make([]byte, 64+1)); n != 64 { + t.Fatalf("s.Read() = %v, %v; want %v, anything", n, err, 64) } + tc.wantFrame("available window increases, send a MAX_DATA", + packetType1RTT, debugFrameMaxData{ + max: 64 + 8 + 64, + }) + tc.wantIdle("connection is idle") } func TestConnInflowReturnOnRacingReads(t *testing.T) { @@ -64,11 +59,11 @@ func TestConnInflowReturnOnRacingReads(t *testing.T) { tc.ignoreFrame(frameTypeAck) tc.writeFrames(packetType1RTT, debugFrameStream{ id: newStreamID(clientSide, uniStream, 0), - data: make([]byte, 32), + data: make([]byte, 16), }) tc.writeFrames(packetType1RTT, debugFrameStream{ id: newStreamID(clientSide, uniStream, 1), - data: make([]byte, 32), + data: make([]byte, 1), }) s1, err := tc.conn.AcceptStream(ctx) if err != nil { @@ -79,10 +74,10 @@ func TestConnInflowReturnOnRacingReads(t *testing.T) { t.Fatalf("conn.AcceptStream() = %v", err) } read1 := runAsync(tc, func(ctx context.Context) (int, error) { - return s1.ReadContext(ctx, make([]byte, 16)) + return s1.Read(make([]byte, 16)) }) read2 := runAsync(tc, func(ctx context.Context) (int, error) { - return s2.ReadContext(ctx, make([]byte, 1)) + return s2.Read(make([]byte, 1)) }) // This MAX_DATA might extend the window by 16 or 17, depending on // whether the second write occurs before the update happens. @@ -90,10 +85,10 @@ func TestConnInflowReturnOnRacingReads(t *testing.T) { packetType1RTT, debugFrameMaxData{}) tc.wantIdle("redundant MAX_DATA is not sent") if _, err := read1.result(); err != nil { - t.Errorf("ReadContext #1 = %v", err) + t.Errorf("Read #1 = %v", err) } if _, err := read2.result(); err != nil { - t.Errorf("ReadContext #2 = %v", err) + t.Errorf("Read #2 = %v", err) } } @@ -204,7 +199,6 @@ func TestConnInflowResetViolation(t *testing.T) { } func TestConnInflowMultipleStreams(t *testing.T) { - ctx := canceledContext() tc := newTestConn(t, serverSide, func(c *Config) { c.MaxConnReadBufferSize = 128 }) @@ -220,21 +214,26 @@ func TestConnInflowMultipleStreams(t *testing.T) { } { tc.writeFrames(packetType1RTT, debugFrameStream{ id: id, - data: make([]byte, 32), + data: make([]byte, 1), }) - s, err := tc.conn.AcceptStream(ctx) - if err != nil { - t.Fatalf("AcceptStream() = %v", err) - } + s := tc.acceptStream() streams = append(streams, s) - if n, err := s.ReadContext(ctx, make([]byte, 1)); err != nil || n != 1 { + if n, err := s.Read(make([]byte, 1)); err != nil || n != 1 { t.Fatalf("s.Read() = %v, %v; want 1, nil", n, err) } } tc.wantIdle("streams have read data, but not enough to update MAX_DATA") - if n, err := streams[0].ReadContext(ctx, make([]byte, 32)); err != nil || n != 31 { - t.Fatalf("s.Read() = %v, %v; want 31, nil", n, err) + for _, s := range streams { + tc.writeFrames(packetType1RTT, debugFrameStream{ + id: s.id, + off: 1, + data: make([]byte, 31), + }) + } + + if n, err := streams[0].Read(make([]byte, 32)); n != 31 { + t.Fatalf("s.Read() = %v, %v; want 31, anything", n, err) } tc.wantFrame("read enough data to trigger a MAX_DATA update", packetType1RTT, debugFrameMaxData{ diff --git a/internal/quic/conn_id.go b/quic/conn_id.go similarity index 100% rename from internal/quic/conn_id.go rename to quic/conn_id.go diff --git a/internal/quic/conn_id_test.go b/quic/conn_id_test.go similarity index 100% rename from internal/quic/conn_id_test.go rename to quic/conn_id_test.go diff --git a/internal/quic/conn_loss.go b/quic/conn_loss.go similarity index 96% rename from internal/quic/conn_loss.go rename to quic/conn_loss.go index 85bda314ec..623ebdd7c6 100644 --- a/internal/quic/conn_loss.go +++ b/quic/conn_loss.go @@ -20,6 +20,10 @@ import "fmt" // See RFC 9000, Section 13.3 for a complete list of information which is retransmitted on loss. // https://www.rfc-editor.org/rfc/rfc9000#section-13.3 func (c *Conn) handleAckOrLoss(space numberSpace, sent *sentPacket, fate packetFate) { + if fate == packetLost && c.logEnabled(QLogLevelPacket) { + c.logPacketLost(space, sent) + } + // The list of frames in a sent packet is marshaled into a buffer in the sentPacket // by the packetWriter. Unmarshal that buffer here. This code must be kept in sync with // packetWriter.append*. diff --git a/internal/quic/conn_loss_test.go b/quic/conn_loss_test.go similarity index 93% rename from internal/quic/conn_loss_test.go rename to quic/conn_loss_test.go index 818816335b..81d537803d 100644 --- a/internal/quic/conn_loss_test.go +++ b/quic/conn_loss_test.go @@ -308,9 +308,9 @@ func TestLostMaxDataFrame(t *testing.T) { tc.writeFrames(packetType1RTT, debugFrameStream{ id: s.id, off: 0, - data: make([]byte, maxWindowSize), + data: make([]byte, maxWindowSize-1), }) - if n, err := s.Read(buf[:maxWindowSize-1]); err != nil || n != maxWindowSize-1 { + if n, err := s.Read(buf[:maxWindowSize]); err != nil || n != maxWindowSize-1 { t.Fatalf("Read() = %v, %v; want %v, nil", n, err, maxWindowSize-1) } tc.wantFrame("conn window is extended after reading data", @@ -319,7 +319,12 @@ func TestLostMaxDataFrame(t *testing.T) { }) // MAX_DATA = 64, which is only one more byte, so we don't send the frame. - if n, err := s.Read(buf); err != nil || n != 1 { + tc.writeFrames(packetType1RTT, debugFrameStream{ + id: s.id, + off: maxWindowSize - 1, + data: make([]byte, 1), + }) + if n, err := s.Read(buf[:1]); err != nil || n != 1 { t.Fatalf("Read() = %v, %v; want %v, nil", n, err, 1) } tc.wantIdle("read doesn't extend window enough to send another MAX_DATA") @@ -348,9 +353,9 @@ func TestLostMaxStreamDataFrame(t *testing.T) { tc.writeFrames(packetType1RTT, debugFrameStream{ id: s.id, off: 0, - data: make([]byte, maxWindowSize), + data: make([]byte, maxWindowSize-1), }) - if n, err := s.Read(buf[:maxWindowSize-1]); err != nil || n != maxWindowSize-1 { + if n, err := s.Read(buf[:maxWindowSize]); err != nil || n != maxWindowSize-1 { t.Fatalf("Read() = %v, %v; want %v, nil", n, err, maxWindowSize-1) } tc.wantFrame("stream window is extended after reading data", @@ -360,6 +365,11 @@ func TestLostMaxStreamDataFrame(t *testing.T) { }) // MAX_STREAM_DATA = 64, which is only one more byte, so we don't send the frame. + tc.writeFrames(packetType1RTT, debugFrameStream{ + id: s.id, + off: maxWindowSize - 1, + data: make([]byte, 1), + }) if n, err := s.Read(buf); err != nil || n != 1 { t.Fatalf("Read() = %v, %v; want %v, nil", n, err, 1) } @@ -433,7 +443,8 @@ func TestLostMaxStreamsFrameMostRecent(t *testing.T) { if err != nil { t.Fatalf("AcceptStream() = %v", err) } - s.CloseContext(ctx) + s.SetWriteContext(ctx) + s.Close() if styp == bidiStream { tc.wantFrame("stream is closed", packetType1RTT, debugFrameStream{ @@ -480,7 +491,7 @@ func TestLostMaxStreamsFrameNotMostRecent(t *testing.T) { if err != nil { t.Fatalf("AcceptStream() = %v", err) } - if err := s.CloseContext(ctx); err != nil { + if err := s.Close(); err != nil { t.Fatalf("stream.Close() = %v", err) } tc.wantFrame("closing stream updates peer's MAX_STREAMS", @@ -512,7 +523,7 @@ func TestLostStreamDataBlockedFrame(t *testing.T) { }) w := runAsync(tc, func(ctx context.Context) (int, error) { - return s.WriteContext(ctx, []byte{0, 1, 2, 3}) + return s.Write([]byte{0, 1, 2, 3}) }) defer w.cancel() tc.wantFrame("write is blocked by flow control", @@ -564,7 +575,7 @@ func TestLostStreamDataBlockedFrameAfterStreamUnblocked(t *testing.T) { data := []byte{0, 1, 2, 3} w := runAsync(tc, func(ctx context.Context) (int, error) { - return s.WriteContext(ctx, data) + return s.Write(data) }) defer w.cancel() tc.wantFrame("write is blocked by flow control", @@ -652,6 +663,29 @@ func TestLostRetireConnectionIDFrame(t *testing.T) { }) } +func TestLostPathResponseFrame(t *testing.T) { + // "Responses to path validation using PATH_RESPONSE frames are sent just once." + // https://www.rfc-editor.org/rfc/rfc9000.html#section-13.3-3.12 + lostFrameTest(t, func(t *testing.T, pto bool) { + tc := newTestConn(t, clientSide) + tc.handshake() + tc.ignoreFrame(frameTypeAck) + tc.ignoreFrame(frameTypePing) + + data := pathChallengeData{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef} + tc.writeFrames(packetType1RTT, debugFramePathChallenge{ + data: data, + }) + tc.wantFrame("response to PATH_CHALLENGE", + packetType1RTT, debugFramePathResponse{ + data: data, + }) + + tc.triggerLossOrPTO(packetType1RTT, pto) + tc.wantIdle("lost PATH_RESPONSE frame is not retransmitted") + }) +} + func TestLostHandshakeDoneFrame(t *testing.T) { // "The HANDSHAKE_DONE frame MUST be retransmitted until it is acknowledged." // https://www.rfc-editor.org/rfc/rfc9000.html#section-13.3-3.16 diff --git a/internal/quic/conn_recv.go b/quic/conn_recv.go similarity index 86% rename from internal/quic/conn_recv.go rename to quic/conn_recv.go index 045bf861cd..b1354cd3a1 100644 --- a/internal/quic/conn_recv.go +++ b/quic/conn_recv.go @@ -13,11 +13,28 @@ import ( "time" ) -func (c *Conn) handleDatagram(now time.Time, dgram *datagram) { +func (c *Conn) handleDatagram(now time.Time, dgram *datagram) (handled bool) { + if !c.localAddr.IsValid() { + // We don't have any way to tell in the general case what address we're + // sending packets from. Set our address from the destination address of + // the first packet received from the peer. + c.localAddr = dgram.localAddr + } + if dgram.peerAddr.IsValid() && dgram.peerAddr != c.peerAddr { + if c.side == clientSide { + // "If a client receives packets from an unknown server address, + // the client MUST discard these packets." + // https://www.rfc-editor.org/rfc/rfc9000#section-9-6 + return false + } + // We currently don't support connection migration, + // so for now the server also drops packets from an unknown address. + return false + } buf := dgram.b c.loss.datagramReceived(now, len(buf)) if c.isDraining() { - return + return false } for len(buf) > 0 { var n int @@ -27,19 +44,19 @@ func (c *Conn) handleDatagram(now time.Time, dgram *datagram) { if c.side == serverSide && len(dgram.b) < paddedInitialDatagramSize { // Discard client-sent Initial packets in too-short datagrams. // https://www.rfc-editor.org/rfc/rfc9000#section-14.1-4 - return + return false } - n = c.handleLongHeader(now, ptype, initialSpace, c.keysInitial.r, buf) + n = c.handleLongHeader(now, dgram, ptype, initialSpace, c.keysInitial.r, buf) case packetTypeHandshake: - n = c.handleLongHeader(now, ptype, handshakeSpace, c.keysHandshake.r, buf) + n = c.handleLongHeader(now, dgram, ptype, handshakeSpace, c.keysHandshake.r, buf) case packetType1RTT: - n = c.handle1RTT(now, buf) + n = c.handle1RTT(now, dgram, buf) case packetTypeRetry: c.handleRetry(now, buf) - return + return true case packetTypeVersionNegotiation: c.handleVersionNegotiation(now, buf) - return + return true default: n = -1 } @@ -56,17 +73,20 @@ func (c *Conn) handleDatagram(now time.Time, dgram *datagram) { if len(buf) == len(dgram.b) && len(buf) > statelessResetTokenLen { var token statelessResetToken copy(token[:], buf[len(buf)-len(token):]) - c.handleStatelessReset(now, token) + if c.handleStatelessReset(now, token) { + return true + } } // Invalid data at the end of a datagram is ignored. - break + return false } c.idleHandlePacketReceived(now) buf = buf[n:] } + return true } -func (c *Conn) handleLongHeader(now time.Time, ptype packetType, space numberSpace, k fixedKeys, buf []byte) int { +func (c *Conn) handleLongHeader(now time.Time, dgram *datagram, ptype packetType, space numberSpace, k fixedKeys, buf []byte) int { if !k.isSet() { return skipLongHeaderPacket(buf) } @@ -105,7 +125,7 @@ func (c *Conn) handleLongHeader(now time.Time, ptype packetType, space numberSpa c.logLongPacketReceived(p, buf[:n]) } c.connIDState.handlePacket(c, p.ptype, p.srcConnID) - ackEliciting := c.handleFrames(now, ptype, space, p.payload) + ackEliciting := c.handleFrames(now, dgram, ptype, space, p.payload) c.acks[space].receive(now, space, p.num, ackEliciting) if p.ptype == packetTypeHandshake && c.side == serverSide { c.loss.validateClientAddress() @@ -118,7 +138,7 @@ func (c *Conn) handleLongHeader(now time.Time, ptype packetType, space numberSpa return n } -func (c *Conn) handle1RTT(now time.Time, buf []byte) int { +func (c *Conn) handle1RTT(now time.Time, dgram *datagram, buf []byte) int { if !c.keysAppData.canRead() { // 1-RTT packets extend to the end of the datagram, // so skip the remainder of the datagram if we can't parse this. @@ -155,7 +175,7 @@ func (c *Conn) handle1RTT(now time.Time, buf []byte) int { if c.logEnabled(QLogLevelPacket) { c.log1RTTPacketReceived(p, buf) } - ackEliciting := c.handleFrames(now, packetType1RTT, appDataSpace, p.payload) + ackEliciting := c.handleFrames(now, dgram, packetType1RTT, appDataSpace, p.payload) c.acks[appDataSpace].receive(now, appDataSpace, p.num, ackEliciting) return len(buf) } @@ -192,7 +212,7 @@ func (c *Conn) handleRetry(now time.Time, pkt []byte) { c.connIDState.handleRetryPacket(p.srcConnID) // We need to resend any data we've already sent in Initial packets. // We must not reuse already sent packet numbers. - c.loss.discardPackets(initialSpace, c.handleAckOrLoss) + c.loss.discardPackets(initialSpace, c.log, c.handleAckOrLoss) // TODO: Discard 0-RTT packets as well, once we support 0-RTT. } @@ -232,7 +252,7 @@ func (c *Conn) handleVersionNegotiation(now time.Time, pkt []byte) { c.abortImmediately(now, errVersionNegotiation) } -func (c *Conn) handleFrames(now time.Time, ptype packetType, space numberSpace, payload []byte) (ackEliciting bool) { +func (c *Conn) handleFrames(now time.Time, dgram *datagram, ptype packetType, space numberSpace, payload []byte) (ackEliciting bool) { if len(payload) == 0 { // "An endpoint MUST treat receipt of a packet containing no frames // as a connection error of type PROTOCOL_VIOLATION." @@ -353,6 +373,16 @@ func (c *Conn) handleFrames(now time.Time, ptype packetType, space numberSpace, return } n = c.handleRetireConnectionIDFrame(now, space, payload) + case frameTypePathChallenge: + if !frameOK(c, ptype, __01) { + return + } + n = c.handlePathChallengeFrame(now, dgram, space, payload) + case frameTypePathResponse: + if !frameOK(c, ptype, ___1) { + return + } + n = c.handlePathResponseFrame(now, space, payload) case frameTypeConnectionCloseTransport: // Transport CONNECTION_CLOSE is OK in all spaces. n = c.handleConnectionCloseTransportFrame(now, payload) @@ -416,7 +446,7 @@ func (c *Conn) handleAckFrame(now time.Time, space numberSpace, payload []byte) if c.peerAckDelayExponent >= 0 { delay = ackDelay.Duration(uint8(c.peerAckDelayExponent)) } - c.loss.receiveAckEnd(now, space, delay, c.handleAckOrLoss) + c.loss.receiveAckEnd(now, c.log, space, delay, c.handleAckOrLoss) if space == appDataSpace { c.keysAppData.handleAckFor(largest) } @@ -526,6 +556,24 @@ func (c *Conn) handleRetireConnectionIDFrame(now time.Time, space numberSpace, p return n } +func (c *Conn) handlePathChallengeFrame(now time.Time, dgram *datagram, space numberSpace, payload []byte) int { + data, n := consumePathChallengeFrame(payload) + if n < 0 { + return -1 + } + c.handlePathChallenge(now, dgram, data) + return n +} + +func (c *Conn) handlePathResponseFrame(now time.Time, space numberSpace, payload []byte) int { + data, n := consumePathResponseFrame(payload) + if n < 0 { + return -1 + } + c.handlePathResponse(now, data) + return n +} + func (c *Conn) handleConnectionCloseTransportFrame(now time.Time, payload []byte) int { code, _, reason, n := consumeConnectionCloseTransportFrame(payload) if n < 0 { @@ -562,10 +610,11 @@ func (c *Conn) handleHandshakeDoneFrame(now time.Time, space numberSpace, payloa var errStatelessReset = errors.New("received stateless reset") -func (c *Conn) handleStatelessReset(now time.Time, resetToken statelessResetToken) { +func (c *Conn) handleStatelessReset(now time.Time, resetToken statelessResetToken) (valid bool) { if !c.connIDState.isValidStatelessResetToken(resetToken) { - return + return false } c.setFinalError(errStatelessReset) c.enterDraining(now) + return true } diff --git a/internal/quic/conn_send.go b/quic/conn_send.go similarity index 95% rename from internal/quic/conn_send.go rename to quic/conn_send.go index ccb467591b..a87cac232e 100644 --- a/internal/quic/conn_send.go +++ b/quic/conn_send.go @@ -22,7 +22,10 @@ func (c *Conn) maybeSend(now time.Time) (next time.Time) { // Assumption: The congestion window is not underutilized. // If congestion control, pacing, and anti-amplification all permit sending, // but we have no packet to send, then we will declare the window underutilized. - c.loss.cc.setUnderutilized(false) + underutilized := false + defer func() { + c.loss.cc.setUnderutilized(c.log, underutilized) + }() // Send one datagram on each iteration of this loop, // until we hit a limit or run out of data to send. @@ -80,7 +83,6 @@ func (c *Conn) maybeSend(now time.Time) (next time.Time) { } sentInitial = c.w.finishProtectedLongHeaderPacket(pnumMaxAcked, c.keysInitial.w, p) if sentInitial != nil { - c.idleHandlePacketSent(now, sentInitial) // Client initial packets and ack-eliciting server initial packaets // need to be sent in a datagram padded to at least 1200 bytes. // We can't add the padding yet, however, since we may want to @@ -111,8 +113,7 @@ func (c *Conn) maybeSend(now time.Time) (next time.Time) { c.logPacketSent(packetTypeHandshake, pnum, p.srcConnID, p.dstConnID, c.w.packetLen(), c.w.payload()) } if sent := c.w.finishProtectedLongHeaderPacket(pnumMaxAcked, c.keysHandshake.w, p); sent != nil { - c.idleHandlePacketSent(now, sent) - c.loss.packetSent(now, handshakeSpace, sent) + c.packetSent(now, handshakeSpace, sent) if c.side == clientSide { // "[...] a client MUST discard Initial keys when it first // sends a Handshake packet [...]" @@ -142,8 +143,7 @@ func (c *Conn) maybeSend(now time.Time) (next time.Time) { c.logPacketSent(packetType1RTT, pnum, nil, dstConnID, c.w.packetLen(), c.w.payload()) } if sent := c.w.finish1RTTPacket(pnum, pnumMaxAcked, dstConnID, &c.keysAppData); sent != nil { - c.idleHandlePacketSent(now, sent) - c.loss.packetSent(now, appDataSpace, sent) + c.packetSent(now, appDataSpace, sent) } } @@ -152,7 +152,7 @@ func (c *Conn) maybeSend(now time.Time) (next time.Time) { if limit == ccOK { // We have nothing to send, and congestion control does not // block sending. The congestion window is underutilized. - c.loss.cc.setUnderutilized(true) + underutilized = true } return next } @@ -175,14 +175,22 @@ func (c *Conn) maybeSend(now time.Time) (next time.Time) { // with a Handshake packet, then we've discarded Initial keys // since constructing the packet and shouldn't record it as in-flight. if c.keysInitial.canWrite() { - c.loss.packetSent(now, initialSpace, sentInitial) + c.packetSent(now, initialSpace, sentInitial) } } - c.endpoint.sendDatagram(buf, c.peerAddr) + c.endpoint.sendDatagram(datagram{ + b: buf, + peerAddr: c.peerAddr, + }) } } +func (c *Conn) packetSent(now time.Time, space numberSpace, sent *sentPacket) { + c.idleHandlePacketSent(now, sent) + c.loss.packetSent(now, c.log, space, sent) +} + func (c *Conn) appendFrames(now time.Time, space numberSpace, pnum packetNumber, limit ccLimit) { if c.lifetime.localErr != nil { c.appendConnectionCloseFrame(now, space, c.lifetime.localErr) @@ -263,6 +271,13 @@ func (c *Conn) appendFrames(now time.Time, space numberSpace, pnum packetNumber, return } + // PATH_RESPONSE + if pad, ok := c.appendPathFrames(); !ok { + return + } else if pad { + defer c.w.appendPaddingTo(smallestMaxDatagramSize) + } + // All stream-related frames. This should come last in the packet, // so large amounts of STREAM data don't crowd out other frames // we may need to send. diff --git a/internal/quic/conn_send_test.go b/quic/conn_send_test.go similarity index 100% rename from internal/quic/conn_send_test.go rename to quic/conn_send_test.go diff --git a/internal/quic/conn_streams.go b/quic/conn_streams.go similarity index 100% rename from internal/quic/conn_streams.go rename to quic/conn_streams.go diff --git a/internal/quic/conn_streams_test.go b/quic/conn_streams_test.go similarity index 95% rename from internal/quic/conn_streams_test.go rename to quic/conn_streams_test.go index 6815e403ef..dc81ad9913 100644 --- a/internal/quic/conn_streams_test.go +++ b/quic/conn_streams_test.go @@ -230,8 +230,8 @@ func TestStreamsWriteQueueFairness(t *testing.T) { t.Fatal(err) } streams = append(streams, s) - if n, err := s.WriteContext(ctx, data); n != len(data) || err != nil { - t.Fatalf("s.WriteContext() = %v, %v; want %v, nil", n, err, len(data)) + if n, err := s.Write(data); n != len(data) || err != nil { + t.Fatalf("s.Write() = %v, %v; want %v, nil", n, err, len(data)) } // Wait for the stream to finish writing whatever frames it can before // congestion control blocks it. @@ -298,7 +298,7 @@ func TestStreamsShutdown(t *testing.T) { side: localStream, styp: uniStream, setup: func(t *testing.T, tc *testConn, s *Stream) { - s.CloseContext(canceledContext()) + s.Close() }, shutdown: func(t *testing.T, tc *testConn, s *Stream) { tc.writeAckForAll() @@ -311,7 +311,7 @@ func TestStreamsShutdown(t *testing.T) { tc.writeFrames(packetType1RTT, debugFrameResetStream{ id: s.id, }) - s.CloseContext(canceledContext()) + s.Close() }, shutdown: func(t *testing.T, tc *testConn, s *Stream) { tc.writeAckForAll() @@ -321,8 +321,8 @@ func TestStreamsShutdown(t *testing.T) { side: localStream, styp: bidiStream, setup: func(t *testing.T, tc *testConn, s *Stream) { - s.CloseContext(canceledContext()) - tc.wantIdle("all frames after CloseContext are ignored") + s.Close() + tc.wantIdle("all frames after Close are ignored") tc.writeAckForAll() }, shutdown: func(t *testing.T, tc *testConn, s *Stream) { @@ -335,13 +335,12 @@ func TestStreamsShutdown(t *testing.T) { side: remoteStream, styp: uniStream, setup: func(t *testing.T, tc *testConn, s *Stream) { - ctx := canceledContext() tc.writeFrames(packetType1RTT, debugFrameStream{ id: s.id, fin: true, }) - if n, err := s.ReadContext(ctx, make([]byte, 16)); n != 0 || err != io.EOF { - t.Errorf("ReadContext() = %v, %v; want 0, io.EOF", n, err) + if n, err := s.Read(make([]byte, 16)); n != 0 || err != io.EOF { + t.Errorf("Read() = %v, %v; want 0, io.EOF", n, err) } }, shutdown: func(t *testing.T, tc *testConn, s *Stream) { @@ -451,17 +450,14 @@ func TestStreamsCreateAndCloseRemote(t *testing.T) { id: op.id, }) case acceptOp: - s, err := tc.conn.AcceptStream(ctx) - if err != nil { - t.Fatalf("AcceptStream() = %q; want stream %v", err, stringID(op.id)) - } + s := tc.acceptStream() if s.id != op.id { - t.Fatalf("accepted stram %v; want stream %v", err, stringID(op.id)) + t.Fatalf("accepted stream %v; want stream %v", stringID(s.id), stringID(op.id)) } t.Logf("accepted stream %v", stringID(op.id)) // Immediately close the stream, so the stream becomes done when the // peer closes its end. - s.CloseContext(ctx) + s.Close() } p := tc.readPacket() if p != nil { diff --git a/internal/quic/conn_test.go b/quic/conn_test.go similarity index 98% rename from internal/quic/conn_test.go rename to quic/conn_test.go index ddf0740e22..f4f1818a64 100644 --- a/internal/quic/conn_test.go +++ b/quic/conn_test.go @@ -21,7 +21,7 @@ import ( "testing" "time" - "golang.org/x/net/internal/quic/qlog" + "golang.org/x/net/quic/qlog" ) var ( @@ -168,6 +168,7 @@ type testConn struct { sentDatagrams [][]byte sentPackets []*testPacket sentFrames []debugFrame + lastDatagram *testDatagram lastPacket *testPacket recvDatagram chan *datagram @@ -241,8 +242,10 @@ func newTestConn(t *testing.T, side connSide, opts ...any) *testConn { endpoint.configTestConn = configTestConn conn, err := endpoint.e.newConn( endpoint.now, + config, side, cids, + "", netip.MustParseAddrPort("127.0.0.1:443")) if err != nil { t.Fatal(err) @@ -382,6 +385,17 @@ func (tc *testConn) cleanup() { <-tc.conn.donec } +func (tc *testConn) acceptStream() *Stream { + tc.t.Helper() + s, err := tc.conn.AcceptStream(canceledContext()) + if err != nil { + tc.t.Fatalf("conn.AcceptStream() = %v, want stream", err) + } + s.SetReadContext(canceledContext()) + s.SetWriteContext(canceledContext()) + return s +} + func logDatagram(t *testing.T, text string, d *testDatagram) { t.Helper() if !*testVV { @@ -442,6 +456,7 @@ func (tc *testConn) writeFrames(ptype packetType, frames ...debugFrame) { dstConnID: dstConnID, srcConnID: tc.peerConnID, }}, + addr: tc.conn.peerAddr, } if ptype == packetTypeInitial && tc.conn.side == serverSide { d.paddedSize = 1200 @@ -564,6 +579,7 @@ func (tc *testConn) readDatagram() *testDatagram { } p.frames = frames } + tc.lastDatagram = d return d } @@ -645,6 +661,12 @@ func (tc *testConn) wantPacket(expectation string, want *testPacket) { } func packetEqual(a, b *testPacket) bool { + if a == nil && b == nil { + return true + } + if a == nil || b == nil { + return false + } ac := *a ac.frames = nil ac.header = 0 diff --git a/internal/quic/crypto_stream.go b/quic/crypto_stream.go similarity index 100% rename from internal/quic/crypto_stream.go rename to quic/crypto_stream.go diff --git a/internal/quic/crypto_stream_test.go b/quic/crypto_stream_test.go similarity index 100% rename from internal/quic/crypto_stream_test.go rename to quic/crypto_stream_test.go diff --git a/internal/quic/dgram.go b/quic/dgram.go similarity index 58% rename from internal/quic/dgram.go rename to quic/dgram.go index 79e6650fa4..6155893732 100644 --- a/internal/quic/dgram.go +++ b/quic/dgram.go @@ -12,10 +12,25 @@ import ( ) type datagram struct { - b []byte - addr netip.AddrPort + b []byte + localAddr netip.AddrPort + peerAddr netip.AddrPort + ecn ecnBits } +// Explicit Congestion Notification bits. +// +// https://www.rfc-editor.org/rfc/rfc3168.html#section-5 +type ecnBits byte + +const ( + ecnMask = 0b000000_11 + ecnNotECT = 0b000000_00 + ecnECT1 = 0b000000_01 + ecnECT0 = 0b000000_10 + ecnCE = 0b000000_11 +) + var datagramPool = sync.Pool{ New: func() any { return &datagram{ @@ -26,7 +41,9 @@ var datagramPool = sync.Pool{ func newDatagram() *datagram { m := datagramPool.Get().(*datagram) - m.b = m.b[:cap(m.b)] + *m = datagram{ + b: m.b[:cap(m.b)], + } return m } diff --git a/quic/doc.go b/quic/doc.go new file mode 100644 index 0000000000..2fd10f0878 --- /dev/null +++ b/quic/doc.go @@ -0,0 +1,45 @@ +// Copyright 2023 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 quic implements the QUIC protocol. +// +// This package is a work in progress. +// It is not ready for production usage. +// Its API is subject to change without notice. +// +// This package is low-level. +// Most users will use it indirectly through an HTTP/3 implementation. +// +// # Usage +// +// An [Endpoint] sends and receives traffic on a network address. +// Create an Endpoint to either accept inbound QUIC connections +// or create outbound ones. +// +// A [Conn] is a QUIC connection. +// +// A [Stream] is a QUIC stream, an ordered, reliable byte stream. +// +// # Cancelation +// +// All blocking operations may be canceled using a context.Context. +// When performing an operation with a canceled context, the operation +// will succeed if doing so does not require blocking. For example, +// reading from a stream will return data when buffered data is available, +// even if the stream context is canceled. +// +// # Limitations +// +// This package is a work in progress. +// Known limitations include: +// +// - Performance is untuned. +// - 0-RTT is not supported. +// - Address migration is not supported. +// - Server preferred addresses are not supported. +// - The latency spin bit is not supported. +// - Stream send/receive windows are configurable, +// but are fixed and do not adapt to available throughput. +// - Path MTU discovery is not implemented. +package quic diff --git a/internal/quic/endpoint.go b/quic/endpoint.go similarity index 79% rename from internal/quic/endpoint.go rename to quic/endpoint.go index 8ed67de542..a55336b240 100644 --- a/internal/quic/endpoint.go +++ b/quic/endpoint.go @@ -22,11 +22,11 @@ import ( // // Multiple goroutines may invoke methods on an Endpoint simultaneously. type Endpoint struct { - config *Config - udpConn udpConn - testHooks endpointTestHooks - resetGen statelessResetTokenGenerator - retry retryState + listenConfig *Config + packetConn packetConn + testHooks endpointTestHooks + resetGen statelessResetTokenGenerator + retry retryState acceptQueue queue[*Conn] // new inbound connections connsMap connsMap // only accessed by the listen loop @@ -42,19 +42,20 @@ type endpointTestHooks interface { newConn(c *Conn) } -// A udpConn is a UDP connection. -// It is implemented by net.UDPConn. -type udpConn interface { +// A packetConn is the interface to sending and receiving UDP packets. +type packetConn interface { Close() error - LocalAddr() net.Addr - ReadMsgUDPAddrPort(b, control []byte) (n, controln, flags int, _ netip.AddrPort, _ error) - WriteToUDPAddrPort(b []byte, addr netip.AddrPort) (int, error) + LocalAddr() netip.AddrPort + Read(f func(*datagram)) + Write(datagram) error } // Listen listens on a local network address. -// The configuration config must be non-nil. -func Listen(network, address string, config *Config) (*Endpoint, error) { - if config.TLSConfig == nil { +// +// The config is used to for connections accepted by the endpoint. +// If the config is nil, the endpoint will not accept connections. +func Listen(network, address string, listenConfig *Config) (*Endpoint, error) { + if listenConfig != nil && listenConfig.TLSConfig == nil { return nil, errors.New("TLSConfig is not set") } a, err := net.ResolveUDPAddr(network, address) @@ -65,21 +66,29 @@ func Listen(network, address string, config *Config) (*Endpoint, error) { if err != nil { return nil, err } - return newEndpoint(udpConn, config, nil) + pc, err := newNetUDPConn(udpConn) + if err != nil { + return nil, err + } + return newEndpoint(pc, listenConfig, nil) } -func newEndpoint(udpConn udpConn, config *Config, hooks endpointTestHooks) (*Endpoint, error) { +func newEndpoint(pc packetConn, config *Config, hooks endpointTestHooks) (*Endpoint, error) { e := &Endpoint{ - config: config, - udpConn: udpConn, - testHooks: hooks, - conns: make(map[*Conn]struct{}), - acceptQueue: newQueue[*Conn](), - closec: make(chan struct{}), - } - e.resetGen.init(config.StatelessResetKey) + listenConfig: config, + packetConn: pc, + testHooks: hooks, + conns: make(map[*Conn]struct{}), + acceptQueue: newQueue[*Conn](), + closec: make(chan struct{}), + } + var statelessResetKey [32]byte + if config != nil { + statelessResetKey = config.StatelessResetKey + } + e.resetGen.init(statelessResetKey) e.connsMap.init() - if config.RequireAddressValidation { + if config != nil && config.RequireAddressValidation { if err := e.retry.init(); err != nil { return nil, err } @@ -90,8 +99,7 @@ func newEndpoint(udpConn udpConn, config *Config, hooks endpointTestHooks) (*End // LocalAddr returns the local network address. func (e *Endpoint) LocalAddr() netip.AddrPort { - a, _ := e.udpConn.LocalAddr().(*net.UDPAddr) - return a.AddrPort() + return e.packetConn.LocalAddr() } // Close closes the Endpoint. @@ -114,7 +122,7 @@ func (e *Endpoint) Close(ctx context.Context) error { conns = append(conns, c) } if len(e.conns) == 0 { - e.udpConn.Close() + e.packetConn.Close() } } e.connsMu.Unlock() @@ -139,14 +147,15 @@ func (e *Endpoint) Accept(ctx context.Context) (*Conn, error) { } // Dial creates and returns a connection to a network address. -func (e *Endpoint) Dial(ctx context.Context, network, address string) (*Conn, error) { +// The config cannot be nil. +func (e *Endpoint) Dial(ctx context.Context, network, address string, config *Config) (*Conn, error) { u, err := net.ResolveUDPAddr(network, address) if err != nil { return nil, err } addr := u.AddrPort() addr = netip.AddrPortFrom(addr.Addr().Unmap(), addr.Port()) - c, err := e.newConn(time.Now(), clientSide, newServerConnIDs{}, addr) + c, err := e.newConn(time.Now(), config, clientSide, newServerConnIDs{}, address, addr) if err != nil { return nil, err } @@ -157,13 +166,13 @@ func (e *Endpoint) Dial(ctx context.Context, network, address string) (*Conn, er return c, nil } -func (e *Endpoint) newConn(now time.Time, side connSide, cids newServerConnIDs, peerAddr netip.AddrPort) (*Conn, error) { +func (e *Endpoint) newConn(now time.Time, config *Config, side connSide, cids newServerConnIDs, peerHostname string, peerAddr netip.AddrPort) (*Conn, error) { e.connsMu.Lock() defer e.connsMu.Unlock() if e.closing { return nil, errors.New("endpoint closed") } - c, err := newConn(now, side, cids, peerAddr, e.config, e) + c, err := newConn(now, side, cids, peerHostname, peerAddr, config, e) if err != nil { return nil, err } @@ -200,34 +209,18 @@ func (e *Endpoint) connDrained(c *Conn) { defer e.connsMu.Unlock() delete(e.conns, c) if e.closing && len(e.conns) == 0 { - e.udpConn.Close() + e.packetConn.Close() } } func (e *Endpoint) listen() { defer close(e.closec) - for { - m := newDatagram() - // TODO: Read and process the ECN (explicit congestion notification) field. - // https://tools.ietf.org/html/draft-ietf-quic-transport-32#section-13.4 - n, _, _, addr, err := e.udpConn.ReadMsgUDPAddrPort(m.b, nil) - if err != nil { - // The user has probably closed the endpoint. - // We currently don't surface errors from other causes; - // we could check to see if the endpoint has been closed and - // record the unexpected error if it has not. - return - } - if n == 0 { - continue - } + e.packetConn.Read(func(m *datagram) { if e.connsMap.updateNeeded.Load() { e.connsMap.applyUpdates() } - m.addr = addr - m.b = m.b[:n] e.handleDatagram(m) - } + }) } func (e *Endpoint) handleDatagram(m *datagram) { @@ -277,7 +270,7 @@ func (e *Endpoint) handleUnknownDestinationDatagram(m *datagram) { // If this is a 1-RTT packet, there's nothing productive we can do with it. // Send a stateless reset if possible. if !isLongHeader(m.b[0]) { - e.maybeSendStatelessReset(m.b, m.addr) + e.maybeSendStatelessReset(m.b, m.peerAddr) return } p, ok := parseGenericLongHeaderPacket(m.b) @@ -291,7 +284,7 @@ func (e *Endpoint) handleUnknownDestinationDatagram(m *datagram) { return default: // Unknown version. - e.sendVersionNegotiation(p, m.addr) + e.sendVersionNegotiation(p, m.peerAddr) return } if getPacketType(m.b) != packetTypeInitial { @@ -302,14 +295,18 @@ func (e *Endpoint) handleUnknownDestinationDatagram(m *datagram) { // https://www.rfc-editor.org/rfc/rfc9000#section-10.3-16 return } + if e.listenConfig == nil { + // We are not configured to accept connections. + return + } cids := newServerConnIDs{ srcConnID: p.srcConnID, dstConnID: p.dstConnID, } - if e.config.RequireAddressValidation { + if e.listenConfig.RequireAddressValidation { var ok bool cids.retrySrcConnID = p.dstConnID - cids.originalDstConnID, ok = e.validateInitialAddress(now, p, m.addr) + cids.originalDstConnID, ok = e.validateInitialAddress(now, p, m.peerAddr) if !ok { return } @@ -317,7 +314,7 @@ func (e *Endpoint) handleUnknownDestinationDatagram(m *datagram) { cids.originalDstConnID = p.dstConnID } var err error - c, err := e.newConn(now, serverSide, cids, m.addr) + c, err := e.newConn(now, e.listenConfig, serverSide, cids, "", m.peerAddr) if err != nil { // The accept queue is probably full. // We could send a CONNECTION_CLOSE to the peer to reject the connection. @@ -329,7 +326,7 @@ func (e *Endpoint) handleUnknownDestinationDatagram(m *datagram) { m = nil // don't recycle, sendMsg takes ownership } -func (e *Endpoint) maybeSendStatelessReset(b []byte, addr netip.AddrPort) { +func (e *Endpoint) maybeSendStatelessReset(b []byte, peerAddr netip.AddrPort) { if !e.resetGen.canReset { // Config.StatelessResetKey isn't set, so we don't send stateless resets. return @@ -370,17 +367,21 @@ func (e *Endpoint) maybeSendStatelessReset(b []byte, addr netip.AddrPort) { b[0] &^= headerFormLong // clear long header bit b[0] |= fixedBit // set fixed bit copy(b[len(b)-statelessResetTokenLen:], token[:]) - e.sendDatagram(b, addr) + e.sendDatagram(datagram{ + b: b, + peerAddr: peerAddr, + }) } -func (e *Endpoint) sendVersionNegotiation(p genericLongPacket, addr netip.AddrPort) { +func (e *Endpoint) sendVersionNegotiation(p genericLongPacket, peerAddr netip.AddrPort) { m := newDatagram() m.b = appendVersionNegotiation(m.b[:0], p.srcConnID, p.dstConnID, quicVersion1) - e.sendDatagram(m.b, addr) + m.peerAddr = peerAddr + e.sendDatagram(*m) m.recycle() } -func (e *Endpoint) sendConnectionClose(in genericLongPacket, addr netip.AddrPort, code transportError) { +func (e *Endpoint) sendConnectionClose(in genericLongPacket, peerAddr netip.AddrPort, code transportError) { keys := initialKeys(in.dstConnID, serverSide) var w packetWriter p := longPacket{ @@ -399,12 +400,14 @@ func (e *Endpoint) sendConnectionClose(in genericLongPacket, addr netip.AddrPort if len(buf) == 0 { return } - e.sendDatagram(buf, addr) + e.sendDatagram(datagram{ + b: buf, + peerAddr: peerAddr, + }) } -func (e *Endpoint) sendDatagram(p []byte, addr netip.AddrPort) error { - _, err := e.udpConn.WriteToUDPAddrPort(p, addr) - return err +func (e *Endpoint) sendDatagram(dgram datagram) error { + return e.packetConn.Write(dgram) } // A connsMap is an endpoint's mapping of conn ids and reset tokens to conns. diff --git a/internal/quic/endpoint_test.go b/quic/endpoint_test.go similarity index 89% rename from internal/quic/endpoint_test.go rename to quic/endpoint_test.go index 16c3e0bce7..d5f436e6d7 100644 --- a/internal/quic/endpoint_test.go +++ b/quic/endpoint_test.go @@ -12,12 +12,11 @@ import ( "crypto/tls" "io" "log/slog" - "net" "net/netip" "testing" "time" - "golang.org/x/net/internal/quic/qlog" + "golang.org/x/net/quic/qlog" ) func TestConnect(t *testing.T) { @@ -63,12 +62,13 @@ func TestStreamTransfer(t *testing.T) { } } -func newLocalConnPair(t *testing.T, conf1, conf2 *Config) (clientConn, serverConn *Conn) { +func newLocalConnPair(t testing.TB, conf1, conf2 *Config) (clientConn, serverConn *Conn) { t.Helper() ctx := context.Background() e1 := newLocalEndpoint(t, serverSide, conf1) e2 := newLocalEndpoint(t, clientSide, conf2) - c2, err := e2.Dial(ctx, "udp", e1.LocalAddr().String()) + conf2 = makeTestConfig(conf2, clientSide) + c2, err := e2.Dial(ctx, "udp", e1.LocalAddr().String(), conf2) if err != nil { t.Fatal(err) } @@ -79,11 +79,26 @@ func newLocalConnPair(t *testing.T, conf1, conf2 *Config) (clientConn, serverCon return c2, c1 } -func newLocalEndpoint(t *testing.T, side connSide, conf *Config) *Endpoint { +func newLocalEndpoint(t testing.TB, side connSide, conf *Config) *Endpoint { t.Helper() + conf = makeTestConfig(conf, side) + e, err := Listen("udp", "127.0.0.1:0", conf) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + e.Close(canceledContext()) + }) + return e +} + +func makeTestConfig(conf *Config, side connSide) *Config { + if conf == nil { + return nil + } + newConf := *conf + conf = &newConf if conf.TLSConfig == nil { - newConf := *conf - conf = &newConf conf.TLSConfig = newTestTLSConfig(side) } if conf.QLogLogger == nil { @@ -92,14 +107,7 @@ func newLocalEndpoint(t *testing.T, side connSide, conf *Config) *Endpoint { Dir: *qlogdir, })) } - e, err := Listen("udp", "127.0.0.1:0", conf) - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { - e.Close(canceledContext()) - }) - return e + return conf } type testEndpoint struct { @@ -190,13 +198,9 @@ func (te *testEndpoint) writeDatagram(d *testDatagram) { for len(buf) < d.paddedSize { buf = append(buf, 0) } - addr := d.addr - if !addr.IsValid() { - addr = testClientAddr - } te.write(&datagram{ - b: buf, - addr: addr, + b: buf, + peerAddr: d.addr, }) } @@ -303,25 +307,24 @@ func (te *testEndpointUDPConn) Close() error { return nil } -func (te *testEndpointUDPConn) LocalAddr() net.Addr { - return net.UDPAddrFromAddrPort(netip.MustParseAddrPort("127.0.0.1:443")) +func (te *testEndpointUDPConn) LocalAddr() netip.AddrPort { + return netip.MustParseAddrPort("127.0.0.1:443") } -func (te *testEndpointUDPConn) ReadMsgUDPAddrPort(b, control []byte) (n, controln, flags int, _ netip.AddrPort, _ error) { +func (te *testEndpointUDPConn) Read(f func(*datagram)) { for { select { case d, ok := <-te.recvc: if !ok { - return 0, 0, 0, netip.AddrPort{}, io.EOF + return } - n = copy(b, d.b) - return n, 0, 0, d.addr, nil + f(d) case <-te.idlec: } } } -func (te *testEndpointUDPConn) WriteToUDPAddrPort(b []byte, addr netip.AddrPort) (int, error) { - te.sentDatagrams = append(te.sentDatagrams, append([]byte(nil), b...)) - return len(b), nil +func (te *testEndpointUDPConn) Write(dgram datagram) error { + te.sentDatagrams = append(te.sentDatagrams, append([]byte(nil), dgram.b...)) + return nil } diff --git a/internal/quic/errors.go b/quic/errors.go similarity index 100% rename from internal/quic/errors.go rename to quic/errors.go diff --git a/internal/quic/files_test.go b/quic/files_test.go similarity index 100% rename from internal/quic/files_test.go rename to quic/files_test.go diff --git a/internal/quic/frame_debug.go b/quic/frame_debug.go similarity index 98% rename from internal/quic/frame_debug.go rename to quic/frame_debug.go index 0902c385f6..17234dd7cd 100644 --- a/internal/quic/frame_debug.go +++ b/quic/frame_debug.go @@ -77,6 +77,7 @@ func parseDebugFrame(b []byte) (f debugFrame, n int) { // debugFramePadding is a sequence of PADDING frames. type debugFramePadding struct { size int + to int // alternate for writing packets: pad to } func parseDebugFramePadding(b []byte) (f debugFramePadding, n int) { @@ -95,6 +96,10 @@ func (f debugFramePadding) write(w *packetWriter) bool { if w.avail() == 0 { return false } + if f.to > 0 { + w.appendPaddingTo(f.to) + return true + } for i := 0; i < f.size && w.avail() > 0; i++ { w.b = append(w.b, frameTypePadding) } @@ -584,7 +589,7 @@ func (f debugFrameRetireConnectionID) LogValue() slog.Value { // debugFramePathChallenge is a PATH_CHALLENGE frame. type debugFramePathChallenge struct { - data uint64 + data pathChallengeData } func parseDebugFramePathChallenge(b []byte) (f debugFramePathChallenge, n int) { @@ -593,7 +598,7 @@ func parseDebugFramePathChallenge(b []byte) (f debugFramePathChallenge, n int) { } func (f debugFramePathChallenge) String() string { - return fmt.Sprintf("PATH_CHALLENGE Data=%016x", f.data) + return fmt.Sprintf("PATH_CHALLENGE Data=%x", f.data) } func (f debugFramePathChallenge) write(w *packetWriter) bool { @@ -603,13 +608,13 @@ func (f debugFramePathChallenge) write(w *packetWriter) bool { func (f debugFramePathChallenge) LogValue() slog.Value { return slog.GroupValue( slog.String("frame_type", "path_challenge"), - slog.String("data", fmt.Sprintf("%016x", f.data)), + slog.String("data", fmt.Sprintf("%x", f.data)), ) } // debugFramePathResponse is a PATH_RESPONSE frame. type debugFramePathResponse struct { - data uint64 + data pathChallengeData } func parseDebugFramePathResponse(b []byte) (f debugFramePathResponse, n int) { @@ -618,7 +623,7 @@ func parseDebugFramePathResponse(b []byte) (f debugFramePathResponse, n int) { } func (f debugFramePathResponse) String() string { - return fmt.Sprintf("PATH_RESPONSE Data=%016x", f.data) + return fmt.Sprintf("PATH_RESPONSE Data=%x", f.data) } func (f debugFramePathResponse) write(w *packetWriter) bool { @@ -628,7 +633,7 @@ func (f debugFramePathResponse) write(w *packetWriter) bool { func (f debugFramePathResponse) LogValue() slog.Value { return slog.GroupValue( slog.String("frame_type", "path_response"), - slog.String("data", fmt.Sprintf("%016x", f.data)), + slog.String("data", fmt.Sprintf("%x", f.data)), ) } diff --git a/internal/quic/gate.go b/quic/gate.go similarity index 100% rename from internal/quic/gate.go rename to quic/gate.go diff --git a/internal/quic/gate_test.go b/quic/gate_test.go similarity index 100% rename from internal/quic/gate_test.go rename to quic/gate_test.go diff --git a/internal/quic/gotraceback_test.go b/quic/gotraceback_test.go similarity index 100% rename from internal/quic/gotraceback_test.go rename to quic/gotraceback_test.go diff --git a/internal/quic/idle.go b/quic/idle.go similarity index 100% rename from internal/quic/idle.go rename to quic/idle.go diff --git a/internal/quic/idle_test.go b/quic/idle_test.go similarity index 100% rename from internal/quic/idle_test.go rename to quic/idle_test.go diff --git a/internal/quic/key_update_test.go b/quic/key_update_test.go similarity index 100% rename from internal/quic/key_update_test.go rename to quic/key_update_test.go diff --git a/internal/quic/log.go b/quic/log.go similarity index 100% rename from internal/quic/log.go rename to quic/log.go diff --git a/internal/quic/loss.go b/quic/loss.go similarity index 90% rename from internal/quic/loss.go rename to quic/loss.go index a59081fd57..796b5f7a34 100644 --- a/internal/quic/loss.go +++ b/quic/loss.go @@ -7,6 +7,8 @@ package quic import ( + "context" + "log/slog" "math" "time" ) @@ -179,7 +181,7 @@ func (c *lossState) nextNumber(space numberSpace) packetNumber { } // packetSent records a sent packet. -func (c *lossState) packetSent(now time.Time, space numberSpace, sent *sentPacket) { +func (c *lossState) packetSent(now time.Time, log *slog.Logger, space numberSpace, sent *sentPacket) { sent.time = now c.spaces[space].add(sent) size := sent.size @@ -187,13 +189,16 @@ func (c *lossState) packetSent(now time.Time, space numberSpace, sent *sentPacke c.antiAmplificationLimit = max(0, c.antiAmplificationLimit-size) } if sent.inFlight { - c.cc.packetSent(now, space, sent) + c.cc.packetSent(now, log, space, sent) c.pacer.packetSent(now, size, c.cc.congestionWindow, c.rtt.smoothedRTT) if sent.ackEliciting { c.spaces[space].lastAckEliciting = sent.num c.ptoExpired = false // reset expired PTO timer after sending probe } c.scheduleTimer(now) + if logEnabled(log, QLogLevelPacket) { + logBytesInFlight(log, c.cc.bytesInFlight) + } } if sent.ackEliciting { c.consecutiveNonAckElicitingPackets = 0 @@ -267,7 +272,7 @@ func (c *lossState) receiveAckRange(now time.Time, space numberSpace, rangeIndex // receiveAckEnd finishes processing an ack frame. // The lossf function is called for each packet newly detected as lost. -func (c *lossState) receiveAckEnd(now time.Time, space numberSpace, ackDelay time.Duration, lossf func(numberSpace, *sentPacket, packetFate)) { +func (c *lossState) receiveAckEnd(now time.Time, log *slog.Logger, space numberSpace, ackDelay time.Duration, lossf func(numberSpace, *sentPacket, packetFate)) { c.spaces[space].sentPacketList.clean() // Update the RTT sample when the largest acknowledged packet in the ACK frame // is newly acknowledged, and at least one newly acknowledged packet is ack-eliciting. @@ -286,13 +291,30 @@ func (c *lossState) receiveAckEnd(now time.Time, space numberSpace, ackDelay tim // https://www.rfc-editor.org/rfc/rfc9002.html#section-6.2.2.1-3 c.timer = time.Time{} c.detectLoss(now, lossf) - c.cc.packetBatchEnd(now, space, &c.rtt, c.maxAckDelay) + c.cc.packetBatchEnd(now, log, space, &c.rtt, c.maxAckDelay) + + if logEnabled(log, QLogLevelPacket) { + var ssthresh slog.Attr + if c.cc.slowStartThreshold != math.MaxInt { + ssthresh = slog.Int("ssthresh", c.cc.slowStartThreshold) + } + log.LogAttrs(context.Background(), QLogLevelPacket, + "recovery:metrics_updated", + slog.Duration("min_rtt", c.rtt.minRTT), + slog.Duration("smoothed_rtt", c.rtt.smoothedRTT), + slog.Duration("latest_rtt", c.rtt.latestRTT), + slog.Duration("rtt_variance", c.rtt.rttvar), + slog.Int("congestion_window", c.cc.congestionWindow), + slog.Int("bytes_in_flight", c.cc.bytesInFlight), + ssthresh, + ) + } } // discardPackets declares that packets within a number space will not be delivered // and that data contained in them should be resent. // For example, after receiving a Retry packet we discard already-sent Initial packets. -func (c *lossState) discardPackets(space numberSpace, lossf func(numberSpace, *sentPacket, packetFate)) { +func (c *lossState) discardPackets(space numberSpace, log *slog.Logger, lossf func(numberSpace, *sentPacket, packetFate)) { for i := 0; i < c.spaces[space].size; i++ { sent := c.spaces[space].nth(i) sent.lost = true @@ -300,10 +322,13 @@ func (c *lossState) discardPackets(space numberSpace, lossf func(numberSpace, *s lossf(numberSpace(space), sent, packetLost) } c.spaces[space].clean() + if logEnabled(log, QLogLevelPacket) { + logBytesInFlight(log, c.cc.bytesInFlight) + } } // discardKeys is called when dropping packet protection keys for a number space. -func (c *lossState) discardKeys(now time.Time, space numberSpace) { +func (c *lossState) discardKeys(now time.Time, log *slog.Logger, space numberSpace) { // https://www.rfc-editor.org/rfc/rfc9002.html#section-6.4 for i := 0; i < c.spaces[space].size; i++ { sent := c.spaces[space].nth(i) @@ -313,6 +338,9 @@ func (c *lossState) discardKeys(now time.Time, space numberSpace) { c.spaces[space].maxAcked = -1 c.spaces[space].lastAckEliciting = -1 c.scheduleTimer(now) + if logEnabled(log, QLogLevelPacket) { + logBytesInFlight(log, c.cc.bytesInFlight) + } } func (c *lossState) lossDuration() time.Duration { @@ -459,3 +487,10 @@ func (c *lossState) ptoBasePeriod() time.Duration { } return pto } + +func logBytesInFlight(log *slog.Logger, bytesInFlight int) { + log.LogAttrs(context.Background(), QLogLevelPacket, + "recovery:metrics_updated", + slog.Int("bytes_in_flight", bytesInFlight), + ) +} diff --git a/internal/quic/loss_test.go b/quic/loss_test.go similarity index 99% rename from internal/quic/loss_test.go rename to quic/loss_test.go index efbf1649ec..1fb9662e4c 100644 --- a/internal/quic/loss_test.go +++ b/quic/loss_test.go @@ -1060,7 +1060,7 @@ func TestLossPersistentCongestion(t *testing.T) { maxDatagramSize: 1200, }) test.send(initialSpace, 0, testSentPacketSize(1200)) - test.c.cc.setUnderutilized(true) + test.c.cc.setUnderutilized(nil, true) test.advance(10 * time.Millisecond) test.ack(initialSpace, 0*time.Millisecond, i64range[packetNumber]{0, 1}) @@ -1377,7 +1377,7 @@ func (c *lossTest) setRTTVar(d time.Duration) { func (c *lossTest) setUnderutilized(v bool) { c.t.Logf("set congestion window underutilized: %v", v) - c.c.cc.setUnderutilized(v) + c.c.cc.setUnderutilized(nil, v) } func (c *lossTest) advance(d time.Duration) { @@ -1438,7 +1438,7 @@ func (c *lossTest) send(spaceID numberSpace, opts ...any) { sent := &sentPacket{} *sent = prototype sent.num = num - c.c.packetSent(c.now, spaceID, sent) + c.c.packetSent(c.now, nil, spaceID, sent) } } @@ -1462,7 +1462,7 @@ func (c *lossTest) ack(spaceID numberSpace, ackDelay time.Duration, rs ...i64ran c.t.Logf("ack %v delay=%v [%v,%v)", spaceID, ackDelay, r.start, r.end) c.c.receiveAckRange(c.now, spaceID, i, r.start, r.end, c.onAckOrLoss) } - c.c.receiveAckEnd(c.now, spaceID, ackDelay, c.onAckOrLoss) + c.c.receiveAckEnd(c.now, nil, spaceID, ackDelay, c.onAckOrLoss) } func (c *lossTest) onAckOrLoss(space numberSpace, sent *sentPacket, fate packetFate) { @@ -1491,7 +1491,7 @@ func (c *lossTest) discardKeys(spaceID numberSpace) { c.t.Helper() c.checkUnexpectedEvents() c.t.Logf("discard %s keys", spaceID) - c.c.discardKeys(c.now, spaceID) + c.c.discardKeys(c.now, nil, spaceID) } func (c *lossTest) setMaxAckDelay(d time.Duration) { diff --git a/internal/quic/main_test.go b/quic/main_test.go similarity index 100% rename from internal/quic/main_test.go rename to quic/main_test.go diff --git a/internal/quic/math.go b/quic/math.go similarity index 100% rename from internal/quic/math.go rename to quic/math.go diff --git a/internal/quic/pacer.go b/quic/pacer.go similarity index 100% rename from internal/quic/pacer.go rename to quic/pacer.go diff --git a/internal/quic/pacer_test.go b/quic/pacer_test.go similarity index 100% rename from internal/quic/pacer_test.go rename to quic/pacer_test.go diff --git a/internal/quic/packet.go b/quic/packet.go similarity index 100% rename from internal/quic/packet.go rename to quic/packet.go diff --git a/internal/quic/packet_codec_test.go b/quic/packet_codec_test.go similarity index 99% rename from internal/quic/packet_codec_test.go rename to quic/packet_codec_test.go index 475e18c1da..3b39795ef5 100644 --- a/internal/quic/packet_codec_test.go +++ b/quic/packet_codec_test.go @@ -15,7 +15,7 @@ import ( "testing" "time" - "golang.org/x/net/internal/quic/qlog" + "golang.org/x/net/quic/qlog" ) func TestParseLongHeaderPacket(t *testing.T) { @@ -517,7 +517,7 @@ func TestFrameEncodeDecode(t *testing.T) { s: "PATH_CHALLENGE Data=0123456789abcdef", j: `{"frame_type":"path_challenge","data":"0123456789abcdef"}`, f: debugFramePathChallenge{ - data: 0x0123456789abcdef, + data: pathChallengeData{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}, }, b: []byte{ 0x1a, // Type (i) = 0x1a, @@ -527,7 +527,7 @@ func TestFrameEncodeDecode(t *testing.T) { s: "PATH_RESPONSE Data=0123456789abcdef", j: `{"frame_type":"path_response","data":"0123456789abcdef"}`, f: debugFramePathResponse{ - data: 0x0123456789abcdef, + data: pathChallengeData{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}, }, b: []byte{ 0x1b, // Type (i) = 0x1b, diff --git a/internal/quic/packet_number.go b/quic/packet_number.go similarity index 100% rename from internal/quic/packet_number.go rename to quic/packet_number.go diff --git a/internal/quic/packet_number_test.go b/quic/packet_number_test.go similarity index 100% rename from internal/quic/packet_number_test.go rename to quic/packet_number_test.go diff --git a/internal/quic/packet_parser.go b/quic/packet_parser.go similarity index 98% rename from internal/quic/packet_parser.go rename to quic/packet_parser.go index 02ef9fb145..feef9eac7f 100644 --- a/internal/quic/packet_parser.go +++ b/quic/packet_parser.go @@ -463,18 +463,17 @@ func consumeRetireConnectionIDFrame(b []byte) (seq int64, n int) { return seq, n } -func consumePathChallengeFrame(b []byte) (data uint64, n int) { +func consumePathChallengeFrame(b []byte) (data pathChallengeData, n int) { n = 1 - var nn int - data, nn = consumeUint64(b[n:]) - if nn < 0 { - return 0, -1 + nn := copy(data[:], b[n:]) + if nn != len(data) { + return data, -1 } n += nn return data, n } -func consumePathResponseFrame(b []byte) (data uint64, n int) { +func consumePathResponseFrame(b []byte) (data pathChallengeData, n int) { return consumePathChallengeFrame(b) // identical frame format } diff --git a/internal/quic/packet_protection.go b/quic/packet_protection.go similarity index 100% rename from internal/quic/packet_protection.go rename to quic/packet_protection.go diff --git a/internal/quic/packet_protection_test.go b/quic/packet_protection_test.go similarity index 100% rename from internal/quic/packet_protection_test.go rename to quic/packet_protection_test.go diff --git a/internal/quic/packet_test.go b/quic/packet_test.go similarity index 100% rename from internal/quic/packet_test.go rename to quic/packet_test.go diff --git a/internal/quic/packet_writer.go b/quic/packet_writer.go similarity index 95% rename from internal/quic/packet_writer.go rename to quic/packet_writer.go index b4e54ce4b0..e4d71e622b 100644 --- a/internal/quic/packet_writer.go +++ b/quic/packet_writer.go @@ -141,7 +141,7 @@ func (w *packetWriter) finishProtectedLongHeaderPacket(pnumMaxAcked packetNumber hdr = appendPacketNumber(hdr, p.num, pnumMaxAcked) k.protect(hdr[w.pktOff:], w.b[len(hdr):], pnumOff-w.pktOff, p.num) - return w.finish(p.num) + return w.finish(p.ptype, p.num) } // start1RTTPacket starts writing a 1-RTT (short header) packet. @@ -183,7 +183,7 @@ func (w *packetWriter) finish1RTTPacket(pnum, pnumMaxAcked packetNumber, dstConn hdr = appendPacketNumber(hdr, pnum, pnumMaxAcked) w.padPacketLength(pnumLen) k.protect(hdr[w.pktOff:], w.b[len(hdr):], pnumOff-w.pktOff, pnum) - return w.finish(pnum) + return w.finish(packetType1RTT, pnum) } // padPacketLength pads out the payload of the current packet to the minimum size, @@ -204,9 +204,10 @@ func (w *packetWriter) padPacketLength(pnumLen int) int { } // finish finishes the current packet after protection is applied. -func (w *packetWriter) finish(pnum packetNumber) *sentPacket { +func (w *packetWriter) finish(ptype packetType, pnum packetNumber) *sentPacket { w.b = w.b[:len(w.b)+aeadOverhead] w.sent.size = len(w.b) - w.pktOff + w.sent.ptype = ptype w.sent.num = pnum sent := w.sent w.sent = nil @@ -242,10 +243,7 @@ func (w *packetWriter) appendPingFrame() (added bool) { return false } w.b = append(w.b, frameTypePing) - // Mark this packet as ack-eliciting and in-flight, - // but there's no need to record the presence of a PING frame in it. - w.sent.ackEliciting = true - w.sent.inFlight = true + w.sent.markAckEliciting() // no need to record the frame itself return true } @@ -387,11 +385,7 @@ func (w *packetWriter) appendStreamFrame(id streamID, off int64, size int, fin b w.b = appendVarint(w.b, uint64(size)) start := len(w.b) w.b = w.b[:start+size] - if fin { - w.sent.appendAckElicitingFrame(frameTypeStreamBase | streamFinBit) - } else { - w.sent.appendAckElicitingFrame(frameTypeStreamBase) - } + w.sent.appendAckElicitingFrame(typ & (frameTypeStreamBase | streamFinBit)) w.sent.appendInt(uint64(id)) w.sent.appendOffAndSize(off, size) return w.b[start:][:size], true @@ -498,23 +492,23 @@ func (w *packetWriter) appendRetireConnectionIDFrame(seq int64) (added bool) { return true } -func (w *packetWriter) appendPathChallengeFrame(data uint64) (added bool) { +func (w *packetWriter) appendPathChallengeFrame(data pathChallengeData) (added bool) { if w.avail() < 1+8 { return false } w.b = append(w.b, frameTypePathChallenge) - w.b = binary.BigEndian.AppendUint64(w.b, data) - w.sent.appendAckElicitingFrame(frameTypePathChallenge) + w.b = append(w.b, data[:]...) + w.sent.markAckEliciting() // no need to record the frame itself return true } -func (w *packetWriter) appendPathResponseFrame(data uint64) (added bool) { +func (w *packetWriter) appendPathResponseFrame(data pathChallengeData) (added bool) { if w.avail() < 1+8 { return false } w.b = append(w.b, frameTypePathResponse) - w.b = binary.BigEndian.AppendUint64(w.b, data) - w.sent.appendAckElicitingFrame(frameTypePathResponse) + w.b = append(w.b, data[:]...) + w.sent.markAckEliciting() // no need to record the frame itself return true } diff --git a/quic/path.go b/quic/path.go new file mode 100644 index 0000000000..8c237dd45f --- /dev/null +++ b/quic/path.go @@ -0,0 +1,89 @@ +// 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.21 + +package quic + +import "time" + +type pathState struct { + // Response to a peer's PATH_CHALLENGE. + // This is not a sentVal, because we don't resend lost PATH_RESPONSE frames. + // We only track the most recent PATH_CHALLENGE. + // If the peer sends a second PATH_CHALLENGE before we respond to the first, + // we'll drop the first response. + sendPathResponse pathResponseType + data pathChallengeData +} + +// pathChallengeData is data carried in a PATH_CHALLENGE or PATH_RESPONSE frame. +type pathChallengeData [64 / 8]byte + +type pathResponseType uint8 + +const ( + pathResponseNotNeeded = pathResponseType(iota) + pathResponseSmall // send PATH_RESPONSE, do not expand datagram + pathResponseExpanded // send PATH_RESPONSE, expand datagram to 1200 bytes +) + +func (c *Conn) handlePathChallenge(_ time.Time, dgram *datagram, data pathChallengeData) { + // A PATH_RESPONSE is sent in a datagram expanded to 1200 bytes, + // except when this would exceed the anti-amplification limit. + // + // Rather than maintaining anti-amplification state for each path + // we may be sending a PATH_RESPONSE on, follow the following heuristic: + // + // If we receive a PATH_CHALLENGE in an expanded datagram, + // respond with an expanded datagram. + // + // If we receive a PATH_CHALLENGE in a non-expanded datagram, + // then the peer is presumably blocked by its own anti-amplification limit. + // Respond with a non-expanded datagram. Receiving this PATH_RESPONSE + // will validate the path to the peer, remove its anti-amplification limit, + // and permit it to send a followup PATH_CHALLENGE in an expanded datagram. + // https://www.rfc-editor.org/rfc/rfc9000.html#section-8.2.1 + if len(dgram.b) >= smallestMaxDatagramSize { + c.path.sendPathResponse = pathResponseExpanded + } else { + c.path.sendPathResponse = pathResponseSmall + } + c.path.data = data +} + +func (c *Conn) handlePathResponse(now time.Time, _ pathChallengeData) { + // "If the content of a PATH_RESPONSE frame does not match the content of + // a PATH_CHALLENGE frame previously sent by the endpoint, + // the endpoint MAY generate a connection error of type PROTOCOL_VIOLATION." + // https://www.rfc-editor.org/rfc/rfc9000.html#section-19.18-4 + // + // We never send PATH_CHALLENGE frames. + c.abort(now, localTransportError{ + code: errProtocolViolation, + reason: "PATH_RESPONSE received when no PATH_CHALLENGE sent", + }) +} + +// appendPathFrames appends path validation related frames to the current packet. +// If the return value pad is true, then the packet should be padded to 1200 bytes. +func (c *Conn) appendPathFrames() (pad, ok bool) { + if c.path.sendPathResponse == pathResponseNotNeeded { + return pad, true + } + // We're required to send the PATH_RESPONSE on the path where the + // PATH_CHALLENGE was received (RFC 9000, Section 8.2.2). + // + // At the moment, we don't support path migration and reject packets if + // the peer changes its source address, so just sending the PATH_RESPONSE + // in a regular datagram is fine. + if !c.w.appendPathResponseFrame(c.path.data) { + return pad, false + } + if c.path.sendPathResponse == pathResponseExpanded { + pad = true + } + c.path.sendPathResponse = pathResponseNotNeeded + return pad, true +} diff --git a/quic/path_test.go b/quic/path_test.go new file mode 100644 index 0000000000..a309ed14ba --- /dev/null +++ b/quic/path_test.go @@ -0,0 +1,66 @@ +// Copyright 2023 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.21 + +package quic + +import ( + "testing" +) + +func TestPathChallengeReceived(t *testing.T) { + for _, test := range []struct { + name string + padTo int + wantPadding int + }{{ + name: "unexpanded", + padTo: 0, + wantPadding: 0, + }, { + name: "expanded", + padTo: 1200, + wantPadding: 1200, + }} { + // "The recipient of [a PATH_CHALLENGE] frame MUST generate + // a PATH_RESPONSE frame [...] containing the same Data value." + // https://www.rfc-editor.org/rfc/rfc9000.html#section-19.17-7 + tc := newTestConn(t, clientSide) + tc.handshake() + tc.ignoreFrame(frameTypeAck) + data := pathChallengeData{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef} + tc.writeFrames(packetType1RTT, debugFramePathChallenge{ + data: data, + }, debugFramePadding{ + to: test.padTo, + }) + tc.wantFrame("response to PATH_CHALLENGE", + packetType1RTT, debugFramePathResponse{ + data: data, + }) + if got, want := tc.lastDatagram.paddedSize, test.wantPadding; got != want { + t.Errorf("PATH_RESPONSE expanded to %v bytes, want %v", got, want) + } + tc.wantIdle("connection is idle") + } +} + +func TestPathResponseMismatchReceived(t *testing.T) { + // "If the content of a PATH_RESPONSE frame does not match the content of + // a PATH_CHALLENGE frame previously sent by the endpoint, + // the endpoint MAY generate a connection error of type PROTOCOL_VIOLATION." + // https://www.rfc-editor.org/rfc/rfc9000.html#section-19.18-4 + tc := newTestConn(t, clientSide) + tc.handshake() + tc.ignoreFrame(frameTypeAck) + tc.writeFrames(packetType1RTT, debugFramePathResponse{ + data: pathChallengeData{}, + }) + tc.wantFrame("invalid PATH_RESPONSE causes the connection to close", + packetType1RTT, debugFrameConnectionCloseTransport{ + code: errProtocolViolation, + }, + ) +} diff --git a/internal/quic/ping.go b/quic/ping.go similarity index 100% rename from internal/quic/ping.go rename to quic/ping.go diff --git a/internal/quic/ping_test.go b/quic/ping_test.go similarity index 100% rename from internal/quic/ping_test.go rename to quic/ping_test.go diff --git a/internal/quic/pipe.go b/quic/pipe.go similarity index 71% rename from internal/quic/pipe.go rename to quic/pipe.go index d3a448df34..75cf76db21 100644 --- a/internal/quic/pipe.go +++ b/quic/pipe.go @@ -17,14 +17,14 @@ import ( // Writing past the end of the window extends it. // Data may be discarded from the start of the pipe, advancing the window. type pipe struct { - start int64 - end int64 - head *pipebuf - tail *pipebuf + start int64 // stream position of first stored byte + end int64 // stream position just past the last stored byte + head *pipebuf // if non-nil, then head.off + len(head.b) > start + tail *pipebuf // if non-nil, then tail.off + len(tail.b) == end } type pipebuf struct { - off int64 + off int64 // stream position of b[0] b []byte next *pipebuf } @@ -111,6 +111,7 @@ func (p *pipe) copy(off int64, b []byte) { // read calls f with the data in [off, off+n) // The data may be provided sequentially across multiple calls to f. +// Note that read (unlike an io.Reader) does not consume the read data. func (p *pipe) read(off int64, n int, f func([]byte) error) error { if off < p.start { panic("invalid read range") @@ -135,6 +136,30 @@ func (p *pipe) read(off int64, n int, f func([]byte) error) error { return nil } +// peek returns a reference to up to n bytes of internal data buffer, starting at p.start. +// The returned slice is valid until the next call to discardBefore. +// The length of the returned slice will be in the range [0,n]. +func (p *pipe) peek(n int64) []byte { + pb := p.head + if pb == nil { + return nil + } + b := pb.b[p.start-pb.off:] + return b[:min(int64(len(b)), n)] +} + +// availableBuffer returns the available contiguous, allocated buffer space +// following the pipe window. +// +// This is used by the stream write fast path, which makes multiple writes into the pipe buffer +// without a lock, and then adjusts p.end at a later time with a lock held. +func (p *pipe) availableBuffer() []byte { + if p.tail == nil { + return nil + } + return p.tail.b[p.end-p.tail.off:] +} + // discardBefore discards all data prior to off. func (p *pipe) discardBefore(off int64) { for p.head != nil && p.head.end() < off { diff --git a/internal/quic/pipe_test.go b/quic/pipe_test.go similarity index 100% rename from internal/quic/pipe_test.go rename to quic/pipe_test.go diff --git a/internal/quic/qlog.go b/quic/qlog.go similarity index 92% rename from internal/quic/qlog.go rename to quic/qlog.go index 82ad92ac8c..36831252c6 100644 --- a/internal/quic/qlog.go +++ b/quic/qlog.go @@ -39,7 +39,11 @@ const ( ) func (c *Conn) logEnabled(level slog.Level) bool { - return c.log != nil && c.log.Enabled(context.Background(), level) + return logEnabled(c.log, level) +} + +func logEnabled(log *slog.Logger, level slog.Level) bool { + return log != nil && log.Enabled(context.Background(), level) } // slogHexstring returns a slog.Attr for a value of the hexstring type. @@ -147,6 +151,12 @@ func (c *Conn) logConnectionClosed() { ) } +func (c *Conn) logPacketDropped(dgram *datagram) { + c.log.LogAttrs(context.Background(), QLogLevelPacket, + "connectivity:packet_dropped", + ) +} + func (c *Conn) logLongPacketReceived(p longPacket, pkt []byte) { var frames slog.Attr if c.logEnabled(QLogLevelFrame) { @@ -252,3 +262,13 @@ func (c *Conn) packetFramesAttr(payload []byte) slog.Attr { } return slog.Any("frames", frames) } + +func (c *Conn) logPacketLost(space numberSpace, sent *sentPacket) { + c.log.LogAttrs(context.Background(), QLogLevelPacket, + "recovery:packet_lost", + slog.Group("header", + slog.String("packet_type", sent.ptype.qlogString()), + slog.Uint64("packet_number", uint64(sent.num)), + ), + ) +} diff --git a/internal/quic/qlog/handler.go b/quic/qlog/handler.go similarity index 100% rename from internal/quic/qlog/handler.go rename to quic/qlog/handler.go diff --git a/internal/quic/qlog/json_writer.go b/quic/qlog/json_writer.go similarity index 99% rename from internal/quic/qlog/json_writer.go rename to quic/qlog/json_writer.go index b2fa3e03e5..6fb8d33b25 100644 --- a/internal/quic/qlog/json_writer.go +++ b/quic/qlog/json_writer.go @@ -45,15 +45,15 @@ func (w *jsonWriter) writeRecordEnd() { func (w *jsonWriter) writeAttrs(attrs []slog.Attr) { w.buf.WriteByte('{') for _, a := range attrs { - if a.Key == "" { - continue - } w.writeAttr(a) } w.buf.WriteByte('}') } func (w *jsonWriter) writeAttr(a slog.Attr) { + if a.Key == "" { + return + } w.writeName(a.Key) w.writeValue(a.Value) } diff --git a/internal/quic/qlog/json_writer_test.go b/quic/qlog/json_writer_test.go similarity index 96% rename from internal/quic/qlog/json_writer_test.go rename to quic/qlog/json_writer_test.go index 6da5566412..03cf6947ce 100644 --- a/internal/quic/qlog/json_writer_test.go +++ b/quic/qlog/json_writer_test.go @@ -85,6 +85,15 @@ func TestJSONWriterAttrs(t *testing.T) { `}}`) } +func TestJSONWriterAttrEmpty(t *testing.T) { + w := newTestJSONWriter() + w.writeRecordStart() + var a slog.Attr + w.writeAttr(a) + w.writeRecordEnd() + wantJSONRecord(t, w, `{}`) +} + func TestJSONWriterObjectEmpty(t *testing.T) { w := newTestJSONWriter() w.writeRecordStart() diff --git a/internal/quic/qlog/qlog.go b/quic/qlog/qlog.go similarity index 100% rename from internal/quic/qlog/qlog.go rename to quic/qlog/qlog.go diff --git a/internal/quic/qlog/qlog_test.go b/quic/qlog/qlog_test.go similarity index 100% rename from internal/quic/qlog/qlog_test.go rename to quic/qlog/qlog_test.go diff --git a/internal/quic/qlog_test.go b/quic/qlog_test.go similarity index 74% rename from internal/quic/qlog_test.go rename to quic/qlog_test.go index e98b118386..c0b5cd170f 100644 --- a/internal/quic/qlog_test.go +++ b/quic/qlog_test.go @@ -7,6 +7,7 @@ package quic import ( + "bytes" "encoding/hex" "encoding/json" "fmt" @@ -16,7 +17,7 @@ import ( "testing" "time" - "golang.org/x/net/internal/quic/qlog" + "golang.org/x/net/quic/qlog" ) func TestQLogHandshake(t *testing.T) { @@ -159,6 +160,98 @@ func TestQLogConnectionClosedTrigger(t *testing.T) { } } +func TestQLogRecovery(t *testing.T) { + qr := &qlogRecord{} + tc, s := newTestConnAndLocalStream(t, clientSide, uniStream, + permissiveTransportParameters, qr.config) + + // Ignore events from the handshake. + qr.ev = nil + + data := make([]byte, 16) + s.Write(data) + s.CloseWrite() + tc.wantFrame("created stream 0", + packetType1RTT, debugFrameStream{ + id: newStreamID(clientSide, uniStream, 0), + fin: true, + data: data, + }) + tc.writeAckForAll() + tc.wantIdle("connection should be idle now") + + // Don't check the contents of fields, but verify that recovery metrics are logged. + qr.wantEvents(t, jsonEvent{ + "name": "recovery:metrics_updated", + "data": map[string]any{ + "bytes_in_flight": nil, + }, + }, jsonEvent{ + "name": "recovery:metrics_updated", + "data": map[string]any{ + "bytes_in_flight": 0, + "congestion_window": nil, + "latest_rtt": nil, + "min_rtt": nil, + "rtt_variance": nil, + "smoothed_rtt": nil, + }, + }) +} + +func TestQLogLoss(t *testing.T) { + qr := &qlogRecord{} + tc, s := newTestConnAndLocalStream(t, clientSide, uniStream, + permissiveTransportParameters, qr.config) + + // Ignore events from the handshake. + qr.ev = nil + + data := make([]byte, 16) + s.Write(data) + s.CloseWrite() + tc.wantFrame("created stream 0", + packetType1RTT, debugFrameStream{ + id: newStreamID(clientSide, uniStream, 0), + fin: true, + data: data, + }) + + const pto = false + tc.triggerLossOrPTO(packetType1RTT, pto) + + qr.wantEvents(t, jsonEvent{ + "name": "recovery:packet_lost", + "data": map[string]any{ + "header": map[string]any{ + "packet_number": nil, + "packet_type": "1RTT", + }, + }, + }) +} + +func TestQLogPacketDropped(t *testing.T) { + qr := &qlogRecord{} + tc := newTestConn(t, clientSide, permissiveTransportParameters, qr.config) + tc.handshake() + + // A garbage-filled datagram with a DCID matching this connection. + dgram := bytes.Join([][]byte{ + {headerFormShort | fixedBit}, + testLocalConnID(0), + make([]byte, 100), + []byte{1, 2, 3, 4}, // random data, to avoid this looking like a stateless reset + }, nil) + tc.endpoint.write(&datagram{ + b: dgram, + }) + + qr.wantEvents(t, jsonEvent{ + "name": "connectivity:packet_dropped", + }) +} + type nopCloseWriter struct { io.Writer } @@ -193,14 +286,15 @@ func jsonPartialEqual(got, want any) (equal bool) { } return v } + if want == nil { + return true // match anything + } got = cmpval(got) want = cmpval(want) if reflect.TypeOf(got) != reflect.TypeOf(want) { return false } switch w := want.(type) { - case nil: - // Match anything. case map[string]any: // JSON object: Every field in want must match a field in got. g := got.(map[string]any) diff --git a/internal/quic/queue.go b/quic/queue.go similarity index 100% rename from internal/quic/queue.go rename to quic/queue.go diff --git a/internal/quic/queue_test.go b/quic/queue_test.go similarity index 100% rename from internal/quic/queue_test.go rename to quic/queue_test.go diff --git a/internal/quic/quic.go b/quic/quic.go similarity index 100% rename from internal/quic/quic.go rename to quic/quic.go diff --git a/internal/quic/quic_test.go b/quic/quic_test.go similarity index 100% rename from internal/quic/quic_test.go rename to quic/quic_test.go diff --git a/internal/quic/rangeset.go b/quic/rangeset.go similarity index 100% rename from internal/quic/rangeset.go rename to quic/rangeset.go diff --git a/internal/quic/rangeset_test.go b/quic/rangeset_test.go similarity index 100% rename from internal/quic/rangeset_test.go rename to quic/rangeset_test.go diff --git a/internal/quic/retry.go b/quic/retry.go similarity index 96% rename from internal/quic/retry.go rename to quic/retry.go index 31cb57b880..5dc39d1d9d 100644 --- a/internal/quic/retry.go +++ b/quic/retry.go @@ -139,7 +139,7 @@ func (rs *retryState) additionalData(srcConnID []byte, addr netip.AddrPort) []by return additional } -func (e *Endpoint) validateInitialAddress(now time.Time, p genericLongPacket, addr netip.AddrPort) (origDstConnID []byte, ok bool) { +func (e *Endpoint) validateInitialAddress(now time.Time, p genericLongPacket, peerAddr netip.AddrPort) (origDstConnID []byte, ok bool) { // The retry token is at the start of an Initial packet's data. token, n := consumeUint8Bytes(p.data) if n < 0 { @@ -151,22 +151,22 @@ func (e *Endpoint) validateInitialAddress(now time.Time, p genericLongPacket, ad if len(token) == 0 { // The sender has not provided a token. // Send a Retry packet to them with one. - e.sendRetry(now, p, addr) + e.sendRetry(now, p, peerAddr) return nil, false } - origDstConnID, ok = e.retry.validateToken(now, token, p.srcConnID, p.dstConnID, addr) + origDstConnID, ok = e.retry.validateToken(now, token, p.srcConnID, p.dstConnID, peerAddr) if !ok { // This does not seem to be a valid token. // Close the connection with an INVALID_TOKEN error. // https://www.rfc-editor.org/rfc/rfc9000#section-8.1.2-5 - e.sendConnectionClose(p, addr, errInvalidToken) + e.sendConnectionClose(p, peerAddr, errInvalidToken) return nil, false } return origDstConnID, true } -func (e *Endpoint) sendRetry(now time.Time, p genericLongPacket, addr netip.AddrPort) { - token, srcConnID, err := e.retry.makeToken(now, p.srcConnID, p.dstConnID, addr) +func (e *Endpoint) sendRetry(now time.Time, p genericLongPacket, peerAddr netip.AddrPort) { + token, srcConnID, err := e.retry.makeToken(now, p.srcConnID, p.dstConnID, peerAddr) if err != nil { return } @@ -175,7 +175,10 @@ func (e *Endpoint) sendRetry(now time.Time, p genericLongPacket, addr netip.Addr srcConnID: srcConnID, token: token, }) - e.sendDatagram(b, addr) + e.sendDatagram(datagram{ + b: b, + peerAddr: peerAddr, + }) } type retryPacket struct { diff --git a/internal/quic/retry_test.go b/quic/retry_test.go similarity index 99% rename from internal/quic/retry_test.go rename to quic/retry_test.go index 8f36e1bd3e..42f2bdd4a5 100644 --- a/internal/quic/retry_test.go +++ b/quic/retry_test.go @@ -436,8 +436,8 @@ func TestRetryClientIgnoresRetryWithInvalidIntegrityTag(t *testing.T) { }) pkt[len(pkt)-1] ^= 1 // invalidate the integrity tag tc.endpoint.write(&datagram{ - b: pkt, - addr: testClientAddr, + b: pkt, + peerAddr: testClientAddr, }) tc.wantIdle("client ignores Retry with invalid integrity tag") } diff --git a/internal/quic/rtt.go b/quic/rtt.go similarity index 100% rename from internal/quic/rtt.go rename to quic/rtt.go diff --git a/internal/quic/rtt_test.go b/quic/rtt_test.go similarity index 100% rename from internal/quic/rtt_test.go rename to quic/rtt_test.go diff --git a/internal/quic/sent_packet.go b/quic/sent_packet.go similarity index 90% rename from internal/quic/sent_packet.go rename to quic/sent_packet.go index 4f11aa1368..226152327d 100644 --- a/internal/quic/sent_packet.go +++ b/quic/sent_packet.go @@ -14,9 +14,10 @@ import ( // A sentPacket tracks state related to an in-flight packet we sent, // to be committed when the peer acks it or resent if the packet is lost. type sentPacket struct { - num packetNumber - size int // size in bytes - time time.Time // time sent + num packetNumber + size int // size in bytes + time time.Time // time sent + ptype packetType ackEliciting bool // https://www.rfc-editor.org/rfc/rfc9002.html#section-2-3.4.1 inFlight bool // https://www.rfc-editor.org/rfc/rfc9002.html#section-2-3.6.1 @@ -58,6 +59,12 @@ func (sent *sentPacket) reset() { } } +// markAckEliciting marks the packet as containing an ack-eliciting frame. +func (sent *sentPacket) markAckEliciting() { + sent.ackEliciting = true + sent.inFlight = true +} + // The append* methods record information about frames in the packet. func (sent *sentPacket) appendNonAckElicitingFrame(frameType byte) { diff --git a/internal/quic/sent_packet_list.go b/quic/sent_packet_list.go similarity index 100% rename from internal/quic/sent_packet_list.go rename to quic/sent_packet_list.go diff --git a/internal/quic/sent_packet_list_test.go b/quic/sent_packet_list_test.go similarity index 100% rename from internal/quic/sent_packet_list_test.go rename to quic/sent_packet_list_test.go diff --git a/internal/quic/sent_packet_test.go b/quic/sent_packet_test.go similarity index 100% rename from internal/quic/sent_packet_test.go rename to quic/sent_packet_test.go diff --git a/internal/quic/sent_val.go b/quic/sent_val.go similarity index 100% rename from internal/quic/sent_val.go rename to quic/sent_val.go diff --git a/internal/quic/sent_val_test.go b/quic/sent_val_test.go similarity index 100% rename from internal/quic/sent_val_test.go rename to quic/sent_val_test.go diff --git a/internal/quic/stateless_reset.go b/quic/stateless_reset.go similarity index 100% rename from internal/quic/stateless_reset.go rename to quic/stateless_reset.go diff --git a/internal/quic/stateless_reset_test.go b/quic/stateless_reset_test.go similarity index 99% rename from internal/quic/stateless_reset_test.go rename to quic/stateless_reset_test.go index 45a49e81e6..9458d2ea9d 100644 --- a/internal/quic/stateless_reset_test.go +++ b/quic/stateless_reset_test.go @@ -57,8 +57,8 @@ func newDatagramForReset(cid []byte, size int, addr netip.AddrPort) *datagram { dgram = append(dgram, byte(len(dgram))) // semi-random junk } return &datagram{ - b: dgram, - addr: addr, + b: dgram, + peerAddr: addr, } } diff --git a/internal/quic/stream.go b/quic/stream.go similarity index 81% rename from internal/quic/stream.go rename to quic/stream.go index fb9c1cf3c0..cb45534f82 100644 --- a/internal/quic/stream.go +++ b/quic/stream.go @@ -14,11 +14,31 @@ import ( "math" ) +// A Stream is an ordered byte stream. +// +// Streams may be bidirectional, read-only, or write-only. +// Methods inappropriate for a stream's direction +// (for example, [Write] to a read-only stream) +// return errors. +// +// It is not safe to perform concurrent reads from or writes to a stream. +// It is safe, however, to read and write at the same time. +// +// Reads and writes are buffered. +// It is generally not necessary to wrap a stream in a [bufio.ReadWriter] +// or otherwise apply additional buffering. +// +// To cancel reads or writes, use the [SetReadContext] and [SetWriteContext] methods. type Stream struct { id streamID conn *Conn - // ingate's lock guards all receive-related state. + // Contexts used for read/write operations. + // Intentionally not mutex-guarded, to allow the race detector to catch concurrent access. + inctx context.Context + outctx context.Context + + // ingate's lock guards receive-related state. // // The gate condition is set if a read from the stream will not block, // either because the stream has available data or because the read will fail. @@ -32,7 +52,7 @@ type Stream struct { inclosed sentVal // set by CloseRead inresetcode int64 // RESET_STREAM code received from the peer; -1 if not reset - // outgate's lock guards all send-related state. + // outgate's lock guards send-related state. // // The gate condition is set if a write to the stream will not block, // either because the stream has available flow control or because @@ -52,6 +72,12 @@ type Stream struct { outresetcode uint64 // reset code to send in RESET_STREAM outdone chan struct{} // closed when all data sent + // Unsynchronized buffers, used for lock-free fast path. + inbuf []byte // received data + inbufoff int // bytes of inbuf which have been consumed + outbuf []byte // written data + outbufoff int // bytes of outbuf which contain data to write + // Atomic stream state bits. // // These bits provide a fast way to coordinate between the @@ -152,6 +178,8 @@ func newStream(c *Conn, id streamID) *Stream { inresetcode: -1, // -1 indicates no RESET_STREAM received ingate: newLockedGate(), outgate: newLockedGate(), + inctx: context.Background(), + outctx: context.Background(), } if !s.IsReadOnly() { s.outdone = make(chan struct{}) @@ -159,6 +187,22 @@ func newStream(c *Conn, id streamID) *Stream { return s } +// SetReadContext sets the context used for reads from the stream. +// +// It is not safe to call SetReadContext concurrently. +func (s *Stream) SetReadContext(ctx context.Context) { + s.inctx = ctx +} + +// SetWriteContext sets the context used for writes to the stream. +// The write context is also used by Close when waiting for writes to be +// received by the peer. +// +// It is not safe to call SetWriteContext concurrently. +func (s *Stream) SetWriteContext(ctx context.Context) { + s.outctx = ctx +} + // IsReadOnly reports whether the stream is read-only // (a unidirectional stream created by the peer). func (s *Stream) IsReadOnly() bool { @@ -172,29 +216,42 @@ func (s *Stream) IsWriteOnly() bool { } // Read reads data from the stream. -// See ReadContext for more details. -func (s *Stream) Read(b []byte) (n int, err error) { - return s.ReadContext(context.Background(), b) -} - -// ReadContext reads data from the stream. // -// ReadContext returns as soon as at least one byte of data is available. +// Read returns as soon as at least one byte of data is available. // -// If the peer closes the stream cleanly, ReadContext returns io.EOF after +// If the peer closes the stream cleanly, Read returns io.EOF after // returning all data sent by the peer. -// If the peer aborts reads on the stream, ReadContext returns +// If the peer aborts reads on the stream, Read returns // an error wrapping StreamResetCode. -func (s *Stream) ReadContext(ctx context.Context, b []byte) (n int, err error) { +// +// It is not safe to call Read concurrently. +func (s *Stream) Read(b []byte) (n int, err error) { if s.IsWriteOnly() { return 0, errors.New("read from write-only stream") } - if err := s.ingate.waitAndLock(ctx, s.conn.testHooks); err != nil { + if len(s.inbuf) > s.inbufoff { + // Fast path: If s.inbuf contains unread bytes, return them immediately + // without taking a lock. + n = copy(b, s.inbuf[s.inbufoff:]) + s.inbufoff += n + return n, nil + } + if err := s.ingate.waitAndLock(s.inctx, s.conn.testHooks); err != nil { return 0, err } + if s.inbufoff > 0 { + // Discard bytes consumed by the fast path above. + s.in.discardBefore(s.in.start + int64(s.inbufoff)) + s.inbufoff = 0 + s.inbuf = nil + } + // bytesRead contains the number of bytes of connection-level flow control to return. + // We return flow control for bytes read by this Read call, as well as bytes moved + // to the fast-path read buffer (s.inbuf). + var bytesRead int64 defer func() { s.inUnlock() - s.conn.handleStreamBytesReadOffLoop(int64(n)) // must be done with ingate unlocked + s.conn.handleStreamBytesReadOffLoop(bytesRead) // must be done with ingate unlocked }() if s.inresetcode != -1 { return 0, fmt.Errorf("stream reset by peer: %w", StreamErrorCode(s.inresetcode)) @@ -212,22 +269,50 @@ func (s *Stream) ReadContext(ctx context.Context, b []byte) (n int, err error) { if size := int(s.inset[0].end - s.in.start); size < len(b) { b = b[:size] } + bytesRead = int64(len(b)) start := s.in.start end := start + int64(len(b)) s.in.copy(start, b) s.in.discardBefore(end) + if end == s.insize { + // We have read up to the end of the stream. + // No need to update stream flow control. + return len(b), io.EOF + } + if len(s.inset) > 0 && s.inset[0].start <= s.in.start && s.inset[0].end > s.in.start { + // If we have more readable bytes available, put the next chunk of data + // in s.inbuf for lock-free reads. + s.inbuf = s.in.peek(s.inset[0].end - s.in.start) + bytesRead += int64(len(s.inbuf)) + } if s.insize == -1 || s.insize > s.inwin { - if shouldUpdateFlowControl(s.inmaxbuf, s.in.start+s.inmaxbuf-s.inwin) { + newWindow := s.in.start + int64(len(s.inbuf)) + s.inmaxbuf + addedWindow := newWindow - s.inwin + if shouldUpdateFlowControl(s.inmaxbuf, addedWindow) { // Update stream flow control with a STREAM_MAX_DATA frame. s.insendmax.setUnsent() } } - if end == s.insize { - return len(b), io.EOF - } return len(b), nil } +// ReadByte reads and returns a single byte from the stream. +// +// It is not safe to call ReadByte concurrently. +func (s *Stream) ReadByte() (byte, error) { + if len(s.inbuf) > s.inbufoff { + b := s.inbuf[s.inbufoff] + s.inbufoff++ + return b, nil + } + var b [1]byte + n, err := s.Read(b[:]) + if n > 0 { + return b[0], err + } + return 0, err +} + // shouldUpdateFlowControl determines whether to send a flow control window update. // // We want to balance keeping the peer well-supplied with flow control with not sending @@ -237,21 +322,22 @@ func shouldUpdateFlowControl(maxWindow, addedWindow int64) bool { } // Write writes data to the stream. -// See WriteContext for more details. -func (s *Stream) Write(b []byte) (n int, err error) { - return s.WriteContext(context.Background(), b) -} - -// WriteContext writes data to the stream. // -// WriteContext writes data to the stream write buffer. +// Write writes data to the stream write buffer. // Buffered data is only sent when the buffer is sufficiently full. // Call the Flush method to ensure buffered data is sent. -func (s *Stream) WriteContext(ctx context.Context, b []byte) (n int, err error) { +func (s *Stream) Write(b []byte) (n int, err error) { if s.IsReadOnly() { return 0, errors.New("write to read-only stream") } + if len(b) > 0 && len(s.outbuf)-s.outbufoff >= len(b) { + // Fast path: The data to write fits in s.outbuf. + copy(s.outbuf[s.outbufoff:], b) + s.outbufoff += len(b) + return len(b), nil + } canWrite := s.outgate.lock() + s.flushFastOutputBuffer() for { // The first time through this loop, we may or may not be write blocked. // We exit the loop after writing all data, so on subsequent passes through @@ -259,7 +345,7 @@ func (s *Stream) WriteContext(ctx context.Context, b []byte) (n int, err error) if len(b) > 0 && !canWrite { // Our send buffer is full. Wait for the peer to ack some data. s.outUnlock() - if err := s.outgate.waitAndLock(ctx, s.conn.testHooks); err != nil { + if err := s.outgate.waitAndLock(s.outctx, s.conn.testHooks); err != nil { return n, err } // Successfully returning from waitAndLockGate means we are no longer @@ -311,13 +397,54 @@ func (s *Stream) WriteContext(ctx context.Context, b []byte) (n int, err error) // If we have bytes left to send, we're blocked. canWrite = false } + if lim := s.out.start + s.outmaxbuf - s.out.end - 1; lim > 0 { + // If s.out has space allocated and available to be written into, + // then reference it in s.outbuf for fast-path writes. + // + // It's perhaps a bit pointless to limit s.outbuf to the send buffer limit. + // We've already allocated this buffer so we aren't saving any memory + // by not using it. + // For now, we limit it anyway to make it easier to reason about limits. + // + // We set the limit to one less than the send buffer limit (the -1 above) + // so that a write which completely fills the buffer will overflow + // s.outbuf and trigger a flush. + s.outbuf = s.out.availableBuffer() + if int64(len(s.outbuf)) > lim { + s.outbuf = s.outbuf[:lim] + } + } s.outUnlock() return n, nil } +// WriteBytes writes a single byte to the stream. +func (s *Stream) WriteByte(c byte) error { + if s.outbufoff < len(s.outbuf) { + s.outbuf[s.outbufoff] = c + s.outbufoff++ + return nil + } + b := [1]byte{c} + _, err := s.Write(b[:]) + return err +} + +func (s *Stream) flushFastOutputBuffer() { + if s.outbuf == nil { + return + } + // Commit data previously written to s.outbuf. + // s.outbuf is a reference to a buffer in s.out, so we just need to record + // that the output buffer has been extended. + s.out.end += int64(s.outbufoff) + s.outbuf = nil + s.outbufoff = 0 +} + // Flush flushes data written to the stream. // It does not wait for the peer to acknowledge receipt of the data. -// Use CloseContext to wait for the peer's acknowledgement. +// Use Close to wait for the peer's acknowledgement. func (s *Stream) Flush() { s.outgate.lock() defer s.outUnlock() @@ -325,6 +452,7 @@ func (s *Stream) Flush() { } func (s *Stream) flushLocked() { + s.flushFastOutputBuffer() s.outopened.set() if s.outflushed < s.outwin { s.outunsent.add(s.outflushed, min(s.outwin, s.out.end)) @@ -333,27 +461,21 @@ func (s *Stream) flushLocked() { } // Close closes the stream. -// See CloseContext for more details. -func (s *Stream) Close() error { - return s.CloseContext(context.Background()) -} - -// CloseContext closes the stream. // Any blocked stream operations will be unblocked and return errors. // -// CloseContext flushes any data in the stream write buffer and waits for the peer to +// Close flushes any data in the stream write buffer and waits for the peer to // acknowledge receipt of the data. // If the stream has been reset, it waits for the peer to acknowledge the reset. // If the context expires before the peer receives the stream's data, -// CloseContext discards the buffer and returns the context error. -func (s *Stream) CloseContext(ctx context.Context) error { +// Close discards the buffer and returns the context error. +func (s *Stream) Close() error { s.CloseRead() if s.IsReadOnly() { return nil } s.CloseWrite() // TODO: Return code from peer's RESET_STREAM frame? - if err := s.conn.waitOnDone(ctx, s.outdone); err != nil { + if err := s.conn.waitOnDone(s.outctx, s.outdone); err != nil { return err } s.outgate.lock() @@ -369,7 +491,7 @@ func (s *Stream) CloseContext(ctx context.Context) error { // // CloseRead notifies the peer that the stream has been closed for reading. // It does not wait for the peer to acknowledge the closure. -// Use CloseContext to wait for the peer's acknowledgement. +// Use Close to wait for the peer's acknowledgement. func (s *Stream) CloseRead() { if s.IsWriteOnly() { return @@ -394,7 +516,7 @@ func (s *Stream) CloseRead() { // // CloseWrite sends any data in the stream write buffer to the peer. // It does not wait for the peer to acknowledge receipt of the data. -// Use CloseContext to wait for the peer's acknowledgement. +// Use Close to wait for the peer's acknowledgement. func (s *Stream) CloseWrite() { if s.IsReadOnly() { return @@ -412,7 +534,7 @@ func (s *Stream) CloseWrite() { // Reset sends the application protocol error code, which must be // less than 2^62, to the peer. // It does not wait for the peer to acknowledge receipt of the error. -// Use CloseContext to wait for the peer's acknowledgement. +// Use Close to wait for the peer's acknowledgement. // // Reset does not affect reads. // Use CloseRead to abort reads on the stream. @@ -446,6 +568,8 @@ func (s *Stream) resetInternal(code uint64, userClosed bool) { // extra RESET_STREAM in this case is harmless. s.outreset.set() s.outresetcode = code + s.outbuf = nil + s.outbufoff = 0 s.out.discardBefore(s.out.end) s.outunsent = rangeset[int64]{} s.outblocked.clear() @@ -488,8 +612,9 @@ func (s *Stream) inUnlock() { // inUnlockNoQueue is inUnlock, // but reports whether s has frames to write rather than notifying the Conn. func (s *Stream) inUnlockNoQueue() streamState { - canRead := s.inset.contains(s.in.start) || // data available to read - s.insize == s.in.start || // at EOF + nextByte := s.in.start + int64(len(s.inbuf)) + canRead := s.inset.contains(nextByte) || // data available to read + s.insize == s.in.start+int64(len(s.inbuf)) || // at EOF s.inresetcode != -1 || // reset by peer s.inclosed.isSet() // closed locally defer s.ingate.unlock(canRead) diff --git a/internal/quic/stream_limits.go b/quic/stream_limits.go similarity index 100% rename from internal/quic/stream_limits.go rename to quic/stream_limits.go diff --git a/internal/quic/stream_limits_test.go b/quic/stream_limits_test.go similarity index 97% rename from internal/quic/stream_limits_test.go rename to quic/stream_limits_test.go index 3f291e9f4c..9c2f71ec1e 100644 --- a/internal/quic/stream_limits_test.go +++ b/quic/stream_limits_test.go @@ -200,7 +200,6 @@ func TestStreamLimitMaxStreamsFrameTooLarge(t *testing.T) { func TestStreamLimitSendUpdatesMaxStreams(t *testing.T) { testStreamTypes(t, "", func(t *testing.T, styp streamType) { - ctx := canceledContext() tc := newTestConn(t, serverSide, func(c *Config) { if styp == uniStream { c.MaxUniRemoteStreams = 4 @@ -218,13 +217,9 @@ func TestStreamLimitSendUpdatesMaxStreams(t *testing.T) { id: newStreamID(clientSide, styp, int64(i)), fin: true, }) - s, err := tc.conn.AcceptStream(ctx) - if err != nil { - t.Fatalf("AcceptStream = %v", err) - } - streams = append(streams, s) + streams = append(streams, tc.acceptStream()) } - streams[3].CloseContext(ctx) + streams[3].Close() if styp == bidiStream { tc.wantFrame("stream is closed", packetType1RTT, debugFrameStream{ diff --git a/internal/quic/stream_test.go b/quic/stream_test.go similarity index 91% rename from internal/quic/stream_test.go rename to quic/stream_test.go index 00e392dbab..9f857f29d4 100644 --- a/internal/quic/stream_test.go +++ b/quic/stream_test.go @@ -19,7 +19,6 @@ import ( func TestStreamWriteBlockedByOutputBuffer(t *testing.T) { testStreamTypes(t, "", func(t *testing.T, styp streamType) { - ctx := canceledContext() want := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9} const writeBufferSize = 4 tc := newTestConn(t, clientSide, permissiveTransportParameters, func(c *Config) { @@ -28,15 +27,12 @@ func TestStreamWriteBlockedByOutputBuffer(t *testing.T) { tc.handshake() tc.ignoreFrame(frameTypeAck) - s, err := tc.conn.newLocalStream(ctx, styp) - if err != nil { - t.Fatal(err) - } + s := newLocalStream(t, tc, styp) // Non-blocking write. - n, err := s.WriteContext(ctx, want) + n, err := s.Write(want) if n != writeBufferSize || err != context.Canceled { - t.Fatalf("s.WriteContext() = %v, %v; want %v, context.Canceled", n, err, writeBufferSize) + t.Fatalf("s.Write() = %v, %v; want %v, context.Canceled", n, err, writeBufferSize) } s.Flush() tc.wantFrame("first write buffer of data sent", @@ -48,7 +44,8 @@ func TestStreamWriteBlockedByOutputBuffer(t *testing.T) { // Blocking write, which must wait for buffer space. w := runAsync(tc, func(ctx context.Context) (int, error) { - n, err := s.WriteContext(ctx, want[writeBufferSize:]) + s.SetWriteContext(ctx) + n, err := s.Write(want[writeBufferSize:]) s.Flush() return n, err }) @@ -75,7 +72,7 @@ func TestStreamWriteBlockedByOutputBuffer(t *testing.T) { }) if n, err := w.result(); n != len(want)-writeBufferSize || err != nil { - t.Fatalf("s.WriteContext() = %v, %v; want %v, nil", + t.Fatalf("s.Write() = %v, %v; want %v, nil", len(want)-writeBufferSize, err, writeBufferSize) } }) @@ -99,10 +96,11 @@ func TestStreamWriteBlockedByStreamFlowControl(t *testing.T) { } // Data is written to the stream output buffer, but we have no flow control. - _, err = s.WriteContext(ctx, want[:1]) + _, err = s.Write(want[:1]) if err != nil { t.Fatalf("write with available output buffer: unexpected error: %v", err) } + s.Flush() tc.wantFrame("write blocked by flow control triggers a STREAM_DATA_BLOCKED frame", packetType1RTT, debugFrameStreamDataBlocked{ id: s.id, @@ -110,10 +108,11 @@ func TestStreamWriteBlockedByStreamFlowControl(t *testing.T) { }) // Write more data. - _, err = s.WriteContext(ctx, want[1:]) + _, err = s.Write(want[1:]) if err != nil { t.Fatalf("write with available output buffer: unexpected error: %v", err) } + s.Flush() tc.wantIdle("adding more blocked data does not trigger another STREAM_DATA_BLOCKED") // Provide some flow control window. @@ -172,7 +171,7 @@ func TestStreamIgnoresMaxStreamDataReduction(t *testing.T) { if err != nil { t.Fatal(err) } - s.WriteContext(ctx, want[:1]) + s.Write(want[:1]) s.Flush() tc.wantFrame("sent data (1 byte) fits within flow control limit", packetType1RTT, debugFrameStream{ @@ -188,7 +187,7 @@ func TestStreamIgnoresMaxStreamDataReduction(t *testing.T) { }) // Write [1,4). - s.WriteContext(ctx, want[1:]) + s.Write(want[1:]) tc.wantFrame("stream limit is 4 bytes, ignoring decrease in MAX_STREAM_DATA", packetType1RTT, debugFrameStream{ id: s.id, @@ -208,7 +207,7 @@ func TestStreamIgnoresMaxStreamDataReduction(t *testing.T) { }) // Write [1,4). - s.WriteContext(ctx, want[4:]) + s.Write(want[4:]) tc.wantFrame("stream limit is 8 bytes, ignoring decrease in MAX_STREAM_DATA", packetType1RTT, debugFrameStream{ id: s.id, @@ -220,7 +219,6 @@ func TestStreamIgnoresMaxStreamDataReduction(t *testing.T) { func TestStreamWriteBlockedByWriteBufferLimit(t *testing.T) { testStreamTypes(t, "", func(t *testing.T, styp streamType) { - ctx := canceledContext() want := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9} const maxWriteBuffer = 4 tc := newTestConn(t, clientSide, func(p *transportParameters) { @@ -238,12 +236,10 @@ func TestStreamWriteBlockedByWriteBufferLimit(t *testing.T) { // Write more data than StreamWriteBufferSize. // The peer has given us plenty of flow control, // so we're just blocked by our local limit. - s, err := tc.conn.newLocalStream(ctx, styp) - if err != nil { - t.Fatal(err) - } + s := newLocalStream(t, tc, styp) w := runAsync(tc, func(ctx context.Context) (int, error) { - return s.WriteContext(ctx, want) + s.SetWriteContext(ctx) + return s.Write(want) }) tc.wantFrame("stream write should send as much data as write buffer allows", packetType1RTT, debugFrameStream{ @@ -266,7 +262,7 @@ func TestStreamWriteBlockedByWriteBufferLimit(t *testing.T) { w.cancel() n, err := w.result() if n != 2*maxWriteBuffer || err == nil { - t.Fatalf("WriteContext() = %v, %v; want %v bytes, error", n, err, 2*maxWriteBuffer) + t.Fatalf("Write() = %v, %v; want %v bytes, error", n, err, 2*maxWriteBuffer) } }) } @@ -397,7 +393,6 @@ func TestStreamReceive(t *testing.T) { }}, }} { testStreamTypes(t, test.name, func(t *testing.T, styp streamType) { - ctx := canceledContext() tc := newTestConn(t, serverSide) tc.handshake() sid := newStreamID(clientSide, styp, 0) @@ -413,21 +408,17 @@ func TestStreamReceive(t *testing.T) { fin: f.fin, }) if s == nil { - var err error - s, err = tc.conn.AcceptStream(ctx) - if err != nil { - tc.t.Fatalf("conn.AcceptStream() = %v", err) - } + s = tc.acceptStream() } for { - n, err := s.ReadContext(ctx, got[total:]) - t.Logf("s.ReadContext() = %v, %v", n, err) + n, err := s.Read(got[total:]) + t.Logf("s.Read() = %v, %v", n, err) total += n if f.wantEOF && err != io.EOF { - t.Fatalf("ReadContext() error = %v; want io.EOF", err) + t.Fatalf("Read() error = %v; want io.EOF", err) } if !f.wantEOF && err == io.EOF { - t.Fatalf("ReadContext() error = io.EOF, want something else") + t.Fatalf("Read() error = io.EOF, want something else") } if err != nil { break @@ -468,8 +459,8 @@ func TestStreamReceiveExtendsStreamWindow(t *testing.T) { } tc.wantIdle("stream window is not extended before data is read") buf := make([]byte, maxWindowSize+1) - if n, err := s.ReadContext(ctx, buf); n != maxWindowSize || err != nil { - t.Fatalf("s.ReadContext() = %v, %v; want %v, nil", n, err, maxWindowSize) + if n, err := s.Read(buf); n != maxWindowSize || err != nil { + t.Fatalf("s.Read() = %v, %v; want %v, nil", n, err, maxWindowSize) } tc.wantFrame("stream window is extended after reading data", packetType1RTT, debugFrameMaxStreamData{ @@ -482,8 +473,8 @@ func TestStreamReceiveExtendsStreamWindow(t *testing.T) { data: make([]byte, maxWindowSize), fin: true, }) - if n, err := s.ReadContext(ctx, buf); n != maxWindowSize || err != io.EOF { - t.Fatalf("s.ReadContext() = %v, %v; want %v, io.EOF", n, err, maxWindowSize) + if n, err := s.Read(buf); n != maxWindowSize || err != io.EOF { + t.Fatalf("s.Read() = %v, %v; want %v, io.EOF", n, err, maxWindowSize) } tc.wantIdle("stream window is not extended after FIN") }) @@ -549,6 +540,32 @@ func TestStreamReceiveDuplicateDataDoesNotViolateLimits(t *testing.T) { }) } +func TestStreamReceiveEmptyEOF(t *testing.T) { + // A stream receives some data, we read a byte of that data + // (causing the rest to be pulled into the s.inbuf buffer), + // and then we receive a FIN with no additional data. + testStreamTypes(t, "", func(t *testing.T, styp streamType) { + tc, s := newTestConnAndRemoteStream(t, serverSide, styp, permissiveTransportParameters) + want := []byte{1, 2, 3} + tc.writeFrames(packetType1RTT, debugFrameStream{ + id: s.id, + data: want, + }) + if got, err := s.ReadByte(); got != want[0] || err != nil { + t.Fatalf("s.ReadByte() = %v, %v; want %v, nil", got, err, want[0]) + } + + tc.writeFrames(packetType1RTT, debugFrameStream{ + id: s.id, + off: 3, + fin: true, + }) + if got, err := io.ReadAll(s); !bytes.Equal(got, want[1:]) || err != nil { + t.Fatalf("io.ReadAll(s) = {%x}, %v; want {%x}, nil", got, err, want[1:]) + } + }) +} + func finalSizeTest(t *testing.T, wantErr transportError, f func(tc *testConn, sid streamID) (finalSize int64), opts ...any) { testStreamTypes(t, "", func(t *testing.T, styp streamType) { for _, test := range []struct { @@ -673,18 +690,19 @@ func TestStreamReceiveUnblocksReader(t *testing.T) { t.Fatalf("AcceptStream() = %v", err) } - // ReadContext succeeds immediately, since we already have data. + // Read succeeds immediately, since we already have data. got := make([]byte, len(want)) read := runAsync(tc, func(ctx context.Context) (int, error) { - return s.ReadContext(ctx, got) + return s.Read(got) }) if n, err := read.result(); n != write1size || err != nil { - t.Fatalf("ReadContext = %v, %v; want %v, nil", n, err, write1size) + t.Fatalf("Read = %v, %v; want %v, nil", n, err, write1size) } - // ReadContext blocks waiting for more data. + // Read blocks waiting for more data. read = runAsync(tc, func(ctx context.Context) (int, error) { - return s.ReadContext(ctx, got[write1size:]) + s.SetReadContext(ctx) + return s.Read(got[write1size:]) }) tc.writeFrames(packetType1RTT, debugFrameStream{ id: sid, @@ -693,7 +711,7 @@ func TestStreamReceiveUnblocksReader(t *testing.T) { fin: true, }) if n, err := read.result(); n != len(want)-write1size || err != io.EOF { - t.Fatalf("ReadContext = %v, %v; want %v, io.EOF", n, err, len(want)-write1size) + t.Fatalf("Read = %v, %v; want %v, io.EOF", n, err, len(want)-write1size) } if !bytes.Equal(got, want) { t.Fatalf("read bytes %x, want %x", got, want) @@ -935,7 +953,8 @@ func TestStreamResetBlockedStream(t *testing.T) { }) tc.ignoreFrame(frameTypeStreamDataBlocked) writing := runAsync(tc, func(ctx context.Context) (int, error) { - return s.WriteContext(ctx, []byte{0, 1, 2, 3, 4, 5, 6, 7}) + s.SetWriteContext(ctx) + return s.Write([]byte{0, 1, 2, 3, 4, 5, 6, 7}) }) tc.wantFrame("stream writes data until write buffer fills", packetType1RTT, debugFrameStream{ @@ -972,7 +991,7 @@ func TestStreamWriteMoreThanOnePacketOfData(t *testing.T) { want := make([]byte, 4096) rand.Read(want) // doesn't need to be crypto/rand, but non-deprecated and harmless w := runAsync(tc, func(ctx context.Context) (int, error) { - n, err := s.WriteContext(ctx, want) + n, err := s.Write(want) s.Flush() return n, err }) @@ -992,7 +1011,7 @@ func TestStreamWriteMoreThanOnePacketOfData(t *testing.T) { got = append(got, sf.data...) } if n, err := w.result(); n != len(want) || err != nil { - t.Fatalf("s.WriteContext() = %v, %v; want %v, nil", n, err, len(want)) + t.Fatalf("s.Write() = %v, %v; want %v, nil", n, err, len(want)) } if !bytes.Equal(got, want) { t.Fatalf("mismatch in received stream data") @@ -1000,17 +1019,16 @@ func TestStreamWriteMoreThanOnePacketOfData(t *testing.T) { } func TestStreamCloseWaitsForAcks(t *testing.T) { - ctx := canceledContext() tc, s := newTestConnAndLocalStream(t, serverSide, uniStream, permissiveTransportParameters) data := make([]byte, 100) - s.WriteContext(ctx, data) + s.Write(data) s.Flush() tc.wantFrame("conn sends data for the stream", packetType1RTT, debugFrameStream{ id: s.id, data: data, }) - if err := s.CloseContext(ctx); err != context.Canceled { + if err := s.Close(); err != context.Canceled { t.Fatalf("s.Close() = %v, want context.Canceled (data not acked yet)", err) } tc.wantFrame("conn sends FIN for closed stream", @@ -1021,21 +1039,22 @@ func TestStreamCloseWaitsForAcks(t *testing.T) { data: []byte{}, }) closing := runAsync(tc, func(ctx context.Context) (struct{}, error) { - return struct{}{}, s.CloseContext(ctx) + s.SetWriteContext(ctx) + return struct{}{}, s.Close() }) if _, err := closing.result(); err != errNotDone { - t.Fatalf("s.CloseContext() = %v, want it to block waiting for acks", err) + t.Fatalf("s.Close() = %v, want it to block waiting for acks", err) } tc.writeAckForAll() if _, err := closing.result(); err != nil { - t.Fatalf("s.CloseContext() = %v, want nil (all data acked)", err) + t.Fatalf("s.Close() = %v, want nil (all data acked)", err) } } func TestStreamCloseReadOnly(t *testing.T) { tc, s := newTestConnAndRemoteStream(t, serverSide, uniStream, permissiveTransportParameters) - if err := s.CloseContext(canceledContext()); err != nil { - t.Errorf("s.CloseContext() = %v, want nil", err) + if err := s.Close(); err != nil { + t.Errorf("s.Close() = %v, want nil", err) } tc.wantFrame("closed stream sends STOP_SENDING", packetType1RTT, debugFrameStopSending{ @@ -1069,17 +1088,16 @@ func TestStreamCloseUnblocked(t *testing.T) { }, }} { t.Run(test.name, func(t *testing.T) { - ctx := canceledContext() tc, s := newTestConnAndLocalStream(t, serverSide, uniStream, permissiveTransportParameters) data := make([]byte, 100) - s.WriteContext(ctx, data) + s.Write(data) s.Flush() tc.wantFrame("conn sends data for the stream", packetType1RTT, debugFrameStream{ id: s.id, data: data, }) - if err := s.CloseContext(ctx); err != context.Canceled { + if err := s.Close(); err != context.Canceled { t.Fatalf("s.Close() = %v, want context.Canceled (data not acked yet)", err) } tc.wantFrame("conn sends FIN for closed stream", @@ -1090,34 +1108,34 @@ func TestStreamCloseUnblocked(t *testing.T) { data: []byte{}, }) closing := runAsync(tc, func(ctx context.Context) (struct{}, error) { - return struct{}{}, s.CloseContext(ctx) + s.SetWriteContext(ctx) + return struct{}{}, s.Close() }) if _, err := closing.result(); err != errNotDone { - t.Fatalf("s.CloseContext() = %v, want it to block waiting for acks", err) + t.Fatalf("s.Close() = %v, want it to block waiting for acks", err) } test.unblock(tc, s) _, err := closing.result() switch { case err == errNotDone: - t.Fatalf("s.CloseContext() still blocking; want it to have returned") + t.Fatalf("s.Close() still blocking; want it to have returned") case err == nil && !test.success: - t.Fatalf("s.CloseContext() = nil, want error") + t.Fatalf("s.Close() = nil, want error") case err != nil && test.success: - t.Fatalf("s.CloseContext() = %v, want nil (all data acked)", err) + t.Fatalf("s.Close() = %v, want nil (all data acked)", err) } }) } } func TestStreamCloseWriteWhenBlockedByStreamFlowControl(t *testing.T) { - ctx := canceledContext() tc, s := newTestConnAndLocalStream(t, serverSide, uniStream, permissiveTransportParameters, func(p *transportParameters) { //p.initialMaxData = 0 p.initialMaxStreamDataUni = 0 }) tc.ignoreFrame(frameTypeStreamDataBlocked) - if _, err := s.WriteContext(ctx, []byte{0, 1}); err != nil { + if _, err := s.Write([]byte{0, 1}); err != nil { t.Fatalf("s.Write = %v", err) } s.CloseWrite() @@ -1149,7 +1167,6 @@ func TestStreamCloseWriteWhenBlockedByStreamFlowControl(t *testing.T) { func TestStreamPeerResetsWithUnreadAndUnsentData(t *testing.T) { testStreamTypes(t, "", func(t *testing.T, styp streamType) { - ctx := canceledContext() tc, s := newTestConnAndRemoteStream(t, serverSide, styp) data := []byte{0, 1, 2, 3, 4, 5, 6, 7} tc.writeFrames(packetType1RTT, debugFrameStream{ @@ -1157,7 +1174,7 @@ func TestStreamPeerResetsWithUnreadAndUnsentData(t *testing.T) { data: data, }) got := make([]byte, 4) - if n, err := s.ReadContext(ctx, got); n != len(got) || err != nil { + if n, err := s.Read(got); n != len(got) || err != nil { t.Fatalf("Read start of stream: got %v, %v; want %v, nil", n, err, len(got)) } const sentCode = 42 @@ -1167,8 +1184,8 @@ func TestStreamPeerResetsWithUnreadAndUnsentData(t *testing.T) { code: sentCode, }) wantErr := StreamErrorCode(sentCode) - if n, err := s.ReadContext(ctx, got); n != 0 || !errors.Is(err, wantErr) { - t.Fatalf("Read reset stream: got %v, %v; want 0, %v", n, err, wantErr) + if _, err := io.ReadAll(s); !errors.Is(err, wantErr) { + t.Fatalf("Read reset stream: ReadAll got error %v; want %v", err, wantErr) } }) } @@ -1177,8 +1194,9 @@ func TestStreamPeerResetWakesBlockedRead(t *testing.T) { testStreamTypes(t, "", func(t *testing.T, styp streamType) { tc, s := newTestConnAndRemoteStream(t, serverSide, styp) reader := runAsync(tc, func(ctx context.Context) (int, error) { + s.SetReadContext(ctx) got := make([]byte, 4) - return s.ReadContext(ctx, got) + return s.Read(got) }) const sentCode = 42 tc.writeFrames(packetType1RTT, debugFrameResetStream{ @@ -1333,7 +1351,6 @@ func TestStreamFlushImplicitExact(t *testing.T) { id: s.id, data: want[0:4], }) - }) } @@ -1348,7 +1365,8 @@ func TestStreamFlushImplicitLargerThanBuffer(t *testing.T) { want := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9} w := runAsync(tc, func(ctx context.Context) (int, error) { - n, err := s.WriteContext(ctx, want) + s.SetWriteContext(ctx) + n, err := s.Write(want) return n, err }) @@ -1401,7 +1419,10 @@ func newTestConnAndLocalStream(t *testing.T, side connSide, styp streamType, opt tc := newTestConn(t, side, opts...) tc.handshake() tc.ignoreFrame(frameTypeAck) - return tc, newLocalStream(t, tc, styp) + s := newLocalStream(t, tc, styp) + s.SetReadContext(canceledContext()) + s.SetWriteContext(canceledContext()) + return tc, s } func newLocalStream(t *testing.T, tc *testConn, styp streamType) *Stream { @@ -1411,6 +1432,8 @@ func newLocalStream(t *testing.T, tc *testConn, styp streamType) *Stream { if err != nil { t.Fatalf("conn.newLocalStream(%v) = %v", styp, err) } + s.SetReadContext(canceledContext()) + s.SetWriteContext(canceledContext()) return s } @@ -1419,7 +1442,10 @@ func newTestConnAndRemoteStream(t *testing.T, side connSide, styp streamType, op tc := newTestConn(t, side, opts...) tc.handshake() tc.ignoreFrame(frameTypeAck) - return tc, newRemoteStream(t, tc, styp) + s := newRemoteStream(t, tc, styp) + s.SetReadContext(canceledContext()) + s.SetWriteContext(canceledContext()) + return tc, s } func newRemoteStream(t *testing.T, tc *testConn, styp streamType) *Stream { @@ -1432,6 +1458,8 @@ func newRemoteStream(t *testing.T, tc *testConn, styp streamType) *Stream { if err != nil { t.Fatalf("conn.AcceptStream() = %v", err) } + s.SetReadContext(canceledContext()) + s.SetWriteContext(canceledContext()) return s } diff --git a/internal/quic/tls.go b/quic/tls.go similarity index 88% rename from internal/quic/tls.go rename to quic/tls.go index a37e26fb8e..e2f2e5bde1 100644 --- a/internal/quic/tls.go +++ b/quic/tls.go @@ -11,14 +11,24 @@ import ( "crypto/tls" "errors" "fmt" + "net" "time" ) // startTLS starts the TLS handshake. -func (c *Conn) startTLS(now time.Time, initialConnID []byte, params transportParameters) error { +func (c *Conn) startTLS(now time.Time, initialConnID []byte, peerHostname string, params transportParameters) error { + tlsConfig := c.config.TLSConfig + if a, _, err := net.SplitHostPort(peerHostname); err == nil { + peerHostname = a + } + if tlsConfig.ServerName == "" && peerHostname != "" { + tlsConfig = tlsConfig.Clone() + tlsConfig.ServerName = peerHostname + } + c.keysInitial = initialKeys(initialConnID, c.side) - qconfig := &tls.QUICConfig{TLSConfig: c.config.TLSConfig} + qconfig := &tls.QUICConfig{TLSConfig: tlsConfig} if c.side == clientSide { c.tls = tls.QUICClient(qconfig) } else { diff --git a/internal/quic/tls_test.go b/quic/tls_test.go similarity index 100% rename from internal/quic/tls_test.go rename to quic/tls_test.go diff --git a/internal/quic/tlsconfig_test.go b/quic/tlsconfig_test.go similarity index 100% rename from internal/quic/tlsconfig_test.go rename to quic/tlsconfig_test.go diff --git a/internal/quic/transport_params.go b/quic/transport_params.go similarity index 100% rename from internal/quic/transport_params.go rename to quic/transport_params.go diff --git a/internal/quic/transport_params_test.go b/quic/transport_params_test.go similarity index 100% rename from internal/quic/transport_params_test.go rename to quic/transport_params_test.go diff --git a/quic/udp.go b/quic/udp.go new file mode 100644 index 0000000000..0a578286b2 --- /dev/null +++ b/quic/udp.go @@ -0,0 +1,30 @@ +// Copyright 2023 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.21 + +package quic + +import "net/netip" + +// Per-plaform consts describing support for various features. +// +// const udpECNSupport indicates whether the platform supports setting +// the ECN (Explicit Congestion Notification) IP header bits. +// +// const udpInvalidLocalAddrIsError indicates whether sending a packet +// from an local address not associated with the system is an error. +// For example, assuming 127.0.0.2 is not a local address, does sending +// from it (using IP_PKTINFO or some other such feature) result in an error? + +// unmapAddrPort returns a with any IPv4-mapped IPv6 address prefix removed. +func unmapAddrPort(a netip.AddrPort) netip.AddrPort { + if a.Addr().Is4In6() { + return netip.AddrPortFrom( + a.Addr().Unmap(), + a.Port(), + ) + } + return a +} diff --git a/quic/udp_darwin.go b/quic/udp_darwin.go new file mode 100644 index 0000000000..2eb2e9f9f0 --- /dev/null +++ b/quic/udp_darwin.go @@ -0,0 +1,38 @@ +// Copyright 2023 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.21 && darwin + +package quic + +import ( + "encoding/binary" + + "golang.org/x/sys/unix" +) + +// See udp.go. +const ( + udpECNSupport = true + udpInvalidLocalAddrIsError = true +) + +// Confusingly, on Darwin the contents of the IP_TOS option differ depending on whether +// it is used as an inbound or outbound cmsg. + +func parseIPTOS(b []byte) (ecnBits, bool) { + // Single byte. The low two bits are the ECN field. + if len(b) != 1 { + return 0, false + } + return ecnBits(b[0] & ecnMask), true +} + +func appendCmsgECNv4(b []byte, ecn ecnBits) []byte { + // 32-bit integer. + // https://github.com/apple/darwin-xnu/blob/2ff845c2e033bd0ff64b5b6aa6063a1f8f65aa32/bsd/netinet/in_tclass.c#L1062-L1073 + b, data := appendCmsg(b, unix.IPPROTO_IP, unix.IP_TOS, 4) + binary.NativeEndian.PutUint32(data, uint32(ecn)) + return b +} diff --git a/quic/udp_linux.go b/quic/udp_linux.go new file mode 100644 index 0000000000..6f191ed398 --- /dev/null +++ b/quic/udp_linux.go @@ -0,0 +1,33 @@ +// Copyright 2023 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.21 && linux + +package quic + +import ( + "golang.org/x/sys/unix" +) + +// See udp.go. +const ( + udpECNSupport = true + udpInvalidLocalAddrIsError = false +) + +// The IP_TOS socket option is a single byte containing the IP TOS field. +// The low two bits are the ECN field. + +func parseIPTOS(b []byte) (ecnBits, bool) { + if len(b) != 1 { + return 0, false + } + return ecnBits(b[0] & ecnMask), true +} + +func appendCmsgECNv4(b []byte, ecn ecnBits) []byte { + b, data := appendCmsg(b, unix.IPPROTO_IP, unix.IP_TOS, 1) + data[0] = byte(ecn) + return b +} diff --git a/quic/udp_msg.go b/quic/udp_msg.go new file mode 100644 index 0000000000..0b600a2b46 --- /dev/null +++ b/quic/udp_msg.go @@ -0,0 +1,247 @@ +// Copyright 2023 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.21 && !quicbasicnet && (darwin || linux) + +package quic + +import ( + "encoding/binary" + "net" + "net/netip" + "sync" + "unsafe" + + "golang.org/x/sys/unix" +) + +// Network interface for platforms using sendmsg/recvmsg with cmsgs. + +type netUDPConn struct { + c *net.UDPConn + localAddr netip.AddrPort +} + +func newNetUDPConn(uc *net.UDPConn) (*netUDPConn, error) { + a, _ := uc.LocalAddr().(*net.UDPAddr) + localAddr := a.AddrPort() + if localAddr.Addr().IsUnspecified() { + // If the conn is not bound to a specified (non-wildcard) address, + // then set localAddr.Addr to an invalid netip.Addr. + // This better conveys that this is not an address we should be using, + // and is a bit more efficient to test against. + localAddr = netip.AddrPortFrom(netip.Addr{}, localAddr.Port()) + } + + sc, err := uc.SyscallConn() + if err != nil { + return nil, err + } + sc.Control(func(fd uintptr) { + // Ask for ECN info and (when we aren't bound to a fixed local address) + // destination info. + // + // If any of these calls fail, we won't get the requested information. + // That's fine, we'll gracefully handle the lack. + unix.SetsockoptInt(int(fd), unix.IPPROTO_IP, unix.IP_RECVTOS, 1) + unix.SetsockoptInt(int(fd), unix.IPPROTO_IPV6, unix.IPV6_RECVTCLASS, 1) + if !localAddr.IsValid() { + unix.SetsockoptInt(int(fd), unix.IPPROTO_IP, unix.IP_PKTINFO, 1) + unix.SetsockoptInt(int(fd), unix.IPPROTO_IPV6, unix.IPV6_RECVPKTINFO, 1) + } + }) + + return &netUDPConn{ + c: uc, + localAddr: localAddr, + }, nil +} + +func (c *netUDPConn) Close() error { return c.c.Close() } + +func (c *netUDPConn) LocalAddr() netip.AddrPort { + a, _ := c.c.LocalAddr().(*net.UDPAddr) + return a.AddrPort() +} + +func (c *netUDPConn) Read(f func(*datagram)) { + // We shouldn't ever see all of these messages at the same time, + // but the total is small so just allocate enough space for everything we use. + const ( + inPktinfoSize = 12 // int + in_addr + in_addr + in6PktinfoSize = 20 // in6_addr + int + ipTOSSize = 4 + ipv6TclassSize = 4 + ) + control := make([]byte, 0+ + unix.CmsgSpace(inPktinfoSize)+ + unix.CmsgSpace(in6PktinfoSize)+ + unix.CmsgSpace(ipTOSSize)+ + unix.CmsgSpace(ipv6TclassSize)) + + for { + d := newDatagram() + n, controlLen, _, peerAddr, err := c.c.ReadMsgUDPAddrPort(d.b, control) + if err != nil { + return + } + if n == 0 { + continue + } + d.localAddr = c.localAddr + d.peerAddr = unmapAddrPort(peerAddr) + d.b = d.b[:n] + parseControl(d, control[:controlLen]) + f(d) + } +} + +var cmsgPool = sync.Pool{ + New: func() any { + return new([]byte) + }, +} + +func (c *netUDPConn) Write(dgram datagram) error { + controlp := cmsgPool.Get().(*[]byte) + control := *controlp + defer func() { + *controlp = control[:0] + cmsgPool.Put(controlp) + }() + + localIP := dgram.localAddr.Addr() + if localIP.IsValid() { + if localIP.Is4() { + control = appendCmsgIPSourceAddrV4(control, localIP) + } else { + control = appendCmsgIPSourceAddrV6(control, localIP) + } + } + if dgram.ecn != ecnNotECT { + if dgram.peerAddr.Addr().Is4() { + control = appendCmsgECNv4(control, dgram.ecn) + } else { + control = appendCmsgECNv6(control, dgram.ecn) + } + } + + _, _, err := c.c.WriteMsgUDPAddrPort(dgram.b, control, dgram.peerAddr) + return err +} + +func parseControl(d *datagram, control []byte) { + for len(control) > 0 { + hdr, data, remainder, err := unix.ParseOneSocketControlMessage(control) + if err != nil { + return + } + control = remainder + switch hdr.Level { + case unix.IPPROTO_IP: + switch hdr.Type { + case unix.IP_TOS, unix.IP_RECVTOS: + // (Linux sets the type to IP_TOS, Darwin to IP_RECVTOS, + // just check for both.) + if ecn, ok := parseIPTOS(data); ok { + d.ecn = ecn + } + case unix.IP_PKTINFO: + if a, ok := parseInPktinfo(data); ok { + d.localAddr = netip.AddrPortFrom(a, d.localAddr.Port()) + } + } + case unix.IPPROTO_IPV6: + switch hdr.Type { + case unix.IPV6_TCLASS: + // 32-bit integer containing the traffic class field. + // The low two bits are the ECN field. + if ecn, ok := parseIPv6TCLASS(data); ok { + d.ecn = ecn + } + case unix.IPV6_PKTINFO: + if a, ok := parseIn6Pktinfo(data); ok { + d.localAddr = netip.AddrPortFrom(a, d.localAddr.Port()) + } + } + } + } +} + +// IPV6_TCLASS is specified by RFC 3542 as an int. + +func parseIPv6TCLASS(b []byte) (ecnBits, bool) { + if len(b) != 4 { + return 0, false + } + return ecnBits(binary.NativeEndian.Uint32(b) & ecnMask), true +} + +func appendCmsgECNv6(b []byte, ecn ecnBits) []byte { + b, data := appendCmsg(b, unix.IPPROTO_IPV6, unix.IPV6_TCLASS, 4) + binary.NativeEndian.PutUint32(data, uint32(ecn)) + return b +} + +// struct in_pktinfo { +// unsigned int ipi_ifindex; /* send/recv interface index */ +// struct in_addr ipi_spec_dst; /* Local address */ +// struct in_addr ipi_addr; /* IP Header dst address */ +// }; + +// parseInPktinfo returns the destination address from an IP_PKTINFO. +func parseInPktinfo(b []byte) (dst netip.Addr, ok bool) { + if len(b) != 12 { + return netip.Addr{}, false + } + return netip.AddrFrom4([4]byte(b[8:][:4])), true +} + +// appendCmsgIPSourceAddrV4 appends an IP_PKTINFO setting the source address +// for an outbound datagram. +func appendCmsgIPSourceAddrV4(b []byte, src netip.Addr) []byte { + // struct in_pktinfo { + // unsigned int ipi_ifindex; /* send/recv interface index */ + // struct in_addr ipi_spec_dst; /* Local address */ + // struct in_addr ipi_addr; /* IP Header dst address */ + // }; + b, data := appendCmsg(b, unix.IPPROTO_IP, unix.IP_PKTINFO, 12) + ip := src.As4() + copy(data[4:], ip[:]) + return b +} + +// struct in6_pktinfo { +// struct in6_addr ipi6_addr; /* src/dst IPv6 address */ +// unsigned int ipi6_ifindex; /* send/recv interface index */ +// }; + +// parseIn6Pktinfo returns the destination address from an IPV6_PKTINFO. +func parseIn6Pktinfo(b []byte) (netip.Addr, bool) { + if len(b) != 20 { + return netip.Addr{}, false + } + return netip.AddrFrom16([16]byte(b[:16])).Unmap(), true +} + +// appendCmsgIPSourceAddrV6 appends an IPV6_PKTINFO setting the source address +// for an outbound datagram. +func appendCmsgIPSourceAddrV6(b []byte, src netip.Addr) []byte { + b, data := appendCmsg(b, unix.IPPROTO_IPV6, unix.IPV6_PKTINFO, 20) + ip := src.As16() + copy(data[0:], ip[:]) + return b +} + +// appendCmsg appends a cmsg with the given level, type, and size to b. +// It returns the new buffer, and the data section of the cmsg. +func appendCmsg(b []byte, level, typ int32, size int) (_, data []byte) { + off := len(b) + b = append(b, make([]byte, unix.CmsgSpace(size))...) + h := (*unix.Cmsghdr)(unsafe.Pointer(&b[off])) + h.Level = level + h.Type = typ + h.SetLen(unix.CmsgLen(size)) + return b, b[off+unix.CmsgSpace(0):][:size] +} diff --git a/quic/udp_other.go b/quic/udp_other.go new file mode 100644 index 0000000000..28be6d2006 --- /dev/null +++ b/quic/udp_other.go @@ -0,0 +1,62 @@ +// Copyright 2023 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.21 && (quicbasicnet || !(darwin || linux)) + +package quic + +import ( + "net" + "net/netip" +) + +// Lowest common denominator network interface: Basic net.UDPConn, no cmsgs. +// We will not be able to send or receive ECN bits, +// and we will not know what our local address is. +// +// The quicbasicnet build tag allows selecting this interface on any platform. + +// See udp.go. +const ( + udpECNSupport = false + udpInvalidLocalAddrIsError = false +) + +type netUDPConn struct { + c *net.UDPConn +} + +func newNetUDPConn(uc *net.UDPConn) (*netUDPConn, error) { + return &netUDPConn{ + c: uc, + }, nil +} + +func (c *netUDPConn) Close() error { return c.c.Close() } + +func (c *netUDPConn) LocalAddr() netip.AddrPort { + a, _ := c.c.LocalAddr().(*net.UDPAddr) + return a.AddrPort() +} + +func (c *netUDPConn) Read(f func(*datagram)) { + for { + dgram := newDatagram() + n, _, _, peerAddr, err := c.c.ReadMsgUDPAddrPort(dgram.b, nil) + if err != nil { + return + } + if n == 0 { + continue + } + dgram.peerAddr = unmapAddrPort(peerAddr) + dgram.b = dgram.b[:n] + f(dgram) + } +} + +func (c *netUDPConn) Write(dgram datagram) error { + _, err := c.c.WriteToUDPAddrPort(dgram.b, dgram.peerAddr) + return err +} diff --git a/quic/udp_test.go b/quic/udp_test.go new file mode 100644 index 0000000000..d3732c140e --- /dev/null +++ b/quic/udp_test.go @@ -0,0 +1,189 @@ +// Copyright 2023 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.21 + +package quic + +import ( + "bytes" + "fmt" + "net" + "net/netip" + "runtime" + "testing" +) + +func TestUDPSourceUnspecified(t *testing.T) { + // Send datagram with no source address set. + runUDPTest(t, func(t *testing.T, test udpTest) { + t.Logf("%v", test.dstAddr) + data := []byte("source unspecified") + if err := test.src.Write(datagram{ + b: data, + peerAddr: test.dstAddr, + }); err != nil { + t.Fatalf("Write: %v", err) + } + got := <-test.dgramc + if !bytes.Equal(got.b, data) { + t.Errorf("got datagram {%x}, want {%x}", got.b, data) + } + }) +} + +func TestUDPSourceSpecified(t *testing.T) { + // Send datagram with source address set. + runUDPTest(t, func(t *testing.T, test udpTest) { + data := []byte("source specified") + if err := test.src.Write(datagram{ + b: data, + peerAddr: test.dstAddr, + localAddr: test.src.LocalAddr(), + }); err != nil { + t.Fatalf("Write: %v", err) + } + got := <-test.dgramc + if !bytes.Equal(got.b, data) { + t.Errorf("got datagram {%x}, want {%x}", got.b, data) + } + }) +} + +func TestUDPSourceInvalid(t *testing.T) { + // Send datagram with source address set to an address not associated with the connection. + if !udpInvalidLocalAddrIsError { + t.Skipf("%v: sending from invalid source succeeds", runtime.GOOS) + } + runUDPTest(t, func(t *testing.T, test udpTest) { + var localAddr netip.AddrPort + if test.src.LocalAddr().Addr().Is4() { + localAddr = netip.MustParseAddrPort("127.0.0.2:1234") + } else { + localAddr = netip.MustParseAddrPort("[::2]:1234") + } + data := []byte("source invalid") + if err := test.src.Write(datagram{ + b: data, + peerAddr: test.dstAddr, + localAddr: localAddr, + }); err == nil { + t.Errorf("Write with invalid localAddr succeeded; want error") + } + }) +} + +func TestUDPECN(t *testing.T) { + if !udpECNSupport { + t.Skipf("%v: no ECN support", runtime.GOOS) + } + // Send datagrams with ECN bits set, verify the ECN bits are received. + runUDPTest(t, func(t *testing.T, test udpTest) { + for _, ecn := range []ecnBits{ecnNotECT, ecnECT1, ecnECT0, ecnCE} { + if err := test.src.Write(datagram{ + b: []byte{1, 2, 3, 4}, + peerAddr: test.dstAddr, + ecn: ecn, + }); err != nil { + t.Fatalf("Write: %v", err) + } + got := <-test.dgramc + if got.ecn != ecn { + t.Errorf("sending ECN bits %x, got %x", ecn, got.ecn) + } + } + }) +} + +type udpTest struct { + src *netUDPConn + dst *netUDPConn + dstAddr netip.AddrPort + dgramc chan *datagram +} + +// runUDPTest calls f with a pair of UDPConns in a matrix of network variations: +// udp, udp4, and udp6, and variations on binding to an unspecified address (0.0.0.0) +// or a specified one. +func runUDPTest(t *testing.T, f func(t *testing.T, u udpTest)) { + for _, test := range []struct { + srcNet, srcAddr, dstNet, dstAddr string + }{ + {"udp4", "127.0.0.1", "udp", ""}, + {"udp4", "127.0.0.1", "udp4", ""}, + {"udp4", "127.0.0.1", "udp4", "127.0.0.1"}, + {"udp6", "::1", "udp", ""}, + {"udp6", "::1", "udp6", ""}, + {"udp6", "::1", "udp6", "::1"}, + } { + spec := "spec" + if test.dstAddr == "" { + spec = "unspec" + } + t.Run(fmt.Sprintf("%v/%v/%v", test.srcNet, test.dstNet, spec), func(t *testing.T) { + // See: https://go.googlesource.com/go/+/refs/tags/go1.22.0/src/net/ipsock.go#47 + // On these platforms, conns with network="udp" cannot accept IPv6. + switch runtime.GOOS { + case "dragonfly", "openbsd": + if test.srcNet == "udp6" && test.dstNet == "udp" { + t.Skipf("%v: no support for mapping IPv4 address to IPv6", runtime.GOOS) + } + } + if runtime.GOARCH == "wasm" && test.srcNet == "udp6" { + t.Skipf("%v: IPv6 tests fail when using wasm fake net", runtime.GOARCH) + } + + srcAddr := netip.AddrPortFrom(netip.MustParseAddr(test.srcAddr), 0) + srcConn, err := net.ListenUDP(test.srcNet, net.UDPAddrFromAddrPort(srcAddr)) + if err != nil { + // If ListenUDP fails here, we presumably don't have + // IPv4/IPv6 configured. + t.Skipf("ListenUDP(%q, %v) = %v", test.srcNet, srcAddr, err) + } + t.Cleanup(func() { srcConn.Close() }) + src, err := newNetUDPConn(srcConn) + if err != nil { + t.Fatalf("newNetUDPConn: %v", err) + } + + var dstAddr netip.AddrPort + if test.dstAddr != "" { + dstAddr = netip.AddrPortFrom(netip.MustParseAddr(test.dstAddr), 0) + } + dstConn, err := net.ListenUDP(test.dstNet, net.UDPAddrFromAddrPort(dstAddr)) + if err != nil { + t.Skipf("ListenUDP(%q, nil) = %v", test.dstNet, err) + } + dst, err := newNetUDPConn(dstConn) + if err != nil { + dstConn.Close() + t.Fatalf("newNetUDPConn: %v", err) + } + + dgramc := make(chan *datagram) + go func() { + defer close(dgramc) + dst.Read(func(dgram *datagram) { + dgramc <- dgram + }) + }() + t.Cleanup(func() { + dstConn.Close() + for range dgramc { + t.Errorf("test read unexpected datagram") + } + }) + + f(t, udpTest{ + src: src, + dst: dst, + dstAddr: netip.AddrPortFrom( + srcAddr.Addr(), + dst.LocalAddr().Port(), + ), + dgramc: dgramc, + }) + }) + } +} diff --git a/internal/quic/version_test.go b/quic/version_test.go similarity index 100% rename from internal/quic/version_test.go rename to quic/version_test.go diff --git a/internal/quic/wire.go b/quic/wire.go similarity index 100% rename from internal/quic/wire.go rename to quic/wire.go diff --git a/internal/quic/wire_test.go b/quic/wire_test.go similarity index 100% rename from internal/quic/wire_test.go rename to quic/wire_test.go diff --git a/websocket/client.go b/websocket/client.go index 69a4ac7eef..1e64157f3e 100644 --- a/websocket/client.go +++ b/websocket/client.go @@ -6,10 +6,12 @@ package websocket import ( "bufio" + "context" "io" "net" "net/http" "net/url" + "time" ) // DialError is an error that occurs while dialling a websocket server. @@ -79,28 +81,59 @@ func parseAuthority(location *url.URL) string { // DialConfig opens a new client connection to a WebSocket with a config. func DialConfig(config *Config) (ws *Conn, err error) { - var client net.Conn + return config.DialContext(context.Background()) +} + +// DialContext opens a new client connection to a WebSocket, with context support for timeouts/cancellation. +func (config *Config) DialContext(ctx context.Context) (*Conn, error) { if config.Location == nil { return nil, &DialError{config, ErrBadWebSocketLocation} } if config.Origin == nil { return nil, &DialError{config, ErrBadWebSocketOrigin} } + dialer := config.Dialer if dialer == nil { dialer = &net.Dialer{} } - client, err = dialWithDialer(dialer, config) - if err != nil { - goto Error - } - ws, err = NewClient(config, client) + + client, err := dialWithDialer(ctx, dialer, config) if err != nil { - client.Close() - goto Error + return nil, &DialError{config, err} } - return -Error: - return nil, &DialError{config, err} + // Cleanup the connection if we fail to create the websocket successfully + success := false + defer func() { + if !success { + _ = client.Close() + } + }() + + var ws *Conn + var wsErr error + doneConnecting := make(chan struct{}) + go func() { + defer close(doneConnecting) + ws, err = NewClient(config, client) + if err != nil { + wsErr = &DialError{config, err} + } + }() + + // The websocket.NewClient() function can block indefinitely, make sure that we + // respect the deadlines specified by the context. + select { + case <-ctx.Done(): + // Force the pending operations to fail, terminating the pending connection attempt + _ = client.SetDeadline(time.Now()) + <-doneConnecting // Wait for the goroutine that tries to establish the connection to finish + return nil, &DialError{config, ctx.Err()} + case <-doneConnecting: + if wsErr == nil { + success = true // Disarm the deferred connection cleanup + } + return ws, wsErr + } } diff --git a/websocket/dial.go b/websocket/dial.go index 2dab943a48..8a2d83c473 100644 --- a/websocket/dial.go +++ b/websocket/dial.go @@ -5,18 +5,23 @@ package websocket import ( + "context" "crypto/tls" "net" ) -func dialWithDialer(dialer *net.Dialer, config *Config) (conn net.Conn, err error) { +func dialWithDialer(ctx context.Context, dialer *net.Dialer, config *Config) (conn net.Conn, err error) { switch config.Location.Scheme { case "ws": - conn, err = dialer.Dial("tcp", parseAuthority(config.Location)) + conn, err = dialer.DialContext(ctx, "tcp", parseAuthority(config.Location)) case "wss": - conn, err = tls.DialWithDialer(dialer, "tcp", parseAuthority(config.Location), config.TlsConfig) + tlsDialer := &tls.Dialer{ + NetDialer: dialer, + Config: config.TlsConfig, + } + conn, err = tlsDialer.DialContext(ctx, "tcp", parseAuthority(config.Location)) default: err = ErrBadScheme } diff --git a/websocket/dial_test.go b/websocket/dial_test.go index aa03e30dd1..dd844872c9 100644 --- a/websocket/dial_test.go +++ b/websocket/dial_test.go @@ -5,10 +5,13 @@ package websocket import ( + "context" "crypto/tls" + "errors" "fmt" "log" "net" + "net/http" "net/http/httptest" "testing" "time" @@ -41,3 +44,37 @@ func TestDialConfigTLSWithDialer(t *testing.T) { t.Fatalf("expected timeout error, got %#v", neterr) } } + +func TestDialConfigTLSWithTimeouts(t *testing.T) { + t.Parallel() + + finishedRequest := make(chan bool) + + // Context for cancellation + ctx, cancel := context.WithCancel(context.Background()) + + // This is a TLS server that blocks each request indefinitely (and cancels the context) + tlsServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + cancel() + <-finishedRequest + })) + + tlsServerAddr := tlsServer.Listener.Addr().String() + log.Print("Test TLS WebSocket server listening on ", tlsServerAddr) + defer tlsServer.Close() + defer close(finishedRequest) + + config, _ := NewConfig(fmt.Sprintf("wss://%s/echo", tlsServerAddr), "http://localhost") + config.TlsConfig = &tls.Config{ + InsecureSkipVerify: true, + } + + _, err := config.DialContext(ctx) + dialerr, ok := err.(*DialError) + if !ok { + t.Fatalf("DialError expected, got %#v", err) + } + if !errors.Is(dialerr.Err, context.Canceled) { + t.Fatalf("context.Canceled error expected, got %#v", dialerr.Err) + } +}