fix(routing/http): support legacy peerids

we already return base58, but we did not support them as input. libp2p specs state implementations MUST support both, it is also better for end users if we are liberal in inputs and support both notations: https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#string-representation
ipfs · Mar 6, 2024 · 97e347e · 97e347e
1 parent 8924807
commit 97e347e
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,8 @@ The following emojis are used to highlight certain changes:
 
 ### Fixed
 
+- 🛠️`routing/http/server`: delegated peer routing endpoint now supports both [PeerID string notaitons from libp2p specs](https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#string-representation).
+
 ### Security
 
 ## [v0.18.0]

diff --git a/routing/http/server/server.go b/routing/http/server/server.go
@@ -242,16 +242,21 @@ func (s *server) findProvidersNDJSON(w http.ResponseWriter, provIter iter.Result
 func (s *server) findPeers(w http.ResponseWriter, r *http.Request) {
 	pidStr := mux.Vars(r)["peer-id"]
 
-	// pidStr must be in CIDv1 format. Therefore, use [cid.Decode]. We can't use
-	// [peer.Decode] because that would allow other formats to pass through.
+	// While specification states that peer-id is expected to be in CIDv1 format, reality
+	// is the clients will often learn legacy PeerID string from other sources,
+	// and try to use it.
+	// See https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#string-representation
+	// We are liberal in inputs here, and uplift legacy PeerID to CID if necessary.
+	// Rationale: it is better to fix this common mistake than to error and break peer routing.
 	cid, err := cid.Decode(pidStr)
 	if err != nil {
-		if pid, err := peer.Decode(pidStr); err == nil {
-			writeErr(w, "FindPeers", http.StatusBadRequest, fmt.Errorf("the value is a peer ID, try using its CID representation: %s", peer.ToCid(pid).String()))
+		// check if input is peer ID in legacy format
+		if pid, err2 := peer.Decode(pidStr); err2 == nil {
+			cid = peer.ToCid(pid)
 		} else {
-			writeErr(w, "FindPeers", http.StatusBadRequest, fmt.Errorf("unable to parse peer ID: %w", err))
+			writeErr(w, "FindPeers", http.StatusBadRequest, fmt.Errorf("unable to parse peer ID as libp2p-key CID: %w", err))
+			return
 		}
-		return
 	}
 
 	pid, err := peer.FromCid(cid)

diff --git a/routing/http/server/server_test.go b/routing/http/server/server_test.go
@@ -147,24 +147,87 @@ func TestPeers(t *testing.T) {
 		return resp
 	}
 
-	t.Run("GET /routing/v1/peers/{non-peer-cid} returns 400", func(t *testing.T) {
+	t.Run("GET /routing/v1/peers/{non-peer-valid-cid} returns 400", func(t *testing.T) {
 		t.Parallel()
 
 		router := &mockContentRouter{}
 		resp := makeRequest(t, router, mediaTypeJSON, "bafkqaaa")
 		require.Equal(t, 400, resp.StatusCode)
 	})
 
-	t.Run("GET /routing/v1/peers/{base58-peer-id} returns 400", func(t *testing.T) {
+	t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body (JSON)", func(t *testing.T) {
 		t.Parallel()
 
 		_, pid := makePeerID(t)
+		results := iter.FromSlice([]iter.Result[*types.PeerRecord]{
+			{Val: &types.PeerRecord{
+				Schema:    types.SchemaPeer,
+				ID:        &pid,
+				Protocols: []string{"transport-bitswap", "transport-foo"},
+				Addrs:     []types.Multiaddr{},
+			}},
+			{Val: &types.PeerRecord{
+				Schema:    types.SchemaPeer,
+				ID:        &pid,
+				Protocols: []string{"transport-foo"},
+				Addrs:     []types.Multiaddr{},
+			}},
+		})
+
 		router := &mockContentRouter{}
-		resp := makeRequest(t, router, mediaTypeJSON, b58.Encode([]byte(pid)))
-		require.Equal(t, 400, resp.StatusCode)
+		router.On("FindPeers", mock.Anything, pid, 20).Return(results, nil)
+
+		libp2pKeyCID := peer.ToCid(pid).String()
+		resp := makeRequest(t, router, mediaTypeJSON, libp2pKeyCID)
+		require.Equal(t, 200, resp.StatusCode)
+
+		header := resp.Header.Get("Content-Type")
+		require.Equal(t, mediaTypeJSON, header)
+
+		body, err := io.ReadAll(resp.Body)
+		require.NoError(t, err)
+
+		expectedBody := `{"Peers":[{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-bitswap","transport-foo"],"Schema":"peer"},{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-foo"],"Schema":"peer"}]}`
+		require.Equal(t, expectedBody, string(body))
+	})
+
+	t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body (NDJSON)", func(t *testing.T) {
+		t.Parallel()
+
+		_, pid := makePeerID(t)
+		results := iter.FromSlice([]iter.Result[*types.PeerRecord]{
+			{Val: &types.PeerRecord{
+				Schema:    types.SchemaPeer,
+				ID:        &pid,
+				Protocols: []string{"transport-bitswap", "transport-foo"},
+				Addrs:     []types.Multiaddr{},
+			}},
+			{Val: &types.PeerRecord{
+				Schema:    types.SchemaPeer,
+				ID:        &pid,
+				Protocols: []string{"transport-foo"},
+				Addrs:     []types.Multiaddr{},
+			}},
+		})
+
+		router := &mockContentRouter{}
+		router.On("FindPeers", mock.Anything, pid, 0).Return(results, nil)
+
+		libp2pKeyCID := peer.ToCid(pid).String()
+		resp := makeRequest(t, router, mediaTypeNDJSON, libp2pKeyCID)
+		require.Equal(t, 200, resp.StatusCode)
+
+		header := resp.Header.Get("Content-Type")
+		require.Equal(t, mediaTypeNDJSON, header)
+
+		body, err := io.ReadAll(resp.Body)
+		require.NoError(t, err)
+
+		expectedBody := `{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-bitswap","transport-foo"],"Schema":"peer"}` + "\n" + `{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-foo"],"Schema":"peer"}` + "\n"
+		require.Equal(t, expectedBody, string(body))
 	})
 
-	t.Run("GET /routing/v1/peers/{cid-peer-id} returns 200 with correct body (JSON)", func(t *testing.T) {
+	t.Run("GET /routing/v1/peers/{legacy-base58-peer-id} returns 200 with correct body (JSON)", func(t *testing.T) {
 		t.Parallel()
 
 		_, pid := makePeerID(t)
@@ -186,7 +249,8 @@ func TestPeers(t *testing.T) {
 		router := &mockContentRouter{}
 		router.On("FindPeers", mock.Anything, pid, 20).Return(results, nil)
 
-		resp := makeRequest(t, router, mediaTypeJSON, peer.ToCid(pid).String())
+		legacyPeerID := b58.Encode([]byte(pid))
+		resp := makeRequest(t, router, mediaTypeJSON, legacyPeerID)
 		require.Equal(t, 200, resp.StatusCode)
 
 		header := resp.Header.Get("Content-Type")
@@ -199,7 +263,7 @@ func TestPeers(t *testing.T) {
 		require.Equal(t, expectedBody, string(body))
 	})
 
-	t.Run("GET /routing/v1/peers/{cid-peer-id} returns 200 with correct body (NDJSON)", func(t *testing.T) {
+	t.Run("GET /routing/v1/peers/{legacy-base58-peer-id} returns 200 with correct body (NDJSON)", func(t *testing.T) {
 		t.Parallel()
 
 		_, pid := makePeerID(t)
@@ -221,7 +285,8 @@ func TestPeers(t *testing.T) {
 		router := &mockContentRouter{}
 		router.On("FindPeers", mock.Anything, pid, 0).Return(results, nil)
 
-		resp := makeRequest(t, router, mediaTypeNDJSON, peer.ToCid(pid).String())
+		legacyPeerID := b58.Encode([]byte(pid))
+		resp := makeRequest(t, router, mediaTypeNDJSON, legacyPeerID)
 		require.Equal(t, 200, resp.StatusCode)
 
 		header := resp.Header.Get("Content-Type")
@@ -233,6 +298,7 @@ func TestPeers(t *testing.T) {
 		expectedBody := `{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-bitswap","transport-foo"],"Schema":"peer"}` + "\n" + `{"Addrs":[],"ID":"` + pid.String() + `","Protocols":["transport-foo"],"Schema":"peer"}` + "\n"
 		require.Equal(t, expectedBody, string(body))
 	})
+
 }
 
 func makeName(t *testing.T) (crypto.PrivKey, ipns.Name) {