From 713a74d9a1b0fb1160e755ab23f63817f6ec3096 Mon Sep 17 00:00:00 2001
From: "Masih H. Derkani" <m@derkani.org>
Date: Wed, 28 Jun 2023 15:11:12 +0100
Subject: [PATCH] Fix bug in provider cache aggregation rage loop

The loop responsible for aggregating contextual and chain level extended
providers ranges over the values fetched from indexers, passes the
values by reference but does not redeclare the value in each iteration.

Redeclare the value to make sure future iterations do not alter the
providers already added to slice.

While at it fix similar issues in other places.
---
 find/client/provider_cache.go      |   3 +
 find/client/provider_cache_test.go | 106 +++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+)
 create mode 100644 find/client/provider_cache_test.go

diff --git a/find/client/provider_cache.go b/find/client/provider_cache.go
index 661033f..7738c41 100644
--- a/find/client/provider_cache.go
+++ b/find/client/provider_cache.go
@@ -114,6 +114,7 @@ func (pc *providerCache) getResults(ctx context.Context, pid peer.ID, ctxID []by
 		}
 		if pinfo.ExtendedProviders != nil {
 			for _, cxp := range pinfo.ExtendedProviders.Contextual {
+				cxp := cxp
 				wrapper.cxps[cxp.ContextID] = &cxp
 			}
 		}
@@ -141,6 +142,7 @@ func (pc *providerCache) getResults(ctx context.Context, pid peer.ID, ctxID []by
 	if contextualEpRecord, ok := wrapper.cxps[string(ctxID)]; ok {
 		override = contextualEpRecord.Override
 		for i, xpinfo := range contextualEpRecord.Providers {
+			xpinfo := xpinfo
 			xmd := contextualEpRecord.Metadatas[i]
 			// Skippng the main provider's record if its metadata is nil or is
 			// the same as the one retrieved from the indexer, because such EP
@@ -170,6 +172,7 @@ func (pc *providerCache) getResults(ctx context.Context, pid peer.ID, ctxID []by
 
 	// Adding chain-level EPs if such exist
 	for i, xpinfo := range wrapper.pinfo.ExtendedProviders.Providers {
+		xpinfo := xpinfo
 		xmd := wrapper.pinfo.ExtendedProviders.Metadatas[i]
 		// Skippng the main provider's record if its metadata is nil or is the
 		// same as the one retrieved from the indexer, because such EP record
diff --git a/find/client/provider_cache_test.go b/find/client/provider_cache_test.go
new file mode 100644
index 0000000..e15ca69
--- /dev/null
+++ b/find/client/provider_cache_test.go
@@ -0,0 +1,106 @@
+package client
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/ipni/go-libipni/find/model"
+	"github.com/stretchr/testify/require"
+)
+
+func TestChainLevelExtendedProviderIsAsExpected(t *testing.T) {
+	var want []model.ProviderResult
+	err := json.Unmarshal([]byte(`[
+  {
+    "ContextID": "bG9ic3Rlcg==",
+    "Metadata": "YmFycmVsZXll",
+    "Provider": {
+      "ID": "12D3KooWNSRG5wTShNu6EXCPTkoH7dWsphKAPrbvQchHa5arfsDC",
+      "Addrs": [
+        "/ip4/209.94.92.6/tcp/24001"
+      ]
+    }
+  },
+  {
+    "ContextID": "bG9ic3Rlcg==",
+    "Metadata": "gBI=",
+    "Provider": {
+      "ID": "12D3KooWHf7cahZvAVB36SGaVXc7fiVDoJdRJq42zDRcN2s2512h",
+      "Addrs": [
+        "/ip4/209.94.92.6/tcp/24123"
+      ]
+    }
+  },
+  {
+    "ContextID": "bG9ic3Rlcg==",
+    "Metadata": "oBIA",
+    "Provider": {
+      "ID": "12D3KooWNSRG5wTShNu6EXCPTkoH7dWsphKAPrbvQchHa5arfsDC",
+      "Addrs": [
+        "/ip4/209.94.92.6/tcp/7575/http"
+      ]
+    }
+  }
+]`), &want)
+	require.NoError(t, err)
+
+	testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
+		_, err := res.Write([]byte(`{
+  "AddrInfo": {
+    "ID": "12D3KooWNSRG5wTShNu6EXCPTkoH7dWsphKAPrbvQchHa5arfsDC",
+    "Addrs": [
+      "/ip4/209.94.92.6/tcp/24001"
+    ]
+  },
+  "LastAdvertisement": {
+    "/": "baguqeerami47x7t3d4raobz7s2cq5np2fy2lhl3xh2ppiwuevxwbdamuikla"
+  },
+  "LastAdvertisementTime": "2023-06-28T12:21:06Z",
+  "Publisher": {
+    "ID": "12D3KooWNSRG5wTShNu6EXCPTkoH7dWsphKAPrbvQchHa5arfsDC",
+    "Addrs": [
+      "/ip4/209.94.92.6/tcp/24001"
+    ]
+  },
+  "IndexCount": 157535217,
+  "ExtendedProviders": {
+    "Providers": [
+      {
+        "ID": "12D3KooWHf7cahZvAVB36SGaVXc7fiVDoJdRJq42zDRcN2s2512h",
+        "Addrs": [
+          "/ip4/209.94.92.6/tcp/24123"
+        ]
+      },
+      {
+        "ID": "12D3KooWNSRG5wTShNu6EXCPTkoH7dWsphKAPrbvQchHa5arfsDC",
+        "Addrs": [
+          "/ip4/209.94.92.6/tcp/7575/http"
+        ]
+      }
+    ],
+    "Metadatas": [
+      "gBI=",
+      "oBIA"
+    ]
+  },
+  "FrozenAt": null
+}`))
+		require.NoError(t, err)
+	}))
+	defer func() { testServer.Close() }()
+
+	u, err := url.Parse(testServer.URL)
+	require.NoError(t, err)
+	subject, err := newProviderCache(u, nil)
+	require.NoError(t, err)
+
+	contextID := []byte("lobster")
+	metadata := []byte("barreleye")
+	got, err := subject.getResults(context.Background(), "fish", contextID, metadata)
+	require.NoError(t, err)
+	require.Equal(t, want, got)
+}