From c165e4b30fbc4d8138f54be2cf86384059cdf9fb Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 5 Mar 2024 18:19:55 +0100 Subject: [PATCH] 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 --- CHANGELOG.md | 2 + routing/http/server/server.go | 17 ++++--- routing/http/server/server_test.go | 82 +++++++++++++++++++++++++++--- 3 files changed, 87 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 785b27904..3d148b03c 100644 --- 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 index d9be47eb2..df93c57fd 100644 --- 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 index f823ac25a..c767d8f2e 100644 --- a/routing/http/server/server_test.go +++ b/routing/http/server/server_test.go @@ -147,7 +147,7 @@ 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{} @@ -155,16 +155,79 @@ func TestPeers(t *testing.T) { 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) {