Repository: libp2p/go-libp2p
Pull request #2992 (2024-10-02 12:30:39)

autonatv2: recover from panics


Patch:
diff --git a/p2p/host/basic/basic_host.go b/p2p/host/basic/basic_host.go
index 761a39b6..b5d252e9 100644
--- a/p2p/host/basic/basic_host.go
+++ b/p2p/host/basic/basic_host.go
@@ -500,7 +500,6 @@ func (h *BasicHost) makeUpdatedAddrEvent(prev, current []ma.Multiaddr) *event.Ev
 		return nil
 	}
 	prevmap := make(map[string]ma.Multiaddr, len(prev))
-	currmap := make(map[string]ma.Multiaddr, len(current))
 	evt := &event.EvtLocalAddressesUpdated{Diffs: true}
 	addrsAdded := false
 
@@ -508,9 +507,6 @@ func (h *BasicHost) makeUpdatedAddrEvent(prev, current []ma.Multiaddr) *event.Ev
 		prevmap[string(addr.Bytes())] = addr
 	}
 	for _, addr := range current {
-		currmap[string(addr.Bytes())] = addr
-	}
-	for _, addr := range currmap {
 		_, ok := prevmap[string(addr.Bytes())]
 		updated := event.UpdatedAddress{Address: addr}
 		if ok {
@@ -835,7 +831,7 @@ func (h *BasicHost) Addrs() []ma.Multiaddr {
 	// Make a copy. Consumers can modify the slice elements
 	res := make([]ma.Multiaddr, len(addrs))
 	copy(res, addrs)
-	return ma.Unique(res)
+	return res
 }
 
 // NormalizeMultiaddr returns a multiaddr suitable for equality checks.
diff --git a/p2p/protocol/autonatv2/autonat.go b/p2p/protocol/autonatv2/autonat.go
index 3dc3bc16..888a95b7 100644
--- a/p2p/protocol/autonatv2/autonat.go
+++ b/p2p/protocol/autonatv2/autonat.go
@@ -25,9 +25,9 @@ const (
 	DialProtocol     = "/libp2p/autonat/2/dial-request"
 
 	maxMsgSize            = 8192
-	streamTimeout         = time.Minute
+	streamTimeout         = 15 * time.Second
 	dialBackStreamTimeout = 5 * time.Second
-	dialBackDialTimeout   = 30 * time.Second
+	dialBackDialTimeout   = 10 * time.Second
 	dialBackMaxMsgSize    = 1024
 	minHandshakeSizeBytes = 30_000 // for amplification attack prevention
 	maxHandshakeSizeBytes = 100_000
diff --git a/p2p/protocol/autonatv2/client.go b/p2p/protocol/autonatv2/client.go
index c1d82b7f..6ed7a6bb 100644
--- a/p2p/protocol/autonatv2/client.go
+++ b/p2p/protocol/autonatv2/client.go
@@ -3,6 +3,8 @@ package autonatv2
 import (
 	"context"
 	"fmt"
+	"os"
+	"runtime/debug"
 	"sync"
 	"time"
 
@@ -248,6 +250,13 @@ func newDialRequest(reqs []Request, nonce uint64) pb.Message {
 
 // handleDialBack receives the nonce on the dial-back stream
 func (ac *client) handleDialBack(s network.Stream) {
+	defer func() {
+		if rerr := recover(); rerr != nil {
+			fmt.Fprintf(os.Stderr, "caught panic: %s\n%s\n", rerr, debug.Stack())
+		}
+		s.Reset()
+	}()
+
 	if err := s.Scope().SetService(ServiceName); err != nil {
 		log.Debugf("failed to attach stream to service %s: %w", ServiceName, err)
 		s.Reset()
diff --git a/p2p/protocol/autonatv2/server.go b/p2p/protocol/autonatv2/server.go
index c5dd2dce..d28cfc93 100644
--- a/p2p/protocol/autonatv2/server.go
+++ b/p2p/protocol/autonatv2/server.go
@@ -5,6 +5,8 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"os"
+	"runtime/debug"
 	"sync"
 	"time"
 
@@ -88,6 +90,14 @@ func (as *server) Close() {
 
 // handleDialRequest is the dial-request protocol stream handler
 func (as *server) handleDialRequest(s network.Stream) {
+	defer func() {
+		if rerr := recover(); rerr != nil {
+			fmt.Fprintf(os.Stderr, "caught panic: %s\n%s\n", rerr, debug.Stack())
+			s.Reset()
+		}
+	}()
+
+	log.Debugf("received dial-request from: %s, addr: %s", s.Conn().RemotePeer(), s.Conn().RemoteMultiaddr())
 	evt := as.serveDialRequest(s)
 	log.Debugf("completed dial-request from %s, response status: %s, dial status: %s, err: %s",
 		s.Conn().RemotePeer(), evt.ResponseStatus, evt.DialStatus, evt.Error)


Test patch:
diff --git a/p2p/host/basic/basic_host_test.go b/p2p/host/basic/basic_host_test.go
index 1ab98aae..9014e1b4 100644
--- a/p2p/host/basic/basic_host_test.go
+++ b/p2p/host/basic/basic_host_test.go
@@ -10,7 +10,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/libp2p/go-libp2p-testing/race"
 	"github.com/libp2p/go-libp2p/core/event"
 	"github.com/libp2p/go-libp2p/core/host"
 	"github.com/libp2p/go-libp2p/core/network"
@@ -249,63 +248,6 @@ func TestAllAddrs(t *testing.T) {
 	require.True(t, ma.Contains(h.AllAddrs(), firstAddr), "should still contain the original addr")
 }
 
-func TestAllAddrsUnique(t *testing.T) {
-	if race.WithRace() {
-		t.Skip("updates addrChangeTickrInterval which might be racy")
-	}
-	oldInterval := addrChangeTickrInterval
-	addrChangeTickrInterval = 100 * time.Millisecond
-	defer func() {
-		addrChangeTickrInterval = oldInterval
-	}()
-	sendNewAddrs := make(chan struct{})
-	opts := HostOpts{
-		AddrsFactory: func(addrs []ma.Multiaddr) []ma.Multiaddr {
-			select {
-			case <-sendNewAddrs:
-				return []ma.Multiaddr{
-					ma.StringCast("/ip4/1.2.3.4/tcp/1"),
-					ma.StringCast("/ip4/1.2.3.4/tcp/1"),
-					ma.StringCast("/ip4/1.2.3.4/tcp/1"),
-					ma.StringCast("/ip4/1.2.3.4/udp/1/quic-v1"),
-					ma.StringCast("/ip4/1.2.3.4/udp/1/quic-v1"),
-				}
-			default:
-				return nil
-			}
-		},
-	}
-	// no listen addrs
-	h, err := NewHost(swarmt.GenSwarm(t, swarmt.OptDialOnly), &opts)
-	require.NoError(t, err)
-	defer h.Close()
-	h.Start()
-
-	sub, err := h.EventBus().Subscribe(&event.EvtLocalAddressesUpdated{})
-	require.NoError(t, err)
-	out := make(chan int)
-	done := make(chan struct{})
-	go func() {
-		cnt := 0
-		for {
-			select {
-			case <-sub.Out():
-				cnt++
-			case <-done:
-				out <- cnt
-				return
-			}
-		}
-	}()
-	close(sendNewAddrs)
-	require.Len(t, h.Addrs(), 2)
-	require.ElementsMatch(t, []ma.Multiaddr{ma.StringCast("/ip4/1.2.3.4/tcp/1"), ma.StringCast("/ip4/1.2.3.4/udp/1/quic-v1")}, h.Addrs())
-	time.Sleep(2*addrChangeTickrInterval + 1*time.Second) // the background loop runs every 5 seconds. Wait for 2x that time.
-	close(done)
-	cnt := <-out
-	require.Equal(t, 1, cnt)
-}
-
 // getHostPair gets a new pair of hosts.
 // The first host initiates the connection to the second host.
 func getHostPair(t *testing.T) (host.Host, host.Host) {
diff --git a/p2p/protocol/autonatv2/autonat_test.go b/p2p/protocol/autonatv2/autonat_test.go
index ee97ccc6..11c8f021 100644
--- a/p2p/protocol/autonatv2/autonat_test.go
+++ b/p2p/protocol/autonatv2/autonat_test.go
@@ -657,5 +657,4 @@ func TestAreAddrsConsistency(t *testing.T) {
 			}
 		})
 	}
-
 }