From 0a302dfd80f9e60ab102e6c10f1a1ffe0b2054cc Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 12 Apr 2024 17:22:10 +0200 Subject: [PATCH 1/3] fix(measurexlite): add robust RemoteAddr accessors Closes https://github.com/ooni/probe/issues/2707 --- internal/measurexlite/conn.go | 26 ++++++++++++++++++++++---- internal/measurexlite/conn_test.go | 21 +++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/internal/measurexlite/conn.go b/internal/measurexlite/conn.go index 54beade596..ae107e88b8 100644 --- a/internal/measurexlite/conn.go +++ b/internal/measurexlite/conn.go @@ -39,11 +39,29 @@ type connTrace struct { var _ net.Conn = &connTrace{} +type remoteAddrProvider interface { + RemoteAddr() net.Addr +} + +func safeRemoteAddrNetwork(rap remoteAddrProvider) (result string) { + if addr := rap.RemoteAddr(); addr != nil { + result = addr.Network() + } + return result +} + +func safeRemoteAddrString(rap remoteAddrProvider) (result string) { + if addr := rap.RemoteAddr(); addr != nil { + result = addr.String() + } + return result +} + // Read implements net.Conn.Read and saves network events. func (c *connTrace) Read(b []byte) (int, error) { // collect preliminary stats when the connection is surely active - network := c.RemoteAddr().Network() - addr := c.RemoteAddr().String() + network := safeRemoteAddrNetwork(c) + addr := safeRemoteAddrString(c) started := c.tx.TimeSince(c.tx.ZeroTime()) // perform the underlying network operation @@ -99,8 +117,8 @@ func (tx *Trace) CloneBytesReceivedMap() (out map[string]int64) { // Write implements net.Conn.Write and saves network events. func (c *connTrace) Write(b []byte) (int, error) { - network := c.RemoteAddr().Network() - addr := c.RemoteAddr().String() + network := safeRemoteAddrNetwork(c) + addr := safeRemoteAddrString(c) started := c.tx.TimeSince(c.tx.ZeroTime()) count, err := c.Conn.Write(b) diff --git a/internal/measurexlite/conn_test.go b/internal/measurexlite/conn_test.go index 563ce31db8..066f4d6872 100644 --- a/internal/measurexlite/conn_test.go +++ b/internal/measurexlite/conn_test.go @@ -12,6 +12,27 @@ import ( "github.com/ooni/probe-cli/v3/internal/testingx" ) +func TestRemoteAddrProvider(t *testing.T) { + conn := &mocks.Conn{ + MockRemoteAddr: func() net.Addr { + return &mocks.Addr{ + MockString: func() string { + return "1.1.1.1:443" + }, + MockNetwork: func() string { + return "tcp" + }, + } + }, + } + if safeRemoteAddrNetwork(conn) != "tcp" { + t.Fatal("unexpected network") + } + if safeRemoteAddrString(conn) != "1.1.1.1:443" { + t.Fatal("unexpected string") + } +} + func TestMaybeClose(t *testing.T) { t.Run("with nil conn", func(t *testing.T) { var conn net.Conn = nil From 0936d94d74f9dfdbb23b2c8ad3ad7fe83d3bba23 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 12 Apr 2024 17:44:25 +0200 Subject: [PATCH 2/3] x --- internal/measurexlite/conn_test.go | 52 +++++++++++++++++++----------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/internal/measurexlite/conn_test.go b/internal/measurexlite/conn_test.go index 066f4d6872..d8f74182dc 100644 --- a/internal/measurexlite/conn_test.go +++ b/internal/measurexlite/conn_test.go @@ -13,24 +13,40 @@ import ( ) func TestRemoteAddrProvider(t *testing.T) { - conn := &mocks.Conn{ - MockRemoteAddr: func() net.Addr { - return &mocks.Addr{ - MockString: func() string { - return "1.1.1.1:443" - }, - MockNetwork: func() string { - return "tcp" - }, - } - }, - } - if safeRemoteAddrNetwork(conn) != "tcp" { - t.Fatal("unexpected network") - } - if safeRemoteAddrString(conn) != "1.1.1.1:443" { - t.Fatal("unexpected string") - } + t.Run("for nil address", func(t *testing.T) { + conn := &mocks.Conn{ + MockRemoteAddr: func() net.Addr { + return nil + }, + } + if safeRemoteAddrNetwork(conn) != "" { + t.Fatal("unexpected empty network") + } + if safeRemoteAddrString(conn) != "" { + t.Fatal("unexpected empty string") + } + }) + + t.Run("for common case", func(t *testing.T) { + conn := &mocks.Conn{ + MockRemoteAddr: func() net.Addr { + return &mocks.Addr{ + MockString: func() string { + return "1.1.1.1:443" + }, + MockNetwork: func() string { + return "tcp" + }, + } + }, + } + if safeRemoteAddrNetwork(conn) != "tcp" { + t.Fatal("unexpected network") + } + if safeRemoteAddrString(conn) != "1.1.1.1:443" { + t.Fatal("unexpected string") + } + }) } func TestMaybeClose(t *testing.T) { From 1315767516ebc380f4c76838e29b5a06304e5f52 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 12 Apr 2024 17:45:30 +0200 Subject: [PATCH 3/3] x --- internal/measurexlite/conn_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/measurexlite/conn_test.go b/internal/measurexlite/conn_test.go index d8f74182dc..d69f52780b 100644 --- a/internal/measurexlite/conn_test.go +++ b/internal/measurexlite/conn_test.go @@ -20,10 +20,10 @@ func TestRemoteAddrProvider(t *testing.T) { }, } if safeRemoteAddrNetwork(conn) != "" { - t.Fatal("unexpected empty network") + t.Fatal("expected empty network") } if safeRemoteAddrString(conn) != "" { - t.Fatal("unexpected empty string") + t.Fatal("expected empty string") } })