Check for no addrs before constructing syncer

Assert that a there are addresses associated to a remote peer before constructing a syncer.
ipni · Feb 15, 2024 · 47e70e2 · 47e70e2
1 parent 08d98c2
commit 47e70e2
Show file tree

Hide file tree

Showing 4 changed files with 88 additions and 14 deletions.
diff --git a/dagsync/ipnisync/sync.go b/dagsync/ipnisync/sync.go
@@ -100,6 +100,22 @@ func (s *Sync) NewSyncer(peerInfo peer.AddrInfo) (*Syncer, error) {
 	if s.authPeerID {
 		rtOpts = append(rtOpts, libp2phttp.ServerMustAuthenticatePeerID)
 	}
+
+	peerInfo = mautil.CleanPeerAddrInfo(peerInfo)
+	if len(peerInfo.Addrs) == 0 {
+		if s.clientHost.StreamHost == nil {
+			return nil, errors.New("no peer addrs and no stream host")
+		}
+		peerStore := s.clientHost.StreamHost.Peerstore()
+		if peerStore == nil {
+			return nil, errors.New("no peer addrs and no stream host peerstore")
+		}
+		peerInfo.Addrs = peerStore.Addrs(peerInfo.ID)
+		if len(peerInfo.Addrs) == 0 {
+			return nil, errors.New("no peer addrs and none found in peertore")
+		}
+	}
+
 	s.clientHostMutex.Lock()
 	cli, err = s.clientHost.NamespacedClient(ProtocolID, peerInfo, rtOpts...)
 	s.clientHostMutex.Unlock()
@@ -131,20 +147,6 @@ func (s *Sync) NewSyncer(peerInfo peer.AddrInfo) (*Syncer, error) {
 		httpClient = rclient.StandardClient()
 	}
 
-	if len(peerInfo.Addrs) == 0 {
-		if s.clientHost.StreamHost == nil {
-			return nil, errors.New("no peer addrs and no stream host")
-		}
-		peerStore := s.clientHost.StreamHost.Peerstore()
-		if peerStore == nil {
-			return nil, errors.New("no peer addrs and no stream host peerstore")
-		}
-		peerInfo.Addrs = peerStore.Addrs(peerInfo.ID)
-		if len(peerInfo.Addrs) == 0 {
-			return nil, errors.New("no peer addrs and none found in peertore")
-		}
-	}
-
 	urls := make([]*url.URL, len(peerInfo.Addrs))
 	for i, addr := range peerInfo.Addrs {
 		u, err := maurl.ToURL(addr)

diff --git a/dagsync/subscriber.go b/dagsync/subscriber.go
@@ -406,6 +406,7 @@ func (s *Subscriber) SyncAdChain(ctx context.Context, peerInfo peer.AddrInfo, op
 		opts.blockHook = s.generalBlockHook
 	}
 
+	peerInfo = mautil.CleanPeerAddrInfo(peerInfo)
 	var err error
 	peerInfo, err = removeIDFromAddrs(peerInfo)
 	if err != nil {
@@ -553,6 +554,7 @@ func (s *Subscriber) syncEntries(ctx context.Context, peerInfo peer.AddrInfo, en
 	s.expSyncMutex.Unlock()
 	defer s.expSyncWG.Done()
 
+	peerInfo = mautil.CleanPeerAddrInfo(peerInfo)
 	peerInfo, err := removeIDFromAddrs(peerInfo)
 	if err != nil {
 		return err

diff --git a/mautil/mautil.go b/mautil/mautil.go
@@ -80,3 +80,15 @@ func StringsToMultiaddrs(addrs []string) ([]multiaddr.Multiaddr, error) {
 	}
 	return maddrs, lastErr
 }
+
+func CleanPeerAddrInfo(target peer.AddrInfo) peer.AddrInfo {
+	for i := 0; i < len(target.Addrs); {
+		if target.Addrs[i] == nil {
+			target.Addrs[i] = target.Addrs[len(target.Addrs)-1]
+			target.Addrs = target.Addrs[:len(target.Addrs)-1]
+			continue
+		}
+		i++
+	}
+	return target
+}
diff --git a/mautil/mautil_test.go b/mautil/mautil_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/ipni/go-libipni/mautil"
+	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/multiformats/go-multiaddr"
 	"github.com/stretchr/testify/require"
 )
@@ -67,3 +68,60 @@ func TestFindHTTPAddrs(t *testing.T) {
 	filtered = mautil.FilterPublic(nil)
 	require.Nil(t, filtered)
 }
+
+func TestCleanPeerAddrInfo(t *testing.T) {
+	t.Run("all-nils", func(t *testing.T) {
+		require.Len(t, mautil.CleanPeerAddrInfo(
+			peer.AddrInfo{
+				Addrs: make([]multiaddr.Multiaddr, 3),
+			}).Addrs, 0)
+	})
+
+	goodAddrs, err := mautil.StringsToMultiaddrs([]string{
+		"/ip4/11.0.0.0/tcp/80/http",
+		"/ip6/fc00::/tcp/1717",
+		"/ip6/fe00::/tcp/8080/https",
+	})
+	require.NoError(t, err)
+
+	t.Run("nil-sandwich", func(t *testing.T) {
+		subject := peer.AddrInfo{
+			Addrs: make([]multiaddr.Multiaddr, 3),
+		}
+		subject.Addrs[1] = goodAddrs[0]
+		cleaned := mautil.CleanPeerAddrInfo(subject)
+		require.Len(t, cleaned.Addrs, 1)
+		require.ElementsMatch(t, cleaned.Addrs, goodAddrs[:1])
+	})
+	t.Run("nil-head", func(t *testing.T) {
+		subject := peer.AddrInfo{
+			Addrs: make([]multiaddr.Multiaddr, 3),
+		}
+		subject.Addrs[1] = goodAddrs[0]
+		subject.Addrs[2] = goodAddrs[1]
+		cleaned := mautil.CleanPeerAddrInfo(subject)
+		require.Len(t, cleaned.Addrs, 2)
+		require.ElementsMatch(t, goodAddrs[:2], cleaned.Addrs)
+	})
+	t.Run("nil-tail", func(t *testing.T) {
+		subject := peer.AddrInfo{
+			Addrs: make([]multiaddr.Multiaddr, 3),
+		}
+		subject.Addrs[0] = goodAddrs[0]
+		subject.Addrs[1] = goodAddrs[1]
+		cleaned := mautil.CleanPeerAddrInfo(subject)
+		require.Len(t, cleaned.Addrs, 2)
+		require.ElementsMatch(t, goodAddrs[:2], cleaned.Addrs)
+	})
+	t.Run("no-nils", func(t *testing.T) {
+		subject := peer.AddrInfo{
+			Addrs: make([]multiaddr.Multiaddr, 3),
+		}
+		subject.Addrs[0] = goodAddrs[0]
+		subject.Addrs[1] = goodAddrs[1]
+		subject.Addrs[2] = goodAddrs[2]
+		cleaned := mautil.CleanPeerAddrInfo(subject)
+		require.Len(t, cleaned.Addrs, 3)
+		require.ElementsMatch(t, goodAddrs, cleaned.Addrs)
+	})
+}