From d2572095b70409a1e07d46e7488dd3ab38fff1e4 Mon Sep 17 00:00:00 2001
From: "Masih H. Derkani" <m@derkani.org>
Date: Wed, 14 Feb 2024 10:02:24 +0000
Subject: [PATCH] Fix empty address check in v0.5.11

Check for empty addrs earlier when instantiating syncer or subscriber.

Patch fix for v0.5.11.

See #155 for fix on `main`.
---
 dagsync/ipnisync/sync.go | 18 +++++++++++++
 dagsync/subscriber.go    |  2 ++
 mautil/mautil.go         | 12 +++++++++
 mautil/mautil_test.go    | 58 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+)

diff --git a/dagsync/ipnisync/sync.go b/dagsync/ipnisync/sync.go
index 8b4feed..dab2572 100644
--- a/dagsync/ipnisync/sync.go
+++ b/dagsync/ipnisync/sync.go
@@ -100,6 +100,24 @@ 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")
+		}
+
+		// TODO: double check that libp2p peerstore never returns nil addrs.
+	}
+
 	s.clientHostMutex.Lock()
 	cli, err = s.clientHost.NamespacedClient(ProtocolID, peerInfo, rtOpts...)
 	s.clientHostMutex.Unlock()
diff --git a/dagsync/subscriber.go b/dagsync/subscriber.go
index c2db8e8..342d4aa 100644
--- a/dagsync/subscriber.go
+++ b/dagsync/subscriber.go
@@ -415,6 +415,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 {
@@ -563,6 +564,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
index 2a48999..4d5a9bc 100644
--- 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
index d4b4a92..59c8479 100644
--- 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)
+	})
+}