From 250a595f8962377aa148fb853ffc1e282839c4f9 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 1 Jul 2021 21:56:29 +0200 Subject: [PATCH] refactor: cleaner way of passing a UDPConn around (#421) * refactor: cleaner way of passing a UDPConn around Also part of https://github.com/ooni/probe/issues/1505 * Update internal/engine/netx/quicdialer/connectionstate.go --- .../engine/netx/quicdialer/connectionstate.go | 4 ++-- internal/engine/netx/quicdialer/quicdialer.go | 6 ------ internal/engine/netx/quicdialer/saver.go | 2 +- internal/engine/netx/quicdialer/system.go | 17 ++++++----------- internal/netxlite/quic.go | 7 ++++--- internal/netxlite/quic_test.go | 3 ++- internal/netxmocks/quic.go | 5 +++-- internal/netxmocks/quic_test.go | 3 ++- internal/quicx/quicx.go | 13 +++++++++++++ 9 files changed, 33 insertions(+), 27 deletions(-) create mode 100644 internal/quicx/quicx.go diff --git a/internal/engine/netx/quicdialer/connectionstate.go b/internal/engine/netx/quicdialer/connectionstate.go index eb69462..7cb37c6 100644 --- a/internal/engine/netx/quicdialer/connectionstate.go +++ b/internal/engine/netx/quicdialer/connectionstate.go @@ -6,7 +6,7 @@ import ( "github.com/lucas-clemente/quic-go" ) -// ConnectionState returns the ConnectionState of a QUIC Session. -func ConnectionState(sess quic.EarlySession) tls.ConnectionState { +// connectionState returns the ConnectionState of a QUIC Session. +func connectionState(sess quic.EarlySession) tls.ConnectionState { return sess.ConnectionState().TLS.ConnectionState } diff --git a/internal/engine/netx/quicdialer/quicdialer.go b/internal/engine/netx/quicdialer/quicdialer.go index 4b5aa99..1babc87 100644 --- a/internal/engine/netx/quicdialer/quicdialer.go +++ b/internal/engine/netx/quicdialer/quicdialer.go @@ -14,12 +14,6 @@ type ContextDialer interface { tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) } -// Dialer dials QUIC connections. -type Dialer interface { - // Note: assumes that tlsCfg and cfg are not nil. - Dial(network, addr string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) -} - // Resolver is the interface we expect from a resolver. type Resolver interface { LookupHost(ctx context.Context, hostname string) (addrs []string, err error) diff --git a/internal/engine/netx/quicdialer/saver.go b/internal/engine/netx/quicdialer/saver.go index d569d4e..76bd28b 100644 --- a/internal/engine/netx/quicdialer/saver.go +++ b/internal/engine/netx/quicdialer/saver.go @@ -45,7 +45,7 @@ func (h HandshakeSaver) DialContext(ctx context.Context, network string, }) return nil, err } - state := ConnectionState(sess) + state := connectionState(sess) h.Saver.Write(trace.Event{ Duration: stop.Sub(start), Name: "quic_handshake_done", diff --git a/internal/engine/netx/quicdialer/system.go b/internal/engine/netx/quicdialer/system.go index fbb39f4..c6f4222 100644 --- a/internal/engine/netx/quicdialer/system.go +++ b/internal/engine/netx/quicdialer/system.go @@ -1,18 +1,18 @@ package quicdialer import ( - "errors" "net" "time" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/quicx" ) // QUICListener listens for QUIC connections. type QUICListener interface { - // Listen creates a new listening PacketConn. - Listen(addr *net.UDPAddr) (net.PacketConn, error) + // Listen creates a new listening UDPConn. + Listen(addr *net.UDPAddr) (quicx.UDPConn, error) } // QUICListenerSaver is a QUICListener that also implements saving events. @@ -25,21 +25,16 @@ type QUICListenerSaver struct { } // Listen implements QUICListener.Listen. -func (qls *QUICListenerSaver) Listen(addr *net.UDPAddr) (net.PacketConn, error) { +func (qls *QUICListenerSaver) Listen(addr *net.UDPAddr) (quicx.UDPConn, error) { pconn, err := qls.QUICListener.Listen(addr) if err != nil { return nil, err } - // TODO(bassosimone): refactor to remove this restriction. - udpConn, ok := pconn.(*net.UDPConn) - if !ok { - return nil, errors.New("quicdialer: cannot convert to udpConn") - } - return saverUDPConn{UDPConn: udpConn, saver: qls.Saver}, nil + return saverUDPConn{UDPConn: pconn, saver: qls.Saver}, nil } type saverUDPConn struct { - *net.UDPConn + quicx.UDPConn saver *trace.Saver } diff --git a/internal/netxlite/quic.go b/internal/netxlite/quic.go index 5473953..3e6173f 100644 --- a/internal/netxlite/quic.go +++ b/internal/netxlite/quic.go @@ -8,6 +8,7 @@ import ( "strconv" "github.com/lucas-clemente/quic-go" + "github.com/ooni/probe-cli/v3/internal/quicx" ) // QUICContextDialer is a dialer for QUIC using Context. @@ -21,8 +22,8 @@ type QUICContextDialer interface { // QUICListener listens for QUIC connections. type QUICListener interface { - // Listen creates a new listening PacketConn. - Listen(addr *net.UDPAddr) (net.PacketConn, error) + // Listen creates a new listening UDPConn. + Listen(addr *net.UDPAddr) (quicx.UDPConn, error) } // QUICListenerStdlib is a QUICListener using the standard library. @@ -31,7 +32,7 @@ type QUICListenerStdlib struct{} var _ QUICListener = &QUICListenerStdlib{} // Listen implements QUICListener.Listen. -func (qls *QUICListenerStdlib) Listen(addr *net.UDPAddr) (net.PacketConn, error) { +func (qls *QUICListenerStdlib) Listen(addr *net.UDPAddr) (quicx.UDPConn, error) { return net.ListenUDP("udp", addr) } diff --git a/internal/netxlite/quic_test.go b/internal/netxlite/quic_test.go index e41ede4..7b8286a 100644 --- a/internal/netxlite/quic_test.go +++ b/internal/netxlite/quic_test.go @@ -12,6 +12,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/lucas-clemente/quic-go" "github.com/ooni/probe-cli/v3/internal/netxmocks" + "github.com/ooni/probe-cli/v3/internal/quicx" ) func TestQUICDialerQUICGoCannotSplitHostPort(t *testing.T) { @@ -75,7 +76,7 @@ func TestQUICDialerQUICGoCannotListen(t *testing.T) { } systemdialer := QUICDialerQUICGo{ QUICListener: &netxmocks.QUICListener{ - MockListen: func(addr *net.UDPAddr) (net.PacketConn, error) { + MockListen: func(addr *net.UDPAddr) (quicx.UDPConn, error) { return nil, expected }, }, diff --git a/internal/netxmocks/quic.go b/internal/netxmocks/quic.go index 6a38b52..10bc935 100644 --- a/internal/netxmocks/quic.go +++ b/internal/netxmocks/quic.go @@ -6,15 +6,16 @@ import ( "net" "github.com/lucas-clemente/quic-go" + "github.com/ooni/probe-cli/v3/internal/quicx" ) // QUICListener is a mockable netxlite.QUICListener. type QUICListener struct { - MockListen func(addr *net.UDPAddr) (net.PacketConn, error) + MockListen func(addr *net.UDPAddr) (quicx.UDPConn, error) } // Listen calls MockListen. -func (ql *QUICListener) Listen(addr *net.UDPAddr) (net.PacketConn, error) { +func (ql *QUICListener) Listen(addr *net.UDPAddr) (quicx.UDPConn, error) { return ql.MockListen(addr) } diff --git a/internal/netxmocks/quic_test.go b/internal/netxmocks/quic_test.go index fd7e7e0..dac90e2 100644 --- a/internal/netxmocks/quic_test.go +++ b/internal/netxmocks/quic_test.go @@ -9,12 +9,13 @@ import ( "testing" "github.com/lucas-clemente/quic-go" + "github.com/ooni/probe-cli/v3/internal/quicx" ) func TestQUICListenerListen(t *testing.T) { expected := errors.New("mocked error") ql := &QUICListener{ - MockListen: func(addr *net.UDPAddr) (net.PacketConn, error) { + MockListen: func(addr *net.UDPAddr) (quicx.UDPConn, error) { return nil, expected }, } diff --git a/internal/quicx/quicx.go b/internal/quicx/quicx.go new file mode 100644 index 0000000..ca9f553 --- /dev/null +++ b/internal/quicx/quicx.go @@ -0,0 +1,13 @@ +// Package quicx contains definitions useful to implement QUIC. +package quicx + +import "net" + +// UDPConn is an UDP connection used by quic. +type UDPConn interface { + // PacketConn is the underlying base interface. + net.PacketConn + + // ReadMsgUDP behaves like net.UDPConn.ReadMsgUDP. + ReadMsgUDP(b, oob []byte) (n, oobn, flags int, addr *net.UDPAddr, err error) +}