From 43eb24b79ae8e921df813389cbb9dc36aa87942f Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Thu, 12 Sep 2024 17:52:35 +0200 Subject: [PATCH 01/20] feat: add protocol filtering implements https://github.com/ipfs/specs/pull/484 --- routing/http/server/filters.go | 143 +++++++++++++ routing/http/server/filters_test.go | 309 ++++++++++++++++++++++++++++ routing/http/server/server.go | 84 +++++++- routing/http/server/server_test.go | 68 ++++-- 4 files changed, 581 insertions(+), 23 deletions(-) create mode 100644 routing/http/server/filters.go create mode 100644 routing/http/server/filters_test.go diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go new file mode 100644 index 000000000..e9411a00a --- /dev/null +++ b/routing/http/server/filters.go @@ -0,0 +1,143 @@ +package server + +import ( + "reflect" + "slices" + "strings" + + "github.com/ipfs/boxo/routing/http/types" + "github.com/multiformats/go-multiaddr" +) + +// filters implements IPIP-0484 + +func parseFilter(param string) []string { + if param == "" { + return nil + } + return strings.Split(strings.ToLower(param), ",") +} + +func filterProviders(providers []types.Record, filterAddrs, filterProtocols []string) []types.Record { + if len(filterAddrs) == 0 && len(filterProtocols) == 0 { + return providers + } + + filtered := make([]types.Record, 0, len(providers)) + + for _, provider := range providers { + if schema := provider.GetSchema(); schema == types.SchemaPeer { + peer, ok := provider.(*types.PeerRecord) + if !ok { + logger.Errorw("problem casting find providers result", "Schema", provider.GetSchema(), "Type", reflect.TypeOf(provider).String()) + // if the type assertion fails, we exlude record from results + continue + } + + record := applyFilters(peer, filterAddrs, filterProtocols) + + if record != nil { + filtered = append(filtered, record) + } + + } else { + // Will we ever encounter the SchemaBitswap type? Evidence seems to suggest that no longer + logger.Errorw("encountered unknown provider schema", "Schema", provider.GetSchema(), "Type", reflect.TypeOf(provider).String()) + } + } + return filtered +} + +// Applies the filters. Returns nil if the provider does not pass the protocols filter +// The address filter is more complicated because it potentially modifies the Addrs slice. +func applyFilters(provider *types.PeerRecord, filterAddrs, filterProtocols []string) *types.PeerRecord { + if !applyProtocolFilter(provider.Protocols, filterProtocols) { + // If the provider doesn't match any of the passed protocols, the provider is omitted from the response. + return nil + } + + // return untouched if there's no filter or filterAddrsQuery contains "unknown" and provider has no addrs + if len(filterAddrs) == 0 || (len(provider.Addrs) == 0 && slices.Contains(filterAddrs, "unknown")) { + return provider + } + + filteredAddrs := applyAddrFilter(provider.Addrs, filterAddrs) + + // If filtering resulted in no addrs, omit the provider + if len(filteredAddrs) == 0 { + return nil + } + + provider.Addrs = filteredAddrs + return provider +} + +// If there are only negative filters, no addresses will be included in the result. The function will return an empty list. +// For an address to be included, it must pass all negative filters AND match at least one positive filter. +func applyAddrFilter(addrs []types.Multiaddr, filterAddrsQuery []string) []types.Multiaddr { + if len(filterAddrsQuery) == 0 { + return addrs + } + + filteredAddrs := make([]types.Multiaddr, 0, len(addrs)) + + for _, addr := range addrs { + protocols := addr.Protocols() + includeAddr := true + + // First, check all negative filters + for _, filter := range filterAddrsQuery { + if strings.HasPrefix(filter, "!") { + protocolStringFromFilter := strings.TrimPrefix(filter, "!") + protocolFromFilter := multiaddr.ProtocolWithName(protocolStringFromFilter) + if containsProtocol(protocols, protocolFromFilter) { + includeAddr = false + break + } + } + } + + // If the address passed all negative filters, check positive filters + if includeAddr { + for _, filter := range filterAddrsQuery { + if !strings.HasPrefix(filter, "!") { + protocolFromFilter := multiaddr.ProtocolWithName(filter) + if containsProtocol(protocols, protocolFromFilter) { + filteredAddrs = append(filteredAddrs, addr) + break + } + } + } + } + } + return filteredAddrs +} + +func containsProtocol(protos []multiaddr.Protocol, proto multiaddr.Protocol) bool { + for _, p := range protos { + if p.Code == proto.Code { + return true + } + } + return false +} + +func applyProtocolFilter(peerProtocols []string, filterProtocols []string) bool { + if len(filterProtocols) == 0 { + // If no filter is passed, do not filter + return true + } + + for _, filter := range filterProtocols { + filterProtocol := strings.TrimPrefix(filter, "!") + + if filterProtocol == "unknown" && len(peerProtocols) == 0 { + return true + } + + for _, peerProtocol := range peerProtocols { + return peerProtocol == filterProtocol + } + } + return false +} diff --git a/routing/http/server/filters_test.go b/routing/http/server/filters_test.go new file mode 100644 index 000000000..85f16380c --- /dev/null +++ b/routing/http/server/filters_test.go @@ -0,0 +1,309 @@ +package server + +import ( + "testing" + + "github.com/ipfs/boxo/routing/http/types" + "github.com/libp2p/go-libp2p/core/peer" + "github.com/multiformats/go-multiaddr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestApplyAddrFilter(t *testing.T) { + // Create some test multiaddrs + addr1, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/tcp/4001/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt") + addr2, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/udp/4001/quic/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt") + addr3, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/tcp/4001/ws/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt") + addr4, _ := multiaddr.NewMultiaddr("/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt") + addr5, _ := multiaddr.NewMultiaddr("/ip4/102.101.1.1/udp/4001/quic-v1/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt") + addr6, _ := multiaddr.NewMultiaddr("/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/certhash/uEiD9f05PrY82lovP4gOFonmY7sO0E7_jyovt9p2LEcAS-Q/certhash/uEiBtGJsNz-PcywwXOVzEYeQQloQiHMqDqdj18t2Fe4GTLQ/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt") + addr7, _ := multiaddr.NewMultiaddr("/dns4/ny5.bootstrap.libp2p.io/tcp/443/wss/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt") + addr8, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/udp/4001/quic-v1/webtransport/certhash/uEiAMrMcVWFNiqtSeRXZTwHTac4p9WcGh5hg8kVBzTC1JTA/certhash/uEiA4dfvbbbnBIYalhp1OpW1Bk-nuWIKSy21ol6vPea67Cw/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt") + + addrs := []types.Multiaddr{ + {Multiaddr: addr1}, + {Multiaddr: addr2}, + {Multiaddr: addr3}, + {Multiaddr: addr4}, + {Multiaddr: addr5}, + {Multiaddr: addr6}, + {Multiaddr: addr7}, + {Multiaddr: addr8}, + } + + testCases := []struct { + name string + filterAddrs []string + expectedAddrs []types.Multiaddr + }{ + { + name: "No filter", + filterAddrs: []string{}, + expectedAddrs: addrs, + }, + { + name: "Filter TCP", + filterAddrs: []string{"tcp"}, + expectedAddrs: []types.Multiaddr{{Multiaddr: addr1}, {Multiaddr: addr3}, {Multiaddr: addr4}, {Multiaddr: addr7}}, + }, + { + name: "Filter UDP", + filterAddrs: []string{"udp"}, + expectedAddrs: []types.Multiaddr{{Multiaddr: addr2}, {Multiaddr: addr5}, {Multiaddr: addr6}, {Multiaddr: addr8}}, + }, + { + name: "Filter WebSocket", + filterAddrs: []string{"ws"}, + expectedAddrs: []types.Multiaddr{{Multiaddr: addr3}}, + }, + { + name: "Exclude TCP", + filterAddrs: []string{"!tcp"}, + expectedAddrs: []types.Multiaddr{}, + }, + { + name: "Include WebTransport and exclude p2p-circuit", + filterAddrs: []string{"webtransport", "!p2p-circuit"}, + expectedAddrs: []types.Multiaddr{{Multiaddr: addr8}}, + }, + { + name: "empty for unknown protocol nae", + filterAddrs: []string{"fakeproto"}, + expectedAddrs: []types.Multiaddr{}, + }, + { + name: "Include WebTransport but ignore unknown protocol name", + filterAddrs: []string{"webtransport", "fakeproto"}, + expectedAddrs: []types.Multiaddr{{Multiaddr: addr6}, {Multiaddr: addr8}}, + }, + { + name: "Multiple filters", + filterAddrs: []string{"tcp", "ws"}, + expectedAddrs: []types.Multiaddr{{Multiaddr: addr1}, {Multiaddr: addr3}, {Multiaddr: addr4}, {Multiaddr: addr7}}, + }, + { + name: "Multiple negative filters", + filterAddrs: []string{"!tcp", "!ws"}, + expectedAddrs: []types.Multiaddr{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := applyAddrFilter(addrs, tc.filterAddrs) + assert.Equal(t, len(tc.expectedAddrs), len(result), "Unexpected number of addresses after filtering") + + // Check that each expected address is in the result + for _, expectedAddr := range tc.expectedAddrs { + found := false + for _, resultAddr := range result { + if expectedAddr.Multiaddr.Equal(resultAddr.Multiaddr) { + found = true + break + } + } + assert.True(t, found, "Expected address not found in test %s result: %s", tc.name, expectedAddr.Multiaddr) + } + + // Check that each result address is in the expected list + for _, resultAddr := range result { + found := false + for _, expectedAddr := range tc.expectedAddrs { + if resultAddr.Multiaddr.Equal(expectedAddr.Multiaddr) { + found = true + break + } + } + assert.True(t, found, "Unexpected address found in test %s result: %s", tc.name, resultAddr.Multiaddr) + } + }) + } +} + +func TestApplyProtocolFilter(t *testing.T) { + testCases := []struct { + name string + peerProtocols []string + filterProtocols []string + expected bool + }{ + { + name: "No filter", + peerProtocols: []string{"transport-bitswap", "transport-ipfs-gateway-http"}, + filterProtocols: []string{}, + expected: true, + }, + { + name: "Single matching protocol", + peerProtocols: []string{"transport-bitswap", "transport-ipfs-gateway-http"}, + filterProtocols: []string{"transport-bitswap"}, + expected: true, + }, + { + name: "Single non-matching protocol", + peerProtocols: []string{"transport-bitswap", "transport-ipfs-gateway-http"}, + filterProtocols: []string{"transport-graphsync-filecoinv1"}, + expected: false, + }, + { + name: "Multiple protocols, one match", + peerProtocols: []string{"transport-bitswap", "transport-ipfs-gateway-http"}, + filterProtocols: []string{"transport-graphsync-filecoinv1", "transport-ipfs-gateway-http"}, + expected: true, + }, + { + name: "Negative filter, no match", + peerProtocols: []string{"transport-bitswap", "transport-ipfs-gateway-http"}, + filterProtocols: []string{"!transport-graphsync-filecoinv1"}, + expected: true, + }, + { + name: "Negative filter, with match", + peerProtocols: []string{"transport-bitswap", "transport-ipfs-gateway-http"}, + filterProtocols: []string{"!transport-ipfs-gateway-http"}, + expected: false, + }, + { + name: "Mixed positive and negative filters, no match", + peerProtocols: []string{"transport-bitswap", "transport-ipfs-gateway-http"}, + filterProtocols: []string{"transport-graphsync-filecoinv1", "!transport-ipfs-gateway-http"}, + expected: false, + }, + { + name: "Unknown protocol for empty peer protocols", + peerProtocols: []string{}, + filterProtocols: []string{"unknown"}, + expected: true, + }, + { + // TODO: Does this case make sense? + name: "Unknown protocol for non-empty peer protocols", + peerProtocols: []string{"transport-bitswap"}, + filterProtocols: []string{"unknown"}, + expected: false, + }, + { + name: "Case insensitive match", + peerProtocols: []string{"TRANSPORT-BITSWAP", "Transport-IPFS-Gateway-HTTP"}, + filterProtocols: []string{"transport-bitswap"}, + expected: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := applyProtocolFilter(tc.peerProtocols, tc.filterProtocols) + assert.Equal(t, tc.expected, result, "Unexpected result for test case: %s", tc.name) + }) + } +} + +func TestApplyFilters(t *testing.T) { + pid, err := peer.Decode("12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn") + require.NoError(t, err) + + tests := []struct { + name string + provider *types.PeerRecord + filterAddrs []string + filterProtocols []string + expected *types.PeerRecord + }{ + { + name: "No filters", + provider: &types.PeerRecord{ + ID: &pid, + Addrs: []types.Multiaddr{ + mustMultiaddr(t, "/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), + mustMultiaddr(t, "/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"), + }, + Protocols: []string{"transport-ipfs-gateway-http"}, + }, + filterAddrs: []string{}, + filterProtocols: []string{}, + expected: &types.PeerRecord{ + ID: &pid, + Addrs: []types.Multiaddr{ + mustMultiaddr(t, "/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), + mustMultiaddr(t, "/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"), + }, + Protocols: []string{"transport-ipfs-gateway-http"}, + }, + }, + { + name: "Protocol filter", + provider: &types.PeerRecord{ + ID: &pid, + Addrs: []types.Multiaddr{ + mustMultiaddr(t, "/ip4/127.0.0.1/tcp/4001"), + mustMultiaddr(t, "/ip4/127.0.0.1/udp/4001/quic-v1"), + mustMultiaddr(t, "/ip4/127.0.0.1/tcp/4001/ws"), + mustMultiaddr(t, "/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), + mustMultiaddr(t, "/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), + mustMultiaddr(t, "/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"), + }, + Protocols: []string{"transport-ipfs-gateway-http"}, + }, + filterAddrs: []string{}, + filterProtocols: []string{"transport-ipfs-gateway-http", "transport-bitswap"}, + expected: &types.PeerRecord{ + ID: &pid, + Addrs: []types.Multiaddr{ + mustMultiaddr(t, "/ip4/127.0.0.1/tcp/4001"), + mustMultiaddr(t, "/ip4/127.0.0.1/udp/4001/quic-v1"), + mustMultiaddr(t, "/ip4/127.0.0.1/tcp/4001/ws"), + mustMultiaddr(t, "/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), + mustMultiaddr(t, "/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), + mustMultiaddr(t, "/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"), + }, + Protocols: []string{"transport-ipfs-gateway-http"}, + }, + }, + { + name: "Address filter", + provider: &types.PeerRecord{ + ID: &pid, + Addrs: []types.Multiaddr{ + mustMultiaddr(t, "/ip4/127.0.0.1/tcp/4001"), + mustMultiaddr(t, "/ip4/127.0.0.1/udp/4001/quic-v1"), + mustMultiaddr(t, "/ip4/127.0.0.1/tcp/4001/ws"), + mustMultiaddr(t, "/ip4/127.0.0.1/udp/4001/webrtc-direct/certhash/uEiCZqN653gMqxrWNmYuNg7Emwb-wvtsuzGE3XD6rypViZA"), + mustMultiaddr(t, "/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), + mustMultiaddr(t, "/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), + mustMultiaddr(t, "/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"), + }, + Protocols: []string{"transport-ipfs-gateway-http"}, + }, + filterAddrs: []string{"webtransport", "wss", "webrtc-direct", "!p2p-circuit"}, + filterProtocols: []string{"transport-ipfs-gateway-http", "transport-bitswap"}, + expected: &types.PeerRecord{ + ID: &pid, + Addrs: []types.Multiaddr{ + mustMultiaddr(t, "/ip4/127.0.0.1/tcp/4001"), + mustMultiaddr(t, "/ip4/127.0.0.1/udp/4001/quic-v1"), + mustMultiaddr(t, "/ip4/127.0.0.1/tcp/4001/ws"), + mustMultiaddr(t, "/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), + mustMultiaddr(t, "/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), + mustMultiaddr(t, "/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"), + }, + Protocols: []string{"transport-ipfs-gateway-http"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := applyFilters(tt.provider, tt.filterAddrs, tt.filterProtocols) + assert.Equal(t, tt.expected, result) + }) + } +} + +func mustMultiaddr(t *testing.T, s string) types.Multiaddr { + addr, err := multiaddr.NewMultiaddr(s) + if err != nil { + t.Fatalf("Failed to create multiaddr: %v", err) + } + return types.Multiaddr{Multiaddr: addr} +} diff --git a/routing/http/server/server.go b/routing/http/server/server.go index 1e1a84770..20d7e6fec 100644 --- a/routing/http/server/server.go +++ b/routing/http/server/server.go @@ -194,6 +194,11 @@ func (s *server) findProviders(w http.ResponseWriter, httpReq *http.Request) { return } + // Parse query parameters + query := httpReq.URL.Query() + filterAddrs := parseFilter(query.Get("filter-addrs")) + filterProtocols := parseFilter(query.Get("filter-protocols")) + mediaType, err := s.detectResponseType(httpReq) if err != nil { writeErr(w, "FindProviders", http.StatusBadRequest, err) @@ -201,7 +206,7 @@ func (s *server) findProviders(w http.ResponseWriter, httpReq *http.Request) { } var ( - handlerFunc func(w http.ResponseWriter, provIter iter.ResultIter[types.Record]) + handlerFunc func(w http.ResponseWriter, provIter iter.ResultIter[types.Record], filterAddrs, filterProtocols []string) recordsLimit int ) @@ -224,10 +229,10 @@ func (s *server) findProviders(w http.ResponseWriter, httpReq *http.Request) { } } - handlerFunc(w, provIter) + handlerFunc(w, provIter, filterAddrs, filterProtocols) } -func (s *server) findProvidersJSON(w http.ResponseWriter, provIter iter.ResultIter[types.Record]) { +func (s *server) findProvidersJSON(w http.ResponseWriter, provIter iter.ResultIter[types.Record], filterAddrs, filterProtocols []string) { defer provIter.Close() providers, err := iter.ReadAllResults(provIter) @@ -236,13 +241,78 @@ func (s *server) findProvidersJSON(w http.ResponseWriter, provIter iter.ResultIt return } + filteredProviders := filterProviders(providers, filterAddrs, filterProtocols) + writeJSONResult(w, "FindProviders", jsontypes.ProvidersResponse{ - Providers: providers, + Providers: filteredProviders, }) } +func (s *server) findProvidersNDJSON(w http.ResponseWriter, provIter iter.ResultIter[types.Record], filterAddrs, filterProtocols []string) { + defer provIter.Close() + + w.Header().Set("Content-Type", mediaTypeNDJSON) + w.Header().Add("Vary", "Accept") + w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat)) + + hasResults := false + for provIter.Next() { + res := provIter.Val() + if res.Err != nil { + logger.Errorw("ndjson iterator error", "Error", res.Err) + return + } + + // handle filtering per record as we iterate + if len(filterAddrs) > 0 || len(filterProtocols) > 0 { + switch v := res.Val.(type) { + case *types.PeerRecord: + record := applyFilters(v, filterAddrs, filterProtocols) + if record == nil { + // if the record is nil, we skip it + continue + } + res.Val = record + default: + logger.Warn("unexpected type for res.Val, expected types.PeerRecord") + continue + } + } + + // don't use an encoder because we can't easily differentiate writer errors from encoding errors + b, err := drjson.MarshalJSONBytes(res.Val) + if err != nil { + logger.Errorw("ndjson marshal error", "Error", err) + return + } + + if !hasResults { + hasResults = true + // There's results, cache useful result for longer + setCacheControl(w, maxAgeWithResults, maxStale) + } + + _, err = w.Write(b) + if err != nil { + logger.Warn("ndjson write error", "Error", err) + return + } + + _, err = w.Write([]byte{'\n'}) + if err != nil { + logger.Warn("ndjson write error", "Error", err) + return + } -func (s *server) findProvidersNDJSON(w http.ResponseWriter, provIter iter.ResultIter[types.Record]) { - writeResultsIterNDJSON(w, provIter) + if f, ok := w.(http.Flusher); ok { + f.Flush() + } + } + + if !hasResults { + // There weren't results, cache for shorter and send 404 + setCacheControl(w, maxAgeWithoutResults, maxStale) + w.WriteHeader(http.StatusNotFound) + } } func (s *server) findPeers(w http.ResponseWriter, r *http.Request) { @@ -572,7 +642,7 @@ func logErr(method, msg string, err error) { logger.Infow(msg, "Method", method, "Error", err) } -func writeResultsIterNDJSON[T any](w http.ResponseWriter, resultIter iter.ResultIter[T]) { +func writeResultsIterNDJSON[T types.Record](w http.ResponseWriter, resultIter iter.ResultIter[T]) { defer resultIter.Close() w.Header().Set("Content-Type", mediaTypeNDJSON) diff --git a/routing/http/server/server_test.go b/routing/http/server/server_test.go index 3f4e7906a..5bdfedb51 100644 --- a/routing/http/server/server_test.go +++ b/routing/http/server/server_test.go @@ -22,6 +22,7 @@ import ( "github.com/libp2p/go-libp2p/core/peer" "github.com/libp2p/go-libp2p/core/routing" b58 "github.com/mr-tron/base58/base58" + "github.com/multiformats/go-multiaddr" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -93,6 +94,13 @@ func TestProviders(t *testing.T) { pid2Str := "12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz" cidStr := "bafkreifjjcie6lypi6ny7amxnfftagclbuxndqonfipmb64f2km2devei4" + addr1, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/tcp/4001") + addr2, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/udp/4001/quic-v1") + addr3, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/tcp/4001/ws") + addr4, _ := multiaddr.NewMultiaddr("/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit") + addr5, _ := multiaddr.NewMultiaddr("/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit") + addr6, _ := multiaddr.NewMultiaddr("/ip4/8.8.8.8/udp/4001/quic-v1/webtransport") + pid, err := peer.Decode(pidStr) require.NoError(t, err) pid2, err := peer.Decode(pid2Str) @@ -101,7 +109,7 @@ func TestProviders(t *testing.T) { cid, err := cid.Decode(cidStr) require.NoError(t, err) - runTest := func(t *testing.T, contentType string, empty bool, expectedStream bool, expectedBody string) { + runTest := func(t *testing.T, contentType string, filterAddrs, filterProtocols string, empty bool, expectedStream bool, expectedBody string) { t.Parallel() var results *iter.SliceIter[iter.Result[types.Record]] @@ -114,16 +122,22 @@ func TestProviders(t *testing.T) { Schema: types.SchemaPeer, ID: &pid, Protocols: []string{"transport-bitswap"}, + Addrs: []types.Multiaddr{ + {Multiaddr: addr1}, + {Multiaddr: addr2}, + {Multiaddr: addr3}, + {Multiaddr: addr4}, + {Multiaddr: addr5}, + {Multiaddr: addr6}, + }, + }}, + {Val: &types.PeerRecord{ + Schema: types.SchemaPeer, + ID: &pid2, + Protocols: []string{"transport-ipfs-gateway-http"}, Addrs: []types.Multiaddr{}, }}, - //lint:ignore SA1019 // ignore staticcheck - {Val: &types.BitswapRecord{ - //lint:ignore SA1019 // ignore staticcheck - Schema: types.SchemaBitswap, - ID: &pid2, - Protocol: "transport-bitswap", - Addrs: []types.Multiaddr{}, - }}}, + }, ) } @@ -136,7 +150,7 @@ func TestProviders(t *testing.T) { limit = DefaultStreamingRecordsLimit } router.On("FindProviders", mock.Anything, cid, limit).Return(results, nil) - urlStr := serverAddr + "/routing/v1/providers/" + cidStr + urlStr := serverAddr + "/routing/v1/providers/" + cidStr + "?filter-addrs=" + filterAddrs + "&filter-protocols=" + filterProtocols req, err := http.NewRequest(http.MethodGet, urlStr, nil) require.NoError(t, err) @@ -174,29 +188,51 @@ func TestProviders(t *testing.T) { } t.Run("JSON Response", func(t *testing.T) { - runTest(t, mediaTypeJSON, false, false, `{"Providers":[{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"},{"Schema":"bitswap","Protocol":"transport-bitswap","ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz"}]}`) + runTest(t, mediaTypeJSON, "", "", false, false, `{"Providers":[{"Addrs":["/ip4/127.0.0.1/tcp/4001","/ip4/127.0.0.1/udp/4001/quic-v1","/ip4/127.0.0.1/tcp/4001/ws","/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"},{"Addrs":["/ip4/127.0.0.1/tcp/4001","/ip4/127.0.0.1/udp/4001/quic-v1","/ip4/127.0.0.1/tcp/4001/ws"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz","Protocols":["transport-ipfs-gateway-http"],"Schema":"peer"}]}`) + }) + + t.Run("JSON Response with addr filtering including unknown", func(t *testing.T) { + runTest(t, mediaTypeJSON, "webtransport,!p2p-circuit,unknown", "", false, false, `{"Providers":[{"Addrs":["/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"},{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz","Protocols":["transport-ipfs-gateway-http"],"Schema":"peer"}]}`) + }) + + t.Run("JSON Response with addr filtering", func(t *testing.T) { + runTest(t, mediaTypeJSON, "webtransport,!p2p-circuit", "", false, false, `{"Providers":[{"Addrs":["/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"}]}`) + }) + + t.Run("JSON Response with protocol and addr filtering", func(t *testing.T) { + runTest(t, mediaTypeJSON, "quic-v1", "transport-bitswap", false, false, + `{"Providers":[{"Addrs":["/ip4/127.0.0.1/udp/4001/quic-v1","/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"}]}`) + }) + + t.Run("JSON Response with protocol filtering", func(t *testing.T) { + runTest(t, mediaTypeJSON, "", "transport-ipfs-gateway-http", false, false, + `{"Providers":[{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz","Protocols":["transport-ipfs-gateway-http"],"Schema":"peer"}]}`) }) t.Run("Empty JSON Response", func(t *testing.T) { - runTest(t, mediaTypeJSON, true, false, `{"Providers":null}`) + runTest(t, mediaTypeJSON, "", "", true, false, `{"Providers":null}`) }) t.Run("Wildcard Accept header defaults to JSON Response", func(t *testing.T) { accept := "text/html,*/*" - runTest(t, accept, true, false, `{"Providers":null}`) + runTest(t, accept, "", "", true, false, `{"Providers":null}`) }) t.Run("Missing Accept header defaults to JSON Response", func(t *testing.T) { accept := "" - runTest(t, accept, true, false, `{"Providers":null}`) + runTest(t, accept, "", "", true, false, `{"Providers":null}`) }) t.Run("NDJSON Response", func(t *testing.T) { - runTest(t, mediaTypeNDJSON, false, true, `{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"}`+"\n"+`{"Schema":"bitswap","Protocol":"transport-bitswap","ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz"}`+"\n") + runTest(t, mediaTypeNDJSON, "", "", false, true, `{"Addrs":["/ip4/127.0.0.1/tcp/4001","/ip4/127.0.0.1/udp/4001/quic-v1","/ip4/127.0.0.1/tcp/4001/ws","/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"}`+"\n"+`{"Schema":"bitswap","Protocol":"transport-bitswap","ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz"}`+"\n") + }) + + t.Run("NDJSON Response with addr filtering", func(t *testing.T) { + runTest(t, mediaTypeNDJSON, "webtransport,!p2p-circuit,unknown", "", false, true, `{"Addrs":["/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"}`+"\n"+`{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz","Protocols":["transport-ipfs-gateway-http"],"Schema":"peer"}`+"\n") }) t.Run("Empty NDJSON Response", func(t *testing.T) { - runTest(t, mediaTypeNDJSON, true, true, "") + runTest(t, mediaTypeNDJSON, "", "", true, true, "") }) t.Run("404 when router returns routing.ErrNotFound", func(t *testing.T) { From 30b853cbda6564c59d944664a7c43018d9f8ed62 Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Thu, 12 Sep 2024 18:03:52 +0200 Subject: [PATCH 02/20] test: improve tests --- routing/http/server/filters.go | 4 +--- routing/http/server/filters_test.go | 23 ++++++++++++++++++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go index e9411a00a..a508610ad 100644 --- a/routing/http/server/filters.go +++ b/routing/http/server/filters.go @@ -128,9 +128,7 @@ func applyProtocolFilter(peerProtocols []string, filterProtocols []string) bool return true } - for _, filter := range filterProtocols { - filterProtocol := strings.TrimPrefix(filter, "!") - + for _, filterProtocol := range filterProtocols { if filterProtocol == "unknown" && len(peerProtocols) == 0 { return true } diff --git a/routing/http/server/filters_test.go b/routing/http/server/filters_test.go index 85f16380c..093b716c6 100644 --- a/routing/http/server/filters_test.go +++ b/routing/http/server/filters_test.go @@ -280,16 +280,29 @@ func TestApplyFilters(t *testing.T) { expected: &types.PeerRecord{ ID: &pid, Addrs: []types.Multiaddr{ - mustMultiaddr(t, "/ip4/127.0.0.1/tcp/4001"), - mustMultiaddr(t, "/ip4/127.0.0.1/udp/4001/quic-v1"), - mustMultiaddr(t, "/ip4/127.0.0.1/tcp/4001/ws"), - mustMultiaddr(t, "/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), - mustMultiaddr(t, "/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"), + mustMultiaddr(t, "/ip4/127.0.0.1/udp/4001/webrtc-direct/certhash/uEiCZqN653gMqxrWNmYuNg7Emwb-wvtsuzGE3XD6rypViZA"), mustMultiaddr(t, "/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"), }, Protocols: []string{"transport-ipfs-gateway-http"}, }, }, + { + name: "Unknown protocol filter", + provider: &types.PeerRecord{ + ID: &pid, + Addrs: []types.Multiaddr{ + mustMultiaddr(t, "/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"), + }, + }, + filterAddrs: []string{}, + filterProtocols: []string{"unknown"}, + expected: &types.PeerRecord{ + ID: &pid, + Addrs: []types.Multiaddr{ + mustMultiaddr(t, "/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"), + }, + }, + }, } for _, tt := range tests { From a920f234deefc3b1e854d24ec589f8421fe29ebf Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Fri, 13 Sep 2024 17:11:09 +0200 Subject: [PATCH 03/20] fix: remove negative filter tests and fix filter --- routing/http/server/filters.go | 4 +++- routing/http/server/filters_test.go | 18 ------------------ 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go index a508610ad..b7329d204 100644 --- a/routing/http/server/filters.go +++ b/routing/http/server/filters.go @@ -134,7 +134,9 @@ func applyProtocolFilter(peerProtocols []string, filterProtocols []string) bool } for _, peerProtocol := range peerProtocols { - return peerProtocol == filterProtocol + if peerProtocol == filterProtocol { + return true + } } } return false diff --git a/routing/http/server/filters_test.go b/routing/http/server/filters_test.go index 093b716c6..30e1e6b39 100644 --- a/routing/http/server/filters_test.go +++ b/routing/http/server/filters_test.go @@ -152,24 +152,6 @@ func TestApplyProtocolFilter(t *testing.T) { filterProtocols: []string{"transport-graphsync-filecoinv1", "transport-ipfs-gateway-http"}, expected: true, }, - { - name: "Negative filter, no match", - peerProtocols: []string{"transport-bitswap", "transport-ipfs-gateway-http"}, - filterProtocols: []string{"!transport-graphsync-filecoinv1"}, - expected: true, - }, - { - name: "Negative filter, with match", - peerProtocols: []string{"transport-bitswap", "transport-ipfs-gateway-http"}, - filterProtocols: []string{"!transport-ipfs-gateway-http"}, - expected: false, - }, - { - name: "Mixed positive and negative filters, no match", - peerProtocols: []string{"transport-bitswap", "transport-ipfs-gateway-http"}, - filterProtocols: []string{"transport-graphsync-filecoinv1", "!transport-ipfs-gateway-http"}, - expected: false, - }, { name: "Unknown protocol for empty peer protocols", peerProtocols: []string{}, From 3dc1a58c3b885e7a10d079c1db989b1cfd536cae Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Tue, 17 Sep 2024 14:18:43 +0200 Subject: [PATCH 04/20] chore: add query params conditionally --- routing/http/server/filters_test.go | 1 - routing/http/server/server_test.go | 13 ++++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/routing/http/server/filters_test.go b/routing/http/server/filters_test.go index 30e1e6b39..3fcda11a5 100644 --- a/routing/http/server/filters_test.go +++ b/routing/http/server/filters_test.go @@ -159,7 +159,6 @@ func TestApplyProtocolFilter(t *testing.T) { expected: true, }, { - // TODO: Does this case make sense? name: "Unknown protocol for non-empty peer protocols", peerProtocols: []string{"transport-bitswap"}, filterProtocols: []string{"unknown"}, diff --git a/routing/http/server/server_test.go b/routing/http/server/server_test.go index 5bdfedb51..7d1aaf020 100644 --- a/routing/http/server/server_test.go +++ b/routing/http/server/server_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "crypto/rand" + "fmt" "io" "net/http" "net/http/httptest" @@ -150,7 +151,17 @@ func TestProviders(t *testing.T) { limit = DefaultStreamingRecordsLimit } router.On("FindProviders", mock.Anything, cid, limit).Return(results, nil) - urlStr := serverAddr + "/routing/v1/providers/" + cidStr + "?filter-addrs=" + filterAddrs + "&filter-protocols=" + filterProtocols + + urlStr := fmt.Sprintf("%s/routing/v1/providers/%s", serverAddr, cidStr) + if filterAddrs != "" || filterProtocols != "" { + urlStr += "?" + if filterAddrs != "" { + urlStr = fmt.Sprintf("%s&filter-addrs=%s", urlStr, filterAddrs) + } + if filterProtocols != "" { + urlStr = fmt.Sprintf("%s&filter-protocols=%s", urlStr, filterProtocols) + } + } req, err := http.NewRequest(http.MethodGet, urlStr, nil) require.NoError(t, err) From 496805db49ae7c973cdf0bb8f9afda41c8aab854 Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Tue, 17 Sep 2024 17:12:05 +0200 Subject: [PATCH 05/20] fix: tests --- routing/http/server/server_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/routing/http/server/server_test.go b/routing/http/server/server_test.go index 7d1aaf020..c0698a51f 100644 --- a/routing/http/server/server_test.go +++ b/routing/http/server/server_test.go @@ -199,7 +199,7 @@ func TestProviders(t *testing.T) { } t.Run("JSON Response", func(t *testing.T) { - runTest(t, mediaTypeJSON, "", "", false, false, `{"Providers":[{"Addrs":["/ip4/127.0.0.1/tcp/4001","/ip4/127.0.0.1/udp/4001/quic-v1","/ip4/127.0.0.1/tcp/4001/ws","/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"},{"Addrs":["/ip4/127.0.0.1/tcp/4001","/ip4/127.0.0.1/udp/4001/quic-v1","/ip4/127.0.0.1/tcp/4001/ws"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz","Protocols":["transport-ipfs-gateway-http"],"Schema":"peer"}]}`) + runTest(t, mediaTypeJSON, "", "", false, false, `{"Providers":[{"Addrs":["/ip4/127.0.0.1/tcp/4001","/ip4/127.0.0.1/udp/4001/quic-v1","/ip4/127.0.0.1/tcp/4001/ws","/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"},{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz","Protocols":["transport-ipfs-gateway-http"],"Schema":"peer"}]}`) }) t.Run("JSON Response with addr filtering including unknown", func(t *testing.T) { @@ -235,7 +235,11 @@ func TestProviders(t *testing.T) { }) t.Run("NDJSON Response", func(t *testing.T) { - runTest(t, mediaTypeNDJSON, "", "", false, true, `{"Addrs":["/ip4/127.0.0.1/tcp/4001","/ip4/127.0.0.1/udp/4001/quic-v1","/ip4/127.0.0.1/tcp/4001/ws","/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"}`+"\n"+`{"Schema":"bitswap","Protocol":"transport-bitswap","ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz"}`+"\n") + runTest(t, mediaTypeNDJSON, "", "", false, true, `{"Addrs":["/ip4/127.0.0.1/tcp/4001","/ip4/127.0.0.1/udp/4001/quic-v1","/ip4/127.0.0.1/tcp/4001/ws","/ip4/102.101.1.1/tcp/4001/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit","/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"}`+"\n"+`{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz","Protocols":["transport-ipfs-gateway-http"],"Schema":"peer"}`+"\n") + }) + + t.Run("NDJSON Response with addr filtering", func(t *testing.T) { + runTest(t, mediaTypeNDJSON, "webtransport,!p2p-circuit,unknown", "", false, true, `{"Addrs":["/ip4/8.8.8.8/udp/4001/quic-v1/webtransport"],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"}`+"\n"+`{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz","Protocols":["transport-ipfs-gateway-http"],"Schema":"peer"}`+"\n") }) t.Run("NDJSON Response with addr filtering", func(t *testing.T) { From a4e9def69297856508c4e4d39d095ca0df2733d4 Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Tue, 17 Sep 2024 17:57:19 +0200 Subject: [PATCH 06/20] chore: update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7081f01cd..e8eb4cca0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ The following emojis are used to highlight certain changes: ### Added +- Added address and protocol filtering to the delegated routing server [IPIP-484](https://github.com/ipfs/specs/pull/484) + ### Changed ### Removed From da4a194685b9c908d5215b14f531b0e36656974a Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Tue, 17 Sep 2024 18:05:40 +0200 Subject: [PATCH 07/20] fix: ensure protocol filter is case-insensitive --- routing/http/server/filters.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go index b7329d204..98e2d5491 100644 --- a/routing/http/server/filters.go +++ b/routing/http/server/filters.go @@ -134,9 +134,10 @@ func applyProtocolFilter(peerProtocols []string, filterProtocols []string) bool } for _, peerProtocol := range peerProtocols { - if peerProtocol == filterProtocol { + if strings.EqualFold(peerProtocol, filterProtocol) { return true } + } } return false From 3a6de236411139635f6241480c7f720d688ad4ff Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Tue, 24 Sep 2024 12:56:01 +0200 Subject: [PATCH 08/20] feat: add generic filter iterator and use for fitlering --- routing/http/server/filters.go | 72 ++++++++++++++++++++++--- routing/http/server/server.go | 73 ++------------------------ routing/http/types/iter/filter.go | 43 +++++++++++++++ routing/http/types/iter/filter_test.go | 41 +++++++++++++++ routing/http/types/record_peer.go | 9 ++++ 5 files changed, 162 insertions(+), 76 deletions(-) create mode 100644 routing/http/types/iter/filter.go create mode 100644 routing/http/types/iter/filter_test.go diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go index 98e2d5491..73859a7f2 100644 --- a/routing/http/server/filters.go +++ b/routing/http/server/filters.go @@ -1,11 +1,13 @@ package server import ( + "errors" "reflect" "slices" "strings" "github.com/ipfs/boxo/routing/http/types" + "github.com/ipfs/boxo/routing/http/types/iter" "github.com/multiformats/go-multiaddr" ) @@ -18,18 +20,68 @@ func parseFilter(param string) []string { return strings.Split(strings.ToLower(param), ",") } -func filterProviders(providers []types.Record, filterAddrs, filterProtocols []string) []types.Record { +// applyFiltersToIter applies the filters to the given iterator and returns a new iterator. +func applyFiltersToIter(recordsIter iter.ResultIter[types.Record], filterAddrs, filterProtocols []string) iter.ResultIter[types.Record] { + mappedIter := iter.Map(recordsIter, func(v iter.Result[types.Record]) iter.Result[types.Record] { + if v.Err != nil || v.Val == nil { + return v + } + + switch v.Val.GetSchema() { + case types.SchemaPeer: + record, ok := v.Val.(*types.PeerRecord) + if !ok { + logger.Errorw("problem casting find providers record", "Schema", v.Val.GetSchema(), "Type", reflect.TypeOf(v).String()) + // TODO: Do we want to let failed type assertions to pass through? + return v + } + + record = applyFilters(record, filterAddrs, filterProtocols) + if record == nil { + return iter.Result[types.Record]{Err: errors.New("record is nil")} + } + v.Val = record + + //lint:ignore SA1019 // ignore staticcheck + case types.SchemaBitswap: + //lint:ignore SA1019 // ignore staticcheck + record, ok := v.Val.(*types.BitswapRecord) + if !ok { + logger.Errorw("problem casting find providers record", "Schema", v.Val.GetSchema(), "Type", reflect.TypeOf(v).String()) + // TODO: Do we want to let failed type assertions to pass through? + return v + } + peerRecord := types.FromBitswapRecord(record) + peerRecord = applyFilters(peerRecord, filterAddrs, filterProtocols) + if peerRecord == nil { + return iter.Result[types.Record]{Err: errors.New("record is nil")} + } + v.Val = peerRecord + } + return v + }) + + // filter out nil results and errors + filteredIter := iter.Filter(mappedIter, func(v iter.Result[types.Record]) bool { + return v.Err == nil && v.Val != nil + }) + + return filteredIter +} + +func filterRecords(records []types.Record, filterAddrs, filterProtocols []string) []types.Record { if len(filterAddrs) == 0 && len(filterProtocols) == 0 { - return providers + return records } - filtered := make([]types.Record, 0, len(providers)) + filtered := make([]types.Record, 0, len(records)) - for _, provider := range providers { - if schema := provider.GetSchema(); schema == types.SchemaPeer { - peer, ok := provider.(*types.PeerRecord) + for _, record := range records { + // TODO: Handle SchemaBitswap + if schema := record.GetSchema(); schema == types.SchemaPeer { + peer, ok := record.(*types.PeerRecord) if !ok { - logger.Errorw("problem casting find providers result", "Schema", provider.GetSchema(), "Type", reflect.TypeOf(provider).String()) + logger.Errorw("problem casting find providers result", "Schema", record.GetSchema(), "Type", reflect.TypeOf(record).String()) // if the type assertion fails, we exlude record from results continue } @@ -42,7 +94,7 @@ func filterProviders(providers []types.Record, filterAddrs, filterProtocols []st } else { // Will we ever encounter the SchemaBitswap type? Evidence seems to suggest that no longer - logger.Errorw("encountered unknown provider schema", "Schema", provider.GetSchema(), "Type", reflect.TypeOf(provider).String()) + logger.Errorw("encountered unknown provider schema", "Schema", record.GetSchema(), "Type", reflect.TypeOf(record).String()) } } return filtered @@ -51,6 +103,10 @@ func filterProviders(providers []types.Record, filterAddrs, filterProtocols []st // Applies the filters. Returns nil if the provider does not pass the protocols filter // The address filter is more complicated because it potentially modifies the Addrs slice. func applyFilters(provider *types.PeerRecord, filterAddrs, filterProtocols []string) *types.PeerRecord { + if len(filterAddrs) == 0 && len(filterProtocols) == 0 { + return provider + } + if !applyProtocolFilter(provider.Protocols, filterProtocols) { // If the provider doesn't match any of the passed protocols, the provider is omitted from the response. return nil diff --git a/routing/http/server/server.go b/routing/http/server/server.go index 20d7e6fec..562220e6f 100644 --- a/routing/http/server/server.go +++ b/routing/http/server/server.go @@ -235,84 +235,21 @@ func (s *server) findProviders(w http.ResponseWriter, httpReq *http.Request) { func (s *server) findProvidersJSON(w http.ResponseWriter, provIter iter.ResultIter[types.Record], filterAddrs, filterProtocols []string) { defer provIter.Close() - providers, err := iter.ReadAllResults(provIter) + filteredIter := applyFiltersToIter(provIter, filterAddrs, filterProtocols) + providers, err := iter.ReadAllResults(filteredIter) if err != nil { writeErr(w, "FindProviders", http.StatusInternalServerError, fmt.Errorf("delegate error: %w", err)) return } - filteredProviders := filterProviders(providers, filterAddrs, filterProtocols) - writeJSONResult(w, "FindProviders", jsontypes.ProvidersResponse{ - Providers: filteredProviders, + Providers: providers, }) } func (s *server) findProvidersNDJSON(w http.ResponseWriter, provIter iter.ResultIter[types.Record], filterAddrs, filterProtocols []string) { - defer provIter.Close() - - w.Header().Set("Content-Type", mediaTypeNDJSON) - w.Header().Add("Vary", "Accept") - w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat)) - - hasResults := false - for provIter.Next() { - res := provIter.Val() - if res.Err != nil { - logger.Errorw("ndjson iterator error", "Error", res.Err) - return - } - - // handle filtering per record as we iterate - if len(filterAddrs) > 0 || len(filterProtocols) > 0 { - switch v := res.Val.(type) { - case *types.PeerRecord: - record := applyFilters(v, filterAddrs, filterProtocols) - if record == nil { - // if the record is nil, we skip it - continue - } - res.Val = record - default: - logger.Warn("unexpected type for res.Val, expected types.PeerRecord") - continue - } - } - - // don't use an encoder because we can't easily differentiate writer errors from encoding errors - b, err := drjson.MarshalJSONBytes(res.Val) - if err != nil { - logger.Errorw("ndjson marshal error", "Error", err) - return - } - - if !hasResults { - hasResults = true - // There's results, cache useful result for longer - setCacheControl(w, maxAgeWithResults, maxStale) - } + filteredIter := applyFiltersToIter(provIter, filterAddrs, filterProtocols) - _, err = w.Write(b) - if err != nil { - logger.Warn("ndjson write error", "Error", err) - return - } - - _, err = w.Write([]byte{'\n'}) - if err != nil { - logger.Warn("ndjson write error", "Error", err) - return - } - - if f, ok := w.(http.Flusher); ok { - f.Flush() - } - } - - if !hasResults { - // There weren't results, cache for shorter and send 404 - setCacheControl(w, maxAgeWithoutResults, maxStale) - w.WriteHeader(http.StatusNotFound) - } + writeResultsIterNDJSON(w, filteredIter) } func (s *server) findPeers(w http.ResponseWriter, r *http.Request) { diff --git a/routing/http/types/iter/filter.go b/routing/http/types/iter/filter.go new file mode 100644 index 000000000..628997811 --- /dev/null +++ b/routing/http/types/iter/filter.go @@ -0,0 +1,43 @@ +package iter + +// Filter returns an iterator that filters out values that don't satisfy the predicate f. +func Filter[T any](iter Iter[T], f func(t T) bool) *FilterIter[T] { + return &FilterIter[T]{iter: iter, f: f} +} + +type FilterIter[T any] struct { + iter Iter[T] + f func(T) bool + + done bool + val T +} + +func (f *FilterIter[T]) Next() bool { + if f.done { + return false + } + + ok := f.iter.Next() + f.done = !ok + + if f.done { + return false + } + + f.val = f.iter.Val() + + if f.f(f.val) { + return true + } + + return f.Next() +} + +func (f *FilterIter[T]) Val() T { + return f.val +} + +func (f *FilterIter[T]) Close() error { + return f.iter.Close() +} diff --git a/routing/http/types/iter/filter_test.go b/routing/http/types/iter/filter_test.go new file mode 100644 index 000000000..6d170285e --- /dev/null +++ b/routing/http/types/iter/filter_test.go @@ -0,0 +1,41 @@ +package iter + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFilter(t *testing.T) { + for _, c := range []struct { + input Iter[int] + f func(int) bool + expResults []int + }{ + { + input: FromSlice([]int{1, 2, 3, 4}), + f: func(i int) bool { return i%2 == 0 }, + expResults: []int{2, 4}, + }, + { + input: FromSlice([]int{}), + f: func(i int) bool { return i%2 == 0 }, + expResults: nil, + }, + { + input: FromSlice([]int{1, 3, 5, 100}), + f: func(i int) bool { return i > 2 }, + expResults: []int{3, 5, 100}, + }, + } { + t.Run(fmt.Sprintf("%v", c.input), func(t *testing.T) { + iter := Filter(c.input, c.f) + var res []int + for iter.Next() { + res = append(res, iter.Val()) + } + assert.Equal(t, c.expResults, res) + }) + } +} diff --git a/routing/http/types/record_peer.go b/routing/http/types/record_peer.go index 76bd810e0..f0d2cab60 100644 --- a/routing/http/types/record_peer.go +++ b/routing/http/types/record_peer.go @@ -79,3 +79,12 @@ func (pr PeerRecord) MarshalJSON() ([]byte, error) { return drjson.MarshalJSONBytes(m) } + +func FromBitswapRecord(br *BitswapRecord) *PeerRecord { + return &PeerRecord{ + Schema: SchemaPeer, + ID: br.ID, + Addrs: br.Addrs, + Protocols: []string{br.Protocol}, + } +} From b9958d0e72fcad567a653624acf6594ddb922c5d Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Tue, 24 Sep 2024 14:44:10 +0200 Subject: [PATCH 09/20] feat: proto & addr filter in peer routing endpoint --- routing/http/server/server.go | 38 +++++++++++++++++++++++----- routing/http/types/json/responses.go | 2 +- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/routing/http/server/server.go b/routing/http/server/server.go index 562220e6f..98d902396 100644 --- a/routing/http/server/server.go +++ b/routing/http/server/server.go @@ -284,6 +284,10 @@ func (s *server) findPeers(w http.ResponseWriter, r *http.Request) { return } + query := r.URL.Query() + filterAddrs := parseFilter(query.Get("filter-addrs")) + filterProtocols := parseFilter(query.Get("filter-protocols")) + mediaType, err := s.detectResponseType(r) if err != nil { writeErr(w, "FindPeers", http.StatusBadRequest, err) @@ -291,7 +295,7 @@ func (s *server) findPeers(w http.ResponseWriter, r *http.Request) { } var ( - handlerFunc func(w http.ResponseWriter, provIter iter.ResultIter[*types.PeerRecord]) + handlerFunc func(w http.ResponseWriter, provIter iter.ResultIter[*types.PeerRecord], filterAddrs, filterProtocols []string) recordsLimit int ) @@ -314,7 +318,7 @@ func (s *server) findPeers(w http.ResponseWriter, r *http.Request) { } } - handlerFunc(w, provIter) + handlerFunc(w, provIter, filterAddrs, filterProtocols) } func (s *server) provide(w http.ResponseWriter, httpReq *http.Request) { @@ -376,10 +380,21 @@ func (s *server) provide(w http.ResponseWriter, httpReq *http.Request) { writeJSONResult(w, "Provide", resp) } -func (s *server) findPeersJSON(w http.ResponseWriter, peersIter iter.ResultIter[*types.PeerRecord]) { +func (s *server) findPeersJSON(w http.ResponseWriter, peersIter iter.ResultIter[*types.PeerRecord], filterAddrs, filterProtocols []string) { defer peersIter.Close() - peers, err := iter.ReadAllResults(peersIter) + // Convert PeerRecord to Record so that we can reuse the filtering logic from findProviders + mappedIter := iter.Map(peersIter, func(v iter.Result[*types.PeerRecord]) iter.Result[types.Record] { + if v.Err != nil || v.Val == nil { + return iter.Result[types.Record]{Err: v.Err} + } + + var record types.Record = v.Val + return iter.Result[types.Record]{Val: record} + }) + + filteredIter := applyFiltersToIter(mappedIter, filterAddrs, filterProtocols) + peers, err := iter.ReadAllResults(filteredIter) if err != nil { writeErr(w, "FindPeers", http.StatusInternalServerError, fmt.Errorf("delegate error: %w", err)) return @@ -390,8 +405,19 @@ func (s *server) findPeersJSON(w http.ResponseWriter, peersIter iter.ResultIter[ }) } -func (s *server) findPeersNDJSON(w http.ResponseWriter, peersIter iter.ResultIter[*types.PeerRecord]) { - writeResultsIterNDJSON(w, peersIter) +func (s *server) findPeersNDJSON(w http.ResponseWriter, peersIter iter.ResultIter[*types.PeerRecord], filterAddrs, filterProtocols []string) { + // Convert PeerRecord to Record so that we can reuse the filtering logic from findProviders + mappedIter := iter.Map(peersIter, func(v iter.Result[*types.PeerRecord]) iter.Result[types.Record] { + if v.Err != nil || v.Val == nil { + return iter.Result[types.Record]{Err: v.Err} + } + + var record types.Record = v.Val + return iter.Result[types.Record]{Val: record} + }) + + filteredIter := applyFiltersToIter(mappedIter, filterAddrs, filterProtocols) + writeResultsIterNDJSON(w, filteredIter) } func (s *server) GetIPNS(w http.ResponseWriter, r *http.Request) { diff --git a/routing/http/types/json/responses.go b/routing/http/types/json/responses.go index d8f659ac5..a4d13b3c4 100644 --- a/routing/http/types/json/responses.go +++ b/routing/http/types/json/responses.go @@ -17,7 +17,7 @@ func (r ProvidersResponse) Length() int { // PeersResponse is the result of a GET Peers request. type PeersResponse struct { - Peers []*types.PeerRecord + Peers []types.Record } func (r PeersResponse) Length() int { From 0a8bebdd9f1ccee0481c063e58f9d9133416a3a3 Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Tue, 24 Sep 2024 15:48:02 +0200 Subject: [PATCH 10/20] fix: use PeerRecord in the FindPeers --- routing/http/server/server.go | 14 +++++++++++++- routing/http/types/json/responses.go | 2 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/routing/http/server/server.go b/routing/http/server/server.go index 98d902396..3cdcefee0 100644 --- a/routing/http/server/server.go +++ b/routing/http/server/server.go @@ -394,7 +394,19 @@ func (s *server) findPeersJSON(w http.ResponseWriter, peersIter iter.ResultIter[ }) filteredIter := applyFiltersToIter(mappedIter, filterAddrs, filterProtocols) - peers, err := iter.ReadAllResults(filteredIter) + + // Convert Record back to PeerRecord 🙃 + finalIter := iter.Map(filteredIter, func(v iter.Result[types.Record]) iter.Result[*types.PeerRecord] { + if v.Err != nil || v.Val == nil { + return iter.Result[*types.PeerRecord]{Err: v.Err} + } + + var record *types.PeerRecord = v.Val.(*types.PeerRecord) + return iter.Result[*types.PeerRecord]{Val: record} + }) + + peers, err := iter.ReadAllResults(finalIter) + if err != nil { writeErr(w, "FindPeers", http.StatusInternalServerError, fmt.Errorf("delegate error: %w", err)) return diff --git a/routing/http/types/json/responses.go b/routing/http/types/json/responses.go index a4d13b3c4..d8f659ac5 100644 --- a/routing/http/types/json/responses.go +++ b/routing/http/types/json/responses.go @@ -17,7 +17,7 @@ func (r ProvidersResponse) Length() int { // PeersResponse is the result of a GET Peers request. type PeersResponse struct { - Peers []types.Record + Peers []*types.PeerRecord } func (r PeersResponse) Length() int { From 825d82fdf589c22671075a652264fcbb5feacdb0 Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Tue, 24 Sep 2024 15:55:33 +0200 Subject: [PATCH 11/20] chore: ignore check --- routing/http/server/filters.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go index 73859a7f2..4cdf04702 100644 --- a/routing/http/server/filters.go +++ b/routing/http/server/filters.go @@ -69,6 +69,7 @@ func applyFiltersToIter(recordsIter iter.ResultIter[types.Record], filterAddrs, return filteredIter } +//lint:ignore U1000 // ignore unused func filterRecords(records []types.Record, filterAddrs, filterProtocols []string) []types.Record { if len(filterAddrs) == 0 && len(filterProtocols) == 0 { return records From b6ed0dcae30cf16046577da2f0c2b0a149a886d9 Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Wed, 25 Sep 2024 11:51:32 +0200 Subject: [PATCH 12/20] fix: conversion from bitswap record --- routing/http/client/client_test.go | 8 +++----- routing/http/types/record_peer.go | 1 + 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/routing/http/client/client_test.go b/routing/http/client/client_test.go index 590deed11..732861797 100644 --- a/routing/http/client/client_test.go +++ b/routing/http/client/client_test.go @@ -228,9 +228,7 @@ func TestClient_FindProviders(t *testing.T) { } bitswapRecord := makeBitswapRecord() - bitswapProviders := []iter.Result[types.Record]{ - {Val: &bitswapRecord}, - } + peerRecordFromBitswapRecord := types.FromBitswapRecord(&bitswapRecord) cases := []struct { name string @@ -254,8 +252,8 @@ func TestClient_FindProviders(t *testing.T) { }, { name: "happy case (with deprecated bitswap schema)", - routerResult: bitswapProviders, - expResult: bitswapProviders, + routerResult: []iter.Result[types.Record]{{Val: &bitswapRecord}}, + expResult: []iter.Result[types.Record]{{Val: peerRecordFromBitswapRecord}}, expStreamingResponse: true, }, { diff --git a/routing/http/types/record_peer.go b/routing/http/types/record_peer.go index f0d2cab60..cb4a04fca 100644 --- a/routing/http/types/record_peer.go +++ b/routing/http/types/record_peer.go @@ -86,5 +86,6 @@ func FromBitswapRecord(br *BitswapRecord) *PeerRecord { ID: br.ID, Addrs: br.Addrs, Protocols: []string{br.Protocol}, + Extra: map[string]json.RawMessage{}, } } From f210c0d7c4e1fb09813be0d00268d5c990f5050c Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Wed, 25 Sep 2024 13:55:32 +0200 Subject: [PATCH 13/20] test: add addr and protocol tests to peer handler --- routing/http/server/server_test.go | 139 ++++++++++++++++++++++++++--- 1 file changed, 127 insertions(+), 12 deletions(-) diff --git a/routing/http/server/server_test.go b/routing/http/server/server_test.go index c0698a51f..772f79999 100644 --- a/routing/http/server/server_test.go +++ b/routing/http/server/server_test.go @@ -268,10 +268,21 @@ func TestProviders(t *testing.T) { } func TestPeers(t *testing.T) { - makeRequest := func(t *testing.T, router *mockContentRouter, contentType, arg string) *http.Response { + makeRequest := func(t *testing.T, router *mockContentRouter, contentType, arg, filterAddrs, filterProtocols string) *http.Response { server := httptest.NewServer(Handler(router)) t.Cleanup(server.Close) - req, err := http.NewRequest(http.MethodGet, "http://"+server.Listener.Addr().String()+"/routing/v1/peers/"+arg, nil) + + urlStr := fmt.Sprintf("http://%s/routing/v1/peers/%s", server.Listener.Addr().String(), arg) + if filterAddrs != "" || filterProtocols != "" { + urlStr += "?" + if filterAddrs != "" { + urlStr = fmt.Sprintf("%s&filter-addrs=%s", urlStr, filterAddrs) + } + if filterProtocols != "" { + urlStr = fmt.Sprintf("%s&filter-protocols=%s", urlStr, filterProtocols) + } + } + req, err := http.NewRequest(http.MethodGet, urlStr, nil) require.NoError(t, err) if contentType != "" { req.Header.Set("Accept", contentType) @@ -285,7 +296,7 @@ func TestPeers(t *testing.T) { t.Parallel() router := &mockContentRouter{} - resp := makeRequest(t, router, mediaTypeJSON, "nonpeerid") + resp := makeRequest(t, router, mediaTypeJSON, "nonpeerid", "", "") require.Equal(t, 400, resp.StatusCode) }) @@ -298,7 +309,7 @@ func TestPeers(t *testing.T) { router := &mockContentRouter{} router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(results, nil) - resp := makeRequest(t, router, mediaTypeJSON, peer.ToCid(pid).String()) + resp := makeRequest(t, router, mediaTypeJSON, peer.ToCid(pid).String(), "", "") require.Equal(t, 404, resp.StatusCode) require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type")) @@ -318,7 +329,7 @@ func TestPeers(t *testing.T) { router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(results, nil) // Simulate request with Accept header that includes wildcard match - resp := makeRequest(t, router, "text/html,*/*", peer.ToCid(pid).String()) + resp := makeRequest(t, router, "text/html,*/*", peer.ToCid(pid).String(), "", "") // Expect response to default to application/json require.Equal(t, 404, resp.StatusCode) @@ -336,7 +347,7 @@ func TestPeers(t *testing.T) { router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(results, nil) // Simulate request without Accept header - resp := makeRequest(t, router, "", peer.ToCid(pid).String()) + resp := makeRequest(t, router, "", peer.ToCid(pid).String(), "", "") // Expect response to default to application/json require.Equal(t, 404, resp.StatusCode) @@ -352,7 +363,7 @@ func TestPeers(t *testing.T) { router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(nil, routing.ErrNotFound) // Simulate request without Accept header - resp := makeRequest(t, router, "", peer.ToCid(pid).String()) + resp := makeRequest(t, router, "", peer.ToCid(pid).String(), "", "") // Expect response to default to application/json require.Equal(t, 404, resp.StatusCode) @@ -382,7 +393,7 @@ func TestPeers(t *testing.T) { router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(results, nil) libp2pKeyCID := peer.ToCid(pid).String() - resp := makeRequest(t, router, mediaTypeJSON, libp2pKeyCID) + resp := makeRequest(t, router, mediaTypeJSON, libp2pKeyCID, "", "") require.Equal(t, 200, resp.StatusCode) require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type")) @@ -398,6 +409,110 @@ func TestPeers(t *testing.T) { require.Equal(t, expectedBody, string(body)) }) + t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body and headers (JSON) with filter-addrs", func(t *testing.T) { + t.Parallel() + + addr1, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/tcp/4001") + addr2, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/udp/4001/quic-v1") + addr3, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/tcp/4001/ws") + addr4, _ := multiaddr.NewMultiaddr("/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit") + addr5, _ := multiaddr.NewMultiaddr("/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu") + _, pid := makeEd25519PeerID(t) + _, pid2 := makeEd25519PeerID(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{ + {Multiaddr: addr1}, + {Multiaddr: addr2}, + {Multiaddr: addr3}, + {Multiaddr: addr4}, + }, + }}, + {Val: &types.PeerRecord{ + Schema: types.SchemaPeer, + ID: &pid2, + Protocols: []string{"transport-foo"}, + Addrs: []types.Multiaddr{ + {Multiaddr: addr5}, + }, + }}, + }) + + router := &mockContentRouter{} + router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(results, nil) + + libp2pKeyCID := peer.ToCid(pid).String() + resp := makeRequest(t, router, mediaTypeJSON, libp2pKeyCID, "tcp", "") + require.Equal(t, 200, resp.StatusCode) + + require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type")) + require.Equal(t, "Accept", resp.Header.Get("Vary")) + require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control")) + + requireCloseToNow(t, resp.Header.Get("Last-Modified")) + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + expectedBody := `{"Peers":[{"Addrs":["/ip4/127.0.0.1/tcp/4001","/ip4/127.0.0.1/tcp/4001/ws"],"ID":"` + pid.String() + `","Protocols":["transport-bitswap","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 and headers (JSON) with filter-protocols", func(t *testing.T) { + t.Parallel() + + addr1, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/tcp/4001") + addr2, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/udp/4001/quic-v1") + addr3, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/tcp/4001/ws") + addr4, _ := multiaddr.NewMultiaddr("/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit") + addr5, _ := multiaddr.NewMultiaddr("/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu") + _, pid := makeEd25519PeerID(t) + _, pid2 := makeEd25519PeerID(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{ + {Multiaddr: addr1}, + {Multiaddr: addr2}, + {Multiaddr: addr3}, + {Multiaddr: addr4}, + }, + }}, + {Val: &types.PeerRecord{ + Schema: types.SchemaPeer, + ID: &pid2, + Protocols: []string{"transport-foo"}, + Addrs: []types.Multiaddr{ + {Multiaddr: addr5}, + }, + }}, + }) + + router := &mockContentRouter{} + router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(results, nil) + + libp2pKeyCID := peer.ToCid(pid).String() + resp := makeRequest(t, router, mediaTypeJSON, libp2pKeyCID, "", "transport-bitswap") + require.Equal(t, 200, resp.StatusCode) + + require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type")) + require.Equal(t, "Accept", resp.Header.Get("Vary")) + require.Equal(t, "public, max-age=300, stale-while-revalidate=172800, stale-if-error=172800", resp.Header.Get("Cache-Control")) + + requireCloseToNow(t, resp.Header.Get("Last-Modified")) + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + expectedBody := `{"Peers":[{"Addrs":["/ip4/127.0.0.1/tcp/4001","/ip4/127.0.0.1/udp/4001/quic-v1","/ip4/127.0.0.1/tcp/4001/ws","/ip4/102.101.1.1/udp/4001/quic-v1/webtransport/p2p/12D3KooWEjsGPUQJ4Ej3d1Jcg4VckWhFbhc6mkGunMm1faeSzZMu/p2p-circuit"],"ID":"` + pid.String() + `","Protocols":["transport-bitswap","transport-foo"],"Schema":"peer"}]}` + require.Equal(t, expectedBody, string(body)) + }) + t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 404 with correct body and headers (No Results, NDJSON)", func(t *testing.T) { t.Parallel() @@ -407,7 +522,7 @@ func TestPeers(t *testing.T) { router := &mockContentRouter{} router.On("FindPeers", mock.Anything, pid, DefaultStreamingRecordsLimit).Return(results, nil) - resp := makeRequest(t, router, mediaTypeNDJSON, peer.ToCid(pid).String()) + resp := makeRequest(t, router, mediaTypeNDJSON, peer.ToCid(pid).String(), "", "") require.Equal(t, 404, resp.StatusCode) require.Equal(t, mediaTypeNDJSON, resp.Header.Get("Content-Type")) @@ -440,7 +555,7 @@ func TestPeers(t *testing.T) { router.On("FindPeers", mock.Anything, pid, DefaultStreamingRecordsLimit).Return(results, nil) libp2pKeyCID := peer.ToCid(pid).String() - resp := makeRequest(t, router, mediaTypeNDJSON, libp2pKeyCID) + resp := makeRequest(t, router, mediaTypeNDJSON, libp2pKeyCID, "", "") require.Equal(t, 200, resp.StatusCode) require.Equal(t, mediaTypeNDJSON, resp.Header.Get("Content-Type")) @@ -502,7 +617,7 @@ func TestPeers(t *testing.T) { router := &mockContentRouter{} router.On("FindPeers", mock.Anything, pid, DefaultStreamingRecordsLimit).Return(iter.FromSlice(results), nil) - resp := makeRequest(t, router, mediaTypeNDJSON, peerIDStr) + resp := makeRequest(t, router, mediaTypeNDJSON, peerIDStr, "", "") require.Equal(t, 200, resp.StatusCode) require.Equal(t, mediaTypeNDJSON, resp.Header.Get("Content-Type")) @@ -522,7 +637,7 @@ func TestPeers(t *testing.T) { router := &mockContentRouter{} router.On("FindPeers", mock.Anything, pid, DefaultRecordsLimit).Return(iter.FromSlice(results), nil) - resp := makeRequest(t, router, mediaTypeJSON, peerIDStr) + resp := makeRequest(t, router, mediaTypeJSON, peerIDStr, "", "") require.Equal(t, 200, resp.StatusCode) require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type")) From ef3f6b67bbd53b27bae609bd4e83614d9a585cf2 Mon Sep 17 00:00:00 2001 From: Daniel Norman <1992255+2color@users.noreply.github.com> Date: Wed, 25 Sep 2024 14:35:53 +0200 Subject: [PATCH 14/20] Apply suggestions from code review Co-authored-by: Andrew Gillis <11790789+gammazero@users.noreply.github.com> --- routing/http/server/filters.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go index 4cdf04702..65e131f27 100644 --- a/routing/http/server/filters.go +++ b/routing/http/server/filters.go @@ -136,7 +136,7 @@ func applyAddrFilter(addrs []types.Multiaddr, filterAddrsQuery []string) []types return addrs } - filteredAddrs := make([]types.Multiaddr, 0, len(addrs)) + var filteredAddrs []types.Multiaddr for _, addr := range addrs { protocols := addr.Protocols() @@ -145,8 +145,7 @@ func applyAddrFilter(addrs []types.Multiaddr, filterAddrsQuery []string) []types // First, check all negative filters for _, filter := range filterAddrsQuery { if strings.HasPrefix(filter, "!") { - protocolStringFromFilter := strings.TrimPrefix(filter, "!") - protocolFromFilter := multiaddr.ProtocolWithName(protocolStringFromFilter) + protocolFromFilter := multiaddr.ProtocolWithName(filter[1:]) if containsProtocol(protocols, protocolFromFilter) { includeAddr = false break From 9811e8320613bd4d4a51d24d5195452d3a1f9d59 Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Wed, 25 Sep 2024 14:44:53 +0200 Subject: [PATCH 15/20] fix: return nil when a record doesnt pass filter --- routing/http/server/filters.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go index 65e131f27..61c2613df 100644 --- a/routing/http/server/filters.go +++ b/routing/http/server/filters.go @@ -1,7 +1,6 @@ package server import ( - "errors" "reflect" "slices" "strings" @@ -38,7 +37,7 @@ func applyFiltersToIter(recordsIter iter.ResultIter[types.Record], filterAddrs, record = applyFilters(record, filterAddrs, filterProtocols) if record == nil { - return iter.Result[types.Record]{Err: errors.New("record is nil")} + return iter.Result[types.Record]{} } v.Val = record @@ -54,7 +53,7 @@ func applyFiltersToIter(recordsIter iter.ResultIter[types.Record], filterAddrs, peerRecord := types.FromBitswapRecord(record) peerRecord = applyFilters(peerRecord, filterAddrs, filterProtocols) if peerRecord == nil { - return iter.Result[types.Record]{Err: errors.New("record is nil")} + return iter.Result[types.Record]{} } v.Val = peerRecord } From 51f200a698a2ec366657b1c4e89cfe7c5dd7e25f Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Wed, 25 Sep 2024 14:50:27 +0200 Subject: [PATCH 16/20] fix: rename to protocolsAllowed for readability --- routing/http/server/filters.go | 5 +++-- routing/http/server/filters_test.go | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go index 61c2613df..ccec45230 100644 --- a/routing/http/server/filters.go +++ b/routing/http/server/filters.go @@ -107,7 +107,7 @@ func applyFilters(provider *types.PeerRecord, filterAddrs, filterProtocols []str return provider } - if !applyProtocolFilter(provider.Protocols, filterProtocols) { + if !protocolsAllowed(provider.Protocols, filterProtocols) { // If the provider doesn't match any of the passed protocols, the provider is omitted from the response. return nil } @@ -177,7 +177,8 @@ func containsProtocol(protos []multiaddr.Protocol, proto multiaddr.Protocol) boo return false } -func applyProtocolFilter(peerProtocols []string, filterProtocols []string) bool { +// protocolsAllowed returns true if the peerProtocols are allowed by the filter protocols. +func protocolsAllowed(peerProtocols []string, filterProtocols []string) bool { if len(filterProtocols) == 0 { // If no filter is passed, do not filter return true diff --git a/routing/http/server/filters_test.go b/routing/http/server/filters_test.go index 3fcda11a5..04f9b7354 100644 --- a/routing/http/server/filters_test.go +++ b/routing/http/server/filters_test.go @@ -121,7 +121,7 @@ func TestApplyAddrFilter(t *testing.T) { } } -func TestApplyProtocolFilter(t *testing.T) { +func TestProtocolsAllowed(t *testing.T) { testCases := []struct { name string peerProtocols []string @@ -174,7 +174,7 @@ func TestApplyProtocolFilter(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - result := applyProtocolFilter(tc.peerProtocols, tc.filterProtocols) + result := protocolsAllowed(tc.peerProtocols, tc.filterProtocols) assert.Equal(t, tc.expected, result, "Unexpected result for test case: %s", tc.name) }) } From d8edbe6e9a1ce2388a8327842cc0dbb3a660380a Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Wed, 25 Sep 2024 15:07:59 +0200 Subject: [PATCH 17/20] fix: include addresses that passed negative filters Addresses https://github.com/ipfs/boxo/pull/671#discussion_r1775014830 --- routing/http/server/filters.go | 52 ++++++++++++++++------------- routing/http/server/filters_test.go | 4 +-- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go index ccec45230..8c3e47ad9 100644 --- a/routing/http/server/filters.go +++ b/routing/http/server/filters.go @@ -129,45 +129,51 @@ func applyFilters(provider *types.PeerRecord, filterAddrs, filterProtocols []str } // If there are only negative filters, no addresses will be included in the result. The function will return an empty list. -// For an address to be included, it must pass all negative filters AND match at least one positive filter. +// For an address to be included, it must pass all negative filters func applyAddrFilter(addrs []types.Multiaddr, filterAddrsQuery []string) []types.Multiaddr { if len(filterAddrsQuery) == 0 { return addrs } var filteredAddrs []types.Multiaddr + var positiveFilters, negativeFilters []multiaddr.Protocol + + // Separate positive and negative filters + for _, filter := range filterAddrsQuery { + if strings.HasPrefix(filter, "!") { + negativeFilters = append(negativeFilters, multiaddr.ProtocolWithName(filter[1:])) + } else { + positiveFilters = append(positiveFilters, multiaddr.ProtocolWithName(filter)) + } + } for _, addr := range addrs { protocols := addr.Protocols() - includeAddr := true - - // First, check all negative filters - for _, filter := range filterAddrsQuery { - if strings.HasPrefix(filter, "!") { - protocolFromFilter := multiaddr.ProtocolWithName(filter[1:]) - if containsProtocol(protocols, protocolFromFilter) { - includeAddr = false - break - } - } + + // Check negative filters + if containsAny(protocols, negativeFilters) { + continue } - // If the address passed all negative filters, check positive filters - if includeAddr { - for _, filter := range filterAddrsQuery { - if !strings.HasPrefix(filter, "!") { - protocolFromFilter := multiaddr.ProtocolWithName(filter) - if containsProtocol(protocols, protocolFromFilter) { - filteredAddrs = append(filteredAddrs, addr) - break - } - } - } + // If no positive filters or matches a positive filter, include the address + if len(positiveFilters) == 0 || containsAny(protocols, positiveFilters) { + filteredAddrs = append(filteredAddrs, addr) } } + return filteredAddrs } +// Helper function to check if protocols contain any of the filters +func containsAny(protocols []multiaddr.Protocol, filters []multiaddr.Protocol) bool { + for _, filter := range filters { + if containsProtocol(protocols, filter) { + return true + } + } + return false +} + func containsProtocol(protos []multiaddr.Protocol, proto multiaddr.Protocol) bool { for _, p := range protos { if p.Code == proto.Code { diff --git a/routing/http/server/filters_test.go b/routing/http/server/filters_test.go index 04f9b7354..462ddfad5 100644 --- a/routing/http/server/filters_test.go +++ b/routing/http/server/filters_test.go @@ -60,7 +60,7 @@ func TestApplyAddrFilter(t *testing.T) { { name: "Exclude TCP", filterAddrs: []string{"!tcp"}, - expectedAddrs: []types.Multiaddr{}, + expectedAddrs: []types.Multiaddr{{Multiaddr: addr2}, {Multiaddr: addr5}, {Multiaddr: addr6}, {Multiaddr: addr8}}, }, { name: "Include WebTransport and exclude p2p-circuit", @@ -85,7 +85,7 @@ func TestApplyAddrFilter(t *testing.T) { { name: "Multiple negative filters", filterAddrs: []string{"!tcp", "!ws"}, - expectedAddrs: []types.Multiaddr{}, + expectedAddrs: []types.Multiaddr{{Multiaddr: addr2}, {Multiaddr: addr5}, {Multiaddr: addr6}, {Multiaddr: addr8}}, }, } From 3d2a8e5a3124d398f370f588304745918d2393ea Mon Sep 17 00:00:00 2001 From: Daniel N <2color@users.noreply.github.com> Date: Wed, 25 Sep 2024 15:39:39 +0200 Subject: [PATCH 18/20] docs: improve comments and add test --- routing/http/server/filters.go | 27 +++++++++++++++++++++++++-- routing/http/server/filters_test.go | 5 +++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go index 8c3e47ad9..a05f781e4 100644 --- a/routing/http/server/filters.go +++ b/routing/http/server/filters.go @@ -20,6 +20,14 @@ func parseFilter(param string) []string { } // applyFiltersToIter applies the filters to the given iterator and returns a new iterator. +// +// The function iterates over the input iterator, applying the specified filters to each record. +// It supports both positive and negative filters for both addresses and protocols. +// +// Parameters: +// - recordsIter: An iterator of types.Record to be filtered. +// - filterAddrs: A slice of strings representing the address filter criteria. +// - filterProtocols: A slice of strings representing the protocol filter criteria. func applyFiltersToIter(recordsIter iter.ResultIter[types.Record], filterAddrs, filterProtocols []string) iter.ResultIter[types.Record] { mappedIter := iter.Map(recordsIter, func(v iter.Result[types.Record]) iter.Result[types.Record] { if v.Err != nil || v.Val == nil { @@ -128,8 +136,23 @@ func applyFilters(provider *types.PeerRecord, filterAddrs, filterProtocols []str return provider } -// If there are only negative filters, no addresses will be included in the result. The function will return an empty list. -// For an address to be included, it must pass all negative filters +// applyAddrFilter filters a list of multiaddresses based on the provided filter query. +// +// Parameters: +// - addrs: A slice of types.Multiaddr to be filtered. +// - filterAddrsQuery: A slice of strings representing the filter criteria. +// +// The function supports both positive and negative filters: +// - Positive filters (e.g., "tcp", "udp") include addresses that match the specified protocols. +// - Negative filters (e.g., "!tcp", "!udp") exclude addresses that match the specified protocols. +// +// If no filters are provided, the original list of addresses is returned unchanged. +// If only negative filters are provided, addresses not matching any negative filter are included. +// If positive filters are provided, only addresses matching at least one positive filter (and no negative filters) are included. +// If both positive and negative filters are provided, the address must match at least one positive filter and no negative filters to be included. +// +// Returns: +// A new slice of types.Multiaddr containing only the addresses that pass the filter criteria. func applyAddrFilter(addrs []types.Multiaddr, filterAddrsQuery []string) []types.Multiaddr { if len(filterAddrsQuery) == 0 { return addrs diff --git a/routing/http/server/filters_test.go b/routing/http/server/filters_test.go index 462ddfad5..c3f2e5a9a 100644 --- a/routing/http/server/filters_test.go +++ b/routing/http/server/filters_test.go @@ -62,6 +62,11 @@ func TestApplyAddrFilter(t *testing.T) { filterAddrs: []string{"!tcp"}, expectedAddrs: []types.Multiaddr{{Multiaddr: addr2}, {Multiaddr: addr5}, {Multiaddr: addr6}, {Multiaddr: addr8}}, }, + { + name: "Filter TCP addresses that don't have WebSocket and p2p-circuit", + filterAddrs: []string{"tcp", "!ws", "!wss", "!p2p-circuit"}, + expectedAddrs: []types.Multiaddr{{Multiaddr: addr1}}, + }, { name: "Include WebTransport and exclude p2p-circuit", filterAddrs: []string{"webtransport", "!p2p-circuit"}, From f13c862ff121c99e2450a7915b8b7068357a10cb Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Thu, 26 Sep 2024 16:37:04 +0200 Subject: [PATCH 19/20] test: add real world test case --- routing/http/server/filters_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/routing/http/server/filters_test.go b/routing/http/server/filters_test.go index c3f2e5a9a..078e4aa96 100644 --- a/routing/http/server/filters_test.go +++ b/routing/http/server/filters_test.go @@ -169,6 +169,24 @@ func TestProtocolsAllowed(t *testing.T) { filterProtocols: []string{"unknown"}, expected: false, }, + { + name: "Unknown or specific protocol for matching non-empty peer protocols", + peerProtocols: []string{"transport-bitswap"}, + filterProtocols: []string{"unknown", "transport-bitswap", "transport-ipfs-gateway-http"}, + expected: true, + }, + { + name: "Unknown or specific protocol for matching empty peer protocols", + peerProtocols: []string{}, + filterProtocols: []string{"unknown", "transport-bitswap", "transport-ipfs-gateway-http"}, + expected: true, + }, + { + name: "Unknown or specific protocol for not matching non-empty peer protocols", + peerProtocols: []string{"transport-graphsync-filecoinv1"}, + filterProtocols: []string{"unknown", "transport-bitswap", "transport-ipfs-gateway-http"}, + expected: false, + }, { name: "Case insensitive match", peerProtocols: []string{"TRANSPORT-BITSWAP", "Transport-IPFS-Gateway-HTTP"}, From 137d34f018eeaf851a19a971fe42afda9b135a02 Mon Sep 17 00:00:00 2001 From: Daniel Norman <1992255+2color@users.noreply.github.com> Date: Fri, 27 Sep 2024 11:23:08 +0200 Subject: [PATCH 20/20] Apply suggestions from code review Co-authored-by: Marcin Rataj --- CHANGELOG.md | 2 +- routing/http/server/filters.go | 40 ++++------------------------------ 2 files changed, 5 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd21360e0..472c0aad2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ The following emojis are used to highlight certain changes: ### Added -- Added address and protocol filtering to the delegated routing server [IPIP-484](https://github.com/ipfs/specs/pull/484) +- `routing/http`: added support for address and protocol filtering to the delegated routing server ([IPIP-484](https://github.com/ipfs/specs/pull/484)) [#671](https://github.com/ipfs/boxo/pull/671) ### Changed diff --git a/routing/http/server/filters.go b/routing/http/server/filters.go index a05f781e4..bb5dfa0d5 100644 --- a/routing/http/server/filters.go +++ b/routing/http/server/filters.go @@ -39,8 +39,8 @@ func applyFiltersToIter(recordsIter iter.ResultIter[types.Record], filterAddrs, record, ok := v.Val.(*types.PeerRecord) if !ok { logger.Errorw("problem casting find providers record", "Schema", v.Val.GetSchema(), "Type", reflect.TypeOf(v).String()) - // TODO: Do we want to let failed type assertions to pass through? - return v + // drop failed type assertion + return iter.Result[types.Record]{} } record = applyFilters(record, filterAddrs, filterProtocols) @@ -55,8 +55,8 @@ func applyFiltersToIter(recordsIter iter.ResultIter[types.Record], filterAddrs, record, ok := v.Val.(*types.BitswapRecord) if !ok { logger.Errorw("problem casting find providers record", "Schema", v.Val.GetSchema(), "Type", reflect.TypeOf(v).String()) - // TODO: Do we want to let failed type assertions to pass through? - return v + // drop failed type assertion + return iter.Result[types.Record]{} } peerRecord := types.FromBitswapRecord(record) peerRecord = applyFilters(peerRecord, filterAddrs, filterProtocols) @@ -76,38 +76,6 @@ func applyFiltersToIter(recordsIter iter.ResultIter[types.Record], filterAddrs, return filteredIter } -//lint:ignore U1000 // ignore unused -func filterRecords(records []types.Record, filterAddrs, filterProtocols []string) []types.Record { - if len(filterAddrs) == 0 && len(filterProtocols) == 0 { - return records - } - - filtered := make([]types.Record, 0, len(records)) - - for _, record := range records { - // TODO: Handle SchemaBitswap - if schema := record.GetSchema(); schema == types.SchemaPeer { - peer, ok := record.(*types.PeerRecord) - if !ok { - logger.Errorw("problem casting find providers result", "Schema", record.GetSchema(), "Type", reflect.TypeOf(record).String()) - // if the type assertion fails, we exlude record from results - continue - } - - record := applyFilters(peer, filterAddrs, filterProtocols) - - if record != nil { - filtered = append(filtered, record) - } - - } else { - // Will we ever encounter the SchemaBitswap type? Evidence seems to suggest that no longer - logger.Errorw("encountered unknown provider schema", "Schema", record.GetSchema(), "Type", reflect.TypeOf(record).String()) - } - } - return filtered -} - // Applies the filters. Returns nil if the provider does not pass the protocols filter // The address filter is more complicated because it potentially modifies the Addrs slice. func applyFilters(provider *types.PeerRecord, filterAddrs, filterProtocols []string) *types.PeerRecord {