From 4941636e6187553f29bae029361b3f8e59afe1e6 Mon Sep 17 00:00:00 2001 From: Jay Stenmark Date: Thu, 13 Feb 2025 23:38:34 +0000 Subject: [PATCH] Fix data race in WriteTo There is no harm in shadowing `err` in the goroutine since we do not need it outside of the routine. I also discovered data races in tcp_conn_test.go while fixing this bug and have included them in this PR. --- internal/client/tcp_conn_test.go | 8 ++++---- internal/client/udp_conn.go | 5 ++--- lt_cred.go | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/internal/client/tcp_conn_test.go b/internal/client/tcp_conn_test.go index e3e4bdc3..fd9e2640 100644 --- a/internal/client/tcp_conn_test.go +++ b/internal/client/tcp_conn_test.go @@ -92,12 +92,12 @@ func TestTCPConn(t *testing.T) { client = &mockClient{ performTransaction: func(msg *stun.Message, _ net.Addr, _ bool) (TransactionResult, error) { if msg.Type.Class == stun.ClassRequest && msg.Type.Method == stun.MethodConnect { - msg, err = stun.Build( + msg, buildErr := stun.Build( stun.TransactionID, stun.NewType(stun.MethodConnect, stun.ClassErrorResponse), stun.ErrorCodeAttribute{Code: stun.CodeBadRequest}, ) - assert.NoError(t, err) + assert.NoError(t, buildErr) return TransactionResult{Msg: msg}, nil } @@ -174,12 +174,12 @@ func TestTCPConn(t *testing.T) { typ = stun.NewType(stun.MethodCreatePermission, stun.ClassSuccessResponse) } - msg, err = stun.Build( + msg, buildErr := stun.Build( stun.TransactionID, typ, cid, ) - assert.NoError(t, err) + assert.NoError(t, buildErr) return TransactionResult{Msg: msg}, nil }, diff --git a/internal/client/udp_conn.go b/internal/client/udp_conn.go index 2a6af1bf..59d10cea 100644 --- a/internal/client/udp_conn.go +++ b/internal/client/udp_conn.go @@ -240,9 +240,8 @@ func (c *UDPConn) WriteTo(payload []byte, addr net.Addr) (int, error) { //nolint if bound.state() == bindingStateReady && time.Since(bound.refreshedAt()) > 5*time.Minute { bound.setState(bindingStateRefresh) go func() { - err = c.bind(bound) - if err != nil { - c.log.Warnf("Failed to bind() for refresh: %s", err) + if bindErr := c.bind(bound); bindErr != nil { + c.log.Warnf("Failed to bind() for refresh: %s", bindErr) bound.setState(bindingStateFailed) // Keep going... } else { diff --git a/lt_cred.go b/lt_cred.go index 33ca3e86..65576be2 100644 --- a/lt_cred.go +++ b/lt_cred.go @@ -3,7 +3,7 @@ package turn -import ( //nolint:gci +import ( "crypto/hmac" "crypto/sha1" //nolint:gosec,gci "encoding/base64"