Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(routing/http): support lookups with legacy peerid notation #585

Merged
merged 1 commit into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
17 changes: 11 additions & 6 deletions routing/http/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,16 +242,21 @@
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

Check warning on line 258 in routing/http/server/server.go

View check run for this annotation

Codecov / codecov/patch

routing/http/server/server.go#L257-L258

Added lines #L257 - L258 were not covered by tests
}
return
}

pid, err := peer.FromCid(cid)
Expand Down
82 changes: 74 additions & 8 deletions routing/http/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
Expand All @@ -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)
Expand All @@ -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")
Expand All @@ -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) {
Expand Down
Loading