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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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/27] 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 628b0f6f68e5028d48ef458e0b7416d6b72d4cea Mon Sep 17 00:00:00 2001 From: Jorropo Date: Tue, 24 Sep 2024 19:15:25 +0200 Subject: [PATCH 12/27] ipld/unixfs/hamt: catch panic in walkChildren (#393) * ipld/unixfs/hamt: catch panic in walkChildren * Add test for nil link and shard * rename test * Update CHANGELOG.md --------- Co-authored-by: Lucas Molas Co-authored-by: Andrew Gillis <11790789+gammazero@users.noreply.github.com> --- CHANGELOG.md | 1 + ipld/unixfs/hamt/hamt.go | 16 +++++++++++----- ipld/unixfs/hamt/hamt_test.go | 21 +++++++++++++++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7081f01cd..3bea6c343 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The following emojis are used to highlight certain changes: ### Removed ### Fixed += `unixfs/hamt` Log error instead of panic if both link and shard are nil [#393](https://github.com/ipfs/boxo/pull/393) ### Security diff --git a/ipld/unixfs/hamt/hamt.go b/ipld/unixfs/hamt/hamt.go index a57ddad41..455d070c6 100644 --- a/ipld/unixfs/hamt/hamt.go +++ b/ipld/unixfs/hamt/hamt.go @@ -29,8 +29,6 @@ import ( "os" "sync" - "golang.org/x/sync/errgroup" - format "github.com/ipfs/boxo/ipld/unixfs" "github.com/ipfs/boxo/ipld/unixfs/internal" @@ -38,8 +36,12 @@ import ( bitfield "github.com/ipfs/go-bitfield" cid "github.com/ipfs/go-cid" ipld "github.com/ipfs/go-ipld-format" + logging "github.com/ipfs/go-log/v2" + "golang.org/x/sync/errgroup" ) +var log = logging.Logger("unixfs-hamt") + const ( // HashMurmur3 is the multiformats identifier for Murmur3 HashMurmur3 uint64 = 0x22 @@ -430,8 +432,13 @@ type listCidsAndShards struct { func (ds *Shard) walkChildren(processLinkValues func(formattedLink *ipld.Link) error) (*listCidsAndShards, error) { res := &listCidsAndShards{} - for idx, lnk := range ds.childer.links { - if nextShard := ds.childer.children[idx]; nextShard == nil { + for i, nextShard := range ds.childer.children { + if nextShard == nil { + lnk := ds.childer.link(i) + if lnk == nil { + log.Warnf("internal HAMT error: both link and shard nil at pos %d, dumping shard: %+v", i, *ds) + return nil, fmt.Errorf("internal HAMT error: both link and shard nil, check log") + } lnkLinkType, err := ds.childLinkType(lnk) if err != nil { return nil, err @@ -454,7 +461,6 @@ func (ds *Shard) walkChildren(processLinkValues func(formattedLink *ipld.Link) e default: return nil, errors.New("unsupported shard link type") } - } else { if nextShard.val != nil { formattedLink := &ipld.Link{ diff --git a/ipld/unixfs/hamt/hamt_test.go b/ipld/unixfs/hamt/hamt_test.go index 1946bbd7c..16b325773 100644 --- a/ipld/unixfs/hamt/hamt_test.go +++ b/ipld/unixfs/hamt/hamt_test.go @@ -749,3 +749,24 @@ func TestHamtBadSize(t *testing.T) { } } } + +func TestHamtNilLinkAndShard(t *testing.T) { + shard, err := NewShard(nil, 1024) + if err != nil { + t.Fatal(err) + } + shard.childer = shard.childer.makeChilder(nil, []*ipld.Link{nil}) + nextShard, err := shard.walkChildren(func(_ *ipld.Link) error { + t.Fatal("processLinkValues function should not have been called") + return nil + }) + if err == nil { + t.Fatal("expected error") + } + if err.Error() != "internal HAMT error: both link and shard nil, check log" { + t.Fatal("did not get expected error") + } + if nextShard != nil { + t.Fatal("nextShard should be nil") + } +} 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 13/27] 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 14/27] 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 15/27] 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 16/27] 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 17/27] 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 18/27] 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 19/27] 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 20/27] 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 21/27] 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 { From 19a402b7dc34491a735696840a67a5343def4dd4 Mon Sep 17 00:00:00 2001 From: Andrew Gillis <11790789+gammazero@users.noreply.github.com> Date: Fri, 27 Sep 2024 16:48:53 -0700 Subject: [PATCH 22/27] feat: option to not read size of blocks for want-have requests (#672) * Do not fetch size for WantHave blocks * Option to set replaceHasWithBlockMaxSize * Log if replace logic enabled/disabled * docs: WithWantHaveReplaceSize --------- Co-authored-by: Marcin Rataj --- CHANGELOG.md | 2 + bitswap/internal/defaults/defaults.go | 3 + bitswap/options.go | 7 + .../internal/decision/blockstoremanager.go | 36 +++++ .../decision/blockstoremanager_test.go | 15 +- bitswap/server/internal/decision/engine.go | 130 ++++++++++-------- .../server/internal/decision/engine_test.go | 12 +- bitswap/server/server.go | 32 +++++ 8 files changed, 163 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 472c0aad2..ee78a8b19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ The following emojis are used to highlight certain changes: ### Added +* `boxo/bitswap/server`: + * A new [`WithWantHaveReplaceSize(n)`](https://pkg.go.dev/github.com/ipfs/boxo/bitswap/server/#WithWantHaveReplaceSize) option can be used with `bitswap.New` to fine-tune cost-vs-performance. It sets the maximum size of a block in bytes up to which the bitswap server will replace a WantHave with a WantBlock response. Setting this to 0 disables this WantHave replacement and means that block sizes are not read when processing WantHave requests. [#672](https://github.com/ipfs/boxo/pull/672) - `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/bitswap/internal/defaults/defaults.go b/bitswap/internal/defaults/defaults.go index f5511cc7a..b30bcc87f 100644 --- a/bitswap/internal/defaults/defaults.go +++ b/bitswap/internal/defaults/defaults.go @@ -37,4 +37,7 @@ const ( // RebroadcastDelay is the default delay to trigger broadcast of // random CIDs in the wantlist. RebroadcastDelay = time.Minute + + // DefaultWantHaveReplaceSize controls the implicit behavior of WithWantHaveReplaceSize. + DefaultWantHaveReplaceSize = 1024 ) diff --git a/bitswap/options.go b/bitswap/options.go index 11e89fdf9..6a98b27db 100644 --- a/bitswap/options.go +++ b/bitswap/options.go @@ -71,6 +71,13 @@ func WithTaskComparator(comparator server.TaskComparator) Option { return Option{server.WithTaskComparator(comparator)} } +// WithWantHaveReplaceSize sets the maximum size of a block in bytes up to +// which the bitswap server will replace a WantHave with a WantBlock response. +// See [server.WithWantHaveReplaceSize] for details. +func WithWantHaveReplaceSize(size int) Option { + return Option{server.WithWantHaveReplaceSize(size)} +} + func ProviderSearchDelay(newProvSearchDelay time.Duration) Option { return Option{client.ProviderSearchDelay(newProvSearchDelay)} } diff --git a/bitswap/server/internal/decision/blockstoremanager.go b/bitswap/server/internal/decision/blockstoremanager.go index aa16b3126..d4c0f4254 100644 --- a/bitswap/server/internal/decision/blockstoremanager.go +++ b/bitswap/server/internal/decision/blockstoremanager.go @@ -121,6 +121,42 @@ func (bsm *blockstoreManager) getBlockSizes(ctx context.Context, ks []cid.Cid) ( return res, nil } +func (bsm *blockstoreManager) hasBlocks(ctx context.Context, ks []cid.Cid) (map[cid.Cid]struct{}, error) { + if len(ks) == 0 { + return nil, nil + } + hasBlocks := make([]bool, len(ks)) + + var count atomic.Int32 + err := bsm.jobPerKey(ctx, ks, func(i int, c cid.Cid) { + has, err := bsm.bs.Has(ctx, c) + if err != nil { + // Note: this isn't a fatal error. We shouldn't abort the request + log.Errorf("blockstore.Has(%c) error: %s", c, err) + return + } + if has { + hasBlocks[i] = true + count.Add(1) + } + }) + if err != nil { + return nil, err + } + results := count.Load() + if results == 0 { + return nil, nil + } + + res := make(map[cid.Cid]struct{}, results) + for i, ok := range hasBlocks { + if ok { + res[ks[i]] = struct{}{} + } + } + return res, nil +} + func (bsm *blockstoreManager) getBlocks(ctx context.Context, ks []cid.Cid) (map[cid.Cid]blocks.Block, error) { if len(ks) == 0 { return nil, nil diff --git a/bitswap/server/internal/decision/blockstoremanager_test.go b/bitswap/server/internal/decision/blockstoremanager_test.go index f65c88e83..2f2b7b23f 100644 --- a/bitswap/server/internal/decision/blockstoremanager_test.go +++ b/bitswap/server/internal/decision/blockstoremanager_test.go @@ -98,29 +98,22 @@ func TestBlockstoreManager(t *testing.T) { cids = append(cids, b.Cid()) } - sizes, err := bsm.getBlockSizes(ctx, cids) + hasBlocks, err := bsm.hasBlocks(ctx, cids) if err != nil { t.Fatal(err) } - if len(sizes) != len(blks)-1 { + if len(hasBlocks) != len(blks)-1 { t.Fatal("Wrong response length") } - for _, c := range cids { - expSize := len(exp[c].RawData()) - size, ok := sizes[c] - - // Only the last key should be missing + _, ok := hasBlocks[c] if c.Equals(cids[len(cids)-1]) { if ok { t.Fatal("Non-existent block should not be in sizes map") } } else { if !ok { - t.Fatal("Block should be in sizes map") - } - if size != expSize { - t.Fatal("Block has wrong size") + t.Fatal("Block should be in hasBlocks") } } } diff --git a/bitswap/server/internal/decision/engine.go b/bitswap/server/internal/decision/engine.go index 1174c94c0..5e4463e33 100644 --- a/bitswap/server/internal/decision/engine.go +++ b/bitswap/server/internal/decision/engine.go @@ -77,10 +77,6 @@ const ( // queuedTagWeight is the default weight for peers that have work queued // on their behalf. queuedTagWeight = 10 - - // maxBlockSizeReplaceHasWithBlock is the maximum size of the block in - // bytes up to which we will replace a want-have with a want-block - maxBlockSizeReplaceHasWithBlock = 1024 ) // Envelope contains a message for a Peer. @@ -202,9 +198,9 @@ type Engine struct { targetMessageSize int - // maxBlockSizeReplaceHasWithBlock is the maximum size of the block in - // bytes up to which we will replace a want-have with a want-block - maxBlockSizeReplaceHasWithBlock int + // wantHaveReplaceSize is the maximum size of the block in bytes up to + // which to replace a WantHave with a WantBlock. + wantHaveReplaceSize int sendDontHaves bool @@ -343,6 +339,14 @@ func WithSetSendDontHave(send bool) Option { } } +// WithWantHaveReplaceSize sets the maximum size of a block in bytes up to +// which to replace a WantHave with a WantBlock response. +func WithWantHaveReplaceSize(size int) Option { + return func(e *Engine) { + e.wantHaveReplaceSize = size + } +} + // wrapTaskComparator wraps a TaskComparator so it can be used as a QueueTaskComparator func wrapTaskComparator(tc TaskComparator) peertask.QueueTaskComparator { return func(a, b *peertask.QueueTask) bool { @@ -369,32 +373,14 @@ func wrapTaskComparator(tc TaskComparator) peertask.QueueTaskComparator { } // NewEngine creates a new block sending engine for the given block store. -// maxOutstandingBytesPerPeer hints to the peer task queue not to give a peer more tasks if it has some maximum -// work already outstanding. +// maxOutstandingBytesPerPeer hints to the peer task queue not to give a peer +// more tasks if it has some maximum work already outstanding. func NewEngine( ctx context.Context, bs bstore.Blockstore, peerTagger PeerTagger, self peer.ID, opts ...Option, -) *Engine { - return newEngine( - ctx, - bs, - peerTagger, - self, - maxBlockSizeReplaceHasWithBlock, - opts..., - ) -} - -func newEngine( - ctx context.Context, - bs bstore.Blockstore, - peerTagger PeerTagger, - self peer.ID, - maxReplaceSize int, - opts ...Option, ) *Engine { e := &Engine{ scoreLedger: NewDefaultScoreLedger(), @@ -404,7 +390,7 @@ func newEngine( outbox: make(chan (<-chan *Envelope), outboxChanBuffer), workSignal: make(chan struct{}, 1), ticker: time.NewTicker(time.Millisecond * 100), - maxBlockSizeReplaceHasWithBlock: maxReplaceSize, + wantHaveReplaceSize: defaults.DefaultWantHaveReplaceSize, taskWorkerCount: defaults.BitswapEngineTaskWorkerCount, sendDontHaves: true, self: self, @@ -445,6 +431,12 @@ func newEngine( e.peerRequestQueue = peertaskqueue.New(peerTaskQueueOpts...) + if e.wantHaveReplaceSize == 0 { + log.Info("Replace WantHave with WantBlock is disabled") + } else { + log.Infow("Replace WantHave with WantBlock is enabled", "maxSize", e.wantHaveReplaceSize) + } + return e } @@ -689,16 +681,38 @@ func (e *Engine) MessageReceived(ctx context.Context, p peer.ID, m bsmsg.BitSwap return true } + noReplace := e.wantHaveReplaceSize == 0 + // Get block sizes for unique CIDs. - wantKs := cid.NewSet() + wantKs := make([]cid.Cid, 0, len(wants)) + var haveKs []cid.Cid for _, entry := range wants { - wantKs.Add(entry.Cid) + if noReplace && entry.WantType == pb.Message_Wantlist_Have { + haveKs = append(haveKs, entry.Cid) + } else { + wantKs = append(wantKs, entry.Cid) + } } - blockSizes, err := e.bsm.getBlockSizes(ctx, wantKs.Keys()) + blockSizes, err := e.bsm.getBlockSizes(ctx, wantKs) if err != nil { log.Info("aborting message processing", err) return false } + if len(haveKs) != 0 { + hasBlocks, err := e.bsm.hasBlocks(ctx, haveKs) + if err != nil { + log.Info("aborting message processing", err) + return false + } + if len(hasBlocks) != 0 { + if blockSizes == nil { + blockSizes = make(map[cid.Cid]int, len(hasBlocks)) + } + for blkCid := range hasBlocks { + blockSizes[blkCid] = 0 + } + } + } e.lock.Lock() @@ -707,20 +721,7 @@ func (e *Engine) MessageReceived(ctx context.Context, p peer.ID, m bsmsg.BitSwap } var overflow []bsmsg.Entry - if len(wants) != 0 { - filteredWants := wants[:0] // shift inplace - for _, entry := range wants { - if !e.peerLedger.Wants(p, entry.Entry) { - // Cannot add entry because it would exceed size limit. - overflow = append(overflow, entry) - continue - } - filteredWants = append(filteredWants, entry) - } - // Clear truncated entries - early GC. - clear(wants[len(filteredWants):]) - wants = filteredWants - } + wants, overflow = e.filterOverflow(p, wants, overflow) if len(overflow) != 0 { log.Infow("handling wantlist overflow", "local", e.self, "from", p, "wantlistSize", len(wants), "overflowSize", len(overflow)) @@ -764,7 +765,7 @@ func (e *Engine) MessageReceived(ctx context.Context, p peer.ID, m bsmsg.BitSwap sendDontHave(entry) } - // For each want-have / want-block + // For each want-block for _, entry := range wants { c := entry.Cid blockSize, found := blockSizes[c] @@ -776,7 +777,10 @@ func (e *Engine) MessageReceived(ctx context.Context, p peer.ID, m bsmsg.BitSwap continue } // The block was found, add it to the queue - isWantBlock := e.sendAsBlock(entry.WantType, blockSize) + + // Check if this is a want-block or a have-block that can be converted + // to a want-block. + isWantBlock := blockSize != 0 && e.sendAsBlock(entry.WantType, blockSize) log.Debugw("Bitswap engine: block found", "local", e.self, "from", p, "cid", c, "isWantBlock", isWantBlock) @@ -810,6 +814,25 @@ func (e *Engine) MessageReceived(ctx context.Context, p peer.ID, m bsmsg.BitSwap return false } +func (e *Engine) filterOverflow(p peer.ID, wants, overflow []bsmsg.Entry) ([]bsmsg.Entry, []bsmsg.Entry) { + if len(wants) == 0 { + return wants, overflow + } + + filteredWants := wants[:0] // shift inplace + for _, entry := range wants { + if !e.peerLedger.Wants(p, entry.Entry) { + // Cannot add entry because it would exceed size limit. + overflow = append(overflow, entry) + continue + } + filteredWants = append(filteredWants, entry) + } + // Clear truncated entries - early GC. + clear(wants[len(filteredWants):]) + return filteredWants, overflow +} + // handleOverflow processes incoming wants that could not be addded to the peer // ledger without exceeding the peer want limit. These are handled by trying to // make room by canceling existing wants for which there is no block. If this @@ -913,17 +936,17 @@ func (e *Engine) splitWantsCancelsDenials(p peer.ID, m bsmsg.BitSwapMessage) ([] continue } + if e.peerBlockRequestFilter != nil && !e.peerBlockRequestFilter(p, c) { + denials = append(denials, et) + continue + } + if et.WantType == pb.Message_Wantlist_Have { log.Debugw("Bitswap engine <- want-have", "local", e.self, "from", p, "cid", c) } else { log.Debugw("Bitswap engine <- want-block", "local", e.self, "from", p, "cid", c) } - if e.peerBlockRequestFilter != nil && !e.peerBlockRequestFilter(p, c) { - denials = append(denials, et) - continue - } - // Do not take more wants that can be handled. if len(wants) < int(e.maxQueuedWantlistEntriesPerPeer) { wants = append(wants, et) @@ -1057,8 +1080,7 @@ func (e *Engine) PeerDisconnected(p peer.ID) { // If the want is a want-have, and it's below a certain size, send the full // block (instead of sending a HAVE) func (e *Engine) sendAsBlock(wantType pb.Message_Wantlist_WantType, blockSize int) bool { - isWantBlock := wantType == pb.Message_Wantlist_Block - return isWantBlock || blockSize <= e.maxBlockSizeReplaceHasWithBlock + return wantType == pb.Message_Wantlist_Block || blockSize <= e.wantHaveReplaceSize } func (e *Engine) numBytesSentTo(p peer.ID) uint64 { diff --git a/bitswap/server/internal/decision/engine_test.go b/bitswap/server/internal/decision/engine_test.go index 593bbde0f..5cc1375c7 100644 --- a/bitswap/server/internal/decision/engine_test.go +++ b/bitswap/server/internal/decision/engine_test.go @@ -188,17 +188,11 @@ func newEngineForTesting( bs blockstore.Blockstore, peerTagger PeerTagger, self peer.ID, - maxReplaceSize int, + wantHaveReplaceSize int, opts ...Option, ) *Engine { - return newEngine( - ctx, - bs, - peerTagger, - self, - maxReplaceSize, - opts..., - ) + opts = append(opts, WithWantHaveReplaceSize(wantHaveReplaceSize)) + return NewEngine(ctx, bs, peerTagger, self, opts...) } func TestOutboxClosedWhenEngineClosed(t *testing.T) { diff --git a/bitswap/server/server.go b/bitswap/server/server.go index 85651a5ef..46d29a8fc 100644 --- a/bitswap/server/server.go +++ b/bitswap/server/server.go @@ -251,6 +251,38 @@ func HasBlockBufferSize(count int) Option { } } +// WithWantHaveReplaceSize sets the maximum size of a block in bytes up to +// which the bitswap server will replace a WantHave with a WantBlock response. +// +// Behavior: +// - If size > 0: The server may send full blocks instead of just confirming possession +// for blocks up to the specified size. +// - If size = 0: WantHave replacement is disabled entirely. This allows the server to +// skip reading block sizes during WantHave request processing, which can be more +// efficient if the data storage bills "possession" checks and "reads" differently. +// +// Performance considerations: +// - Enabling replacement (size > 0) may reduce network round-trips but requires +// checking block sizes for each WantHave request to decide if replacement should occur. +// - Disabling replacement (size = 0) optimizes server performance by avoiding +// block size checks, potentially reducing infrastructure costs if possession checks +// are less expensive than full reads. +// +// It defaults to [defaults.DefaultWantHaveReplaceSize] +// and the value may change in future releases. +// +// Use this option to set explicit behavior to balance between network +// efficiency, server performance, and potential storage cost optimizations +// based on your specific use case and storage backend. +func WithWantHaveReplaceSize(size int) Option { + if size < 0 { + size = 0 + } + return func(bs *Server) { + bs.engineOptions = append(bs.engineOptions, decision.WithWantHaveReplaceSize(size)) + } +} + // WantlistForPeer returns the currently understood list of blocks requested by a // given peer. func (bs *Server) WantlistForPeer(p peer.ID) []cid.Cid { From 4d0ae4542217669607aaa0372777cec4a37846d5 Mon Sep 17 00:00:00 2001 From: Daniel Norman <1992255+2color@users.noreply.github.com> Date: Wed, 2 Oct 2024 00:29:58 +0200 Subject: [PATCH 23/27] feat: add protocol and address filtering to delegated routing api (#678) * feat: add filtering on client * refactor: abstract add filters to url function * feat: add client filtering to FindPeers * test: test filtering in findPeers --------- Co-authored-by: Daniel N <2color@users.noreply.github.com> Co-authored-by: gammazero <11790789+gammazero@users.noreply.github.com> --- CHANGELOG.md | 4 +- routing/http/client/client.go | 61 ++++++++++++++++++- routing/http/client/client_test.go | 59 +++++++++++++----- routing/http/{server => filters}/filters.go | 56 +++++++++++++++-- .../http/{server => filters}/filters_test.go | 55 ++++++++++++++++- routing/http/server/server.go | 39 +++--------- routing/http/server/server_test.go | 22 ++----- 7 files changed, 225 insertions(+), 71 deletions(-) rename routing/http/{server => filters}/filters.go (78%) rename routing/http/{server => filters}/filters_test.go (82%) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee78a8b19..1e6d468f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,9 @@ The following emojis are used to highlight certain changes: * `boxo/bitswap/server`: * A new [`WithWantHaveReplaceSize(n)`](https://pkg.go.dev/github.com/ipfs/boxo/bitswap/server/#WithWantHaveReplaceSize) option can be used with `bitswap.New` to fine-tune cost-vs-performance. It sets the maximum size of a block in bytes up to which the bitswap server will replace a WantHave with a WantBlock response. Setting this to 0 disables this WantHave replacement and means that block sizes are not read when processing WantHave requests. [#672](https://github.com/ipfs/boxo/pull/672) -- `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) +* `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) [#678](https://github.com/ipfs/boxo/pull/678) + * added support for address and protocol filtering to the delegated routing client ([IPIP-484](https://github.com/ipfs/specs/pull/484)) [#678](https://github.com/ipfs/boxo/pull/678). To add filtering to the client, use the [`WithFilterAddrs`](https://pkg.go.dev/github.com/ipfs/boxo/routing/http/client#WithFilterAddrs) and [`WithFilterProtocols`](https://pkg.go.dev/github.com/ipfs/boxo/routing/http/client#WithFilterProtocols) options when creating the client.Client-side filtering for servers that don't support filtering is enabled by default. To disable it, use the [`disableLocalFiltering`](https://pkg.go.dev/github.com/ipfs/boxo/routing/http/client#disableLocalFiltering) option when creating the client. ### Changed diff --git a/routing/http/client/client.go b/routing/http/client/client.go index 16840cab5..9b85a5066 100644 --- a/routing/http/client/client.go +++ b/routing/http/client/client.go @@ -9,12 +9,15 @@ import ( "io" "mime" "net/http" + gourl "net/url" + "sort" "strings" "time" "github.com/benbjohnson/clock" ipns "github.com/ipfs/boxo/ipns" "github.com/ipfs/boxo/routing/http/contentrouter" + "github.com/ipfs/boxo/routing/http/filters" "github.com/ipfs/boxo/routing/http/internal/drjson" "github.com/ipfs/boxo/routing/http/types" "github.com/ipfs/boxo/routing/http/types/iter" @@ -52,6 +55,11 @@ type Client struct { // for testing, e.g., testing the server with a mangled signature. //lint:ignore SA1019 // ignore staticcheck afterSignCallback func(req *types.WriteBitswapRecord) + + // disableLocalFiltering is used to disable local filtering of the results + disableLocalFiltering bool + protocolFilter []string + addrFilter []string } // defaultUserAgent is used as a fallback to inform HTTP server which library @@ -83,6 +91,37 @@ func WithIdentity(identity crypto.PrivKey) Option { } } +// WithDisabledLocalFiltering disables local filtering of the results. +// This should be used for delegated routing servers that already implement filtering +func WithDisabledLocalFiltering(val bool) Option { + return func(c *Client) error { + c.disableLocalFiltering = val + return nil + } +} + +// WithProtocolFilter adds a protocol filter to the client. +// The protocol filter is added to the request URL. +// The protocols are ordered alphabetically for cache key (url) consistency +func WithProtocolFilter(protocolFilter []string) Option { + return func(c *Client) error { + sort.Strings(protocolFilter) + c.protocolFilter = protocolFilter + return nil + } +} + +// WithAddrFilter adds an address filter to the client. +// The address filter is added to the request URL. +// The addresses are ordered alphabetically for cache key (url) consistency +func WithAddrFilter(addrFilter []string) Option { + return func(c *Client) error { + sort.Strings(addrFilter) + c.addrFilter = addrFilter + return nil + } +} + // WithHTTPClient sets a custom HTTP Client to be used with [Client]. func WithHTTPClient(h httpClient) Option { return func(c *Client) error { @@ -184,7 +223,12 @@ func (c *Client) FindProviders(ctx context.Context, key cid.Cid) (providers iter // TODO test measurements m := newMeasurement("FindProviders") - url := c.baseURL + "/routing/v1/providers/" + key.String() + url, err := gourl.JoinPath(c.baseURL, "routing/v1/providers", key.String()) + if err != nil { + return nil, err + } + url = filters.AddFiltersToURL(url, c.protocolFilter, c.addrFilter) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return nil, err @@ -251,6 +295,10 @@ func (c *Client) FindProviders(ctx context.Context, key cid.Cid) (providers iter return nil, errors.New("unknown content type") } + if !c.disableLocalFiltering { + it = filters.ApplyFiltersToIter(it, c.addrFilter, c.protocolFilter) + } + return &measuringIter[iter.Result[types.Record]]{Iter: it, ctx: ctx, m: m}, nil } @@ -356,7 +404,12 @@ func (c *Client) provideSignedBitswapRecord(ctx context.Context, bswp *types.Wri func (c *Client) FindPeers(ctx context.Context, pid peer.ID) (peers iter.ResultIter[*types.PeerRecord], err error) { m := newMeasurement("FindPeers") - url := c.baseURL + "/routing/v1/peers/" + peer.ToCid(pid).String() + url, err := gourl.JoinPath(c.baseURL, "routing/v1/peers", peer.ToCid(pid).String()) + if err != nil { + return nil, err + } + url = filters.AddFiltersToURL(url, c.protocolFilter, c.addrFilter) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return nil, err @@ -423,6 +476,10 @@ func (c *Client) FindPeers(ctx context.Context, pid peer.ID) (peers iter.ResultI return nil, errors.New("unknown content type") } + if !c.disableLocalFiltering { + it = filters.ApplyFiltersToPeerRecordIter(it, c.addrFilter, c.protocolFilter) + } + return &measuringIter[iter.Result[*types.PeerRecord]]{Iter: it, ctx: ctx, m: m}, nil } diff --git a/routing/http/client/client_test.go b/routing/http/client/client_test.go index 732861797..0b5dc29f7 100644 --- a/routing/http/client/client_test.go +++ b/routing/http/client/client_test.go @@ -49,7 +49,8 @@ func (m *mockContentRouter) FindPeers(ctx context.Context, pid peer.ID, limit in func (m *mockContentRouter) GetIPNS(ctx context.Context, name ipns.Name) (*ipns.Record, error) { args := m.Called(ctx, name) - return args.Get(0).(*ipns.Record), args.Error(1) + rec, _ := args.Get(0).(*ipns.Record) + return rec, args.Error(1) } func (m *mockContentRouter) PutIPNS(ctx context.Context, name ipns.Name, record *ipns.Record) error { @@ -158,12 +159,12 @@ func addrsToDRAddrs(addrs []multiaddr.Multiaddr) (drmas []types.Multiaddr) { return } -func makePeerRecord() types.PeerRecord { +func makePeerRecord(protocols []string) types.PeerRecord { peerID, addrs, _ := makeProviderAndIdentity() return types.PeerRecord{ Schema: types.SchemaPeer, ID: &peerID, - Protocols: []string{"transport-bitswap"}, + Protocols: protocols, Addrs: addrsToDRAddrs(addrs), Extra: map[string]json.RawMessage{}, } @@ -196,7 +197,7 @@ func makeProviderAndIdentity() (peer.ID, []multiaddr.Multiaddr, crypto.PrivKey) panic(err) } - ma2, err := multiaddr.NewMultiaddr("/ip4/0.0.0.0/tcp/4002") + ma2, err := multiaddr.NewMultiaddr("/ip4/0.0.0.0/udp/4002") if err != nil { panic(err) } @@ -222,9 +223,11 @@ func (e *osErrContains) errContains(t *testing.T, err error) { } func TestClient_FindProviders(t *testing.T) { - peerRecord := makePeerRecord() + bitswapPeerRecord := makePeerRecord([]string{"transport-bitswap"}) + httpPeerRecord := makePeerRecord([]string{"transport-ipfs-gateway-http"}) peerProviders := []iter.Result[types.Record]{ - {Val: &peerRecord}, + {Val: &bitswapPeerRecord}, + {Val: &httpPeerRecord}, } bitswapRecord := makeBitswapRecord() @@ -238,6 +241,7 @@ func TestClient_FindProviders(t *testing.T) { routerErr error clientRequiresStreaming bool serverStreamingDisabled bool + filterProtocols []string expErrContains osErrContains expResult []iter.Result[types.Record] @@ -250,6 +254,13 @@ func TestClient_FindProviders(t *testing.T) { expResult: peerProviders, expStreamingResponse: true, }, + { + name: "happy case with protocol filter", + filterProtocols: []string{"transport-bitswap"}, + routerResult: peerProviders, + expResult: []iter.Result[types.Record]{{Val: &bitswapPeerRecord}}, + expStreamingResponse: true, + }, { name: "happy case (with deprecated bitswap schema)", routerResult: []iter.Result[types.Record]{{Val: &bitswapRecord}}, @@ -305,6 +316,10 @@ func TestClient_FindProviders(t *testing.T) { }) } + if c.filterProtocols != nil { + clientOpts = append(clientOpts, WithProtocolFilter(c.filterProtocols)) + } + if c.expStreamingResponse { onRespReceived = append(onRespReceived, func(r *http.Response) { assert.Equal(t, mediaTypeNDJSON, r.Header.Get("Content-Type")) @@ -482,11 +497,13 @@ func TestClient_Provide(t *testing.T) { } func TestClient_FindPeers(t *testing.T) { - peerRecord := makePeerRecord() + peerRecord1 := makePeerRecord([]string{"transport-bitswap"}) + peerRecord2 := makePeerRecord([]string{"transport-ipfs-gateway-http"}) peerRecords := []iter.Result[*types.PeerRecord]{ - {Val: &peerRecord}, + {Val: &peerRecord1}, + {Val: &peerRecord2}, } - pid := *peerRecord.ID + pid := *peerRecord1.ID cases := []struct { name string @@ -496,6 +513,7 @@ func TestClient_FindPeers(t *testing.T) { routerErr error clientRequiresStreaming bool serverStreamingDisabled bool + filterProtocols []string expErrContains osErrContains expResult []iter.Result[*types.PeerRecord] @@ -508,6 +526,13 @@ func TestClient_FindPeers(t *testing.T) { expResult: peerRecords, expStreamingResponse: true, }, + { + name: "happy case with protocol filter", + filterProtocols: []string{"transport-bitswap"}, + routerResult: peerRecords, + expResult: []iter.Result[*types.PeerRecord]{{Val: &peerRecord1}}, + expStreamingResponse: true, + }, { name: "server doesn't support streaming", routerResult: peerRecords, @@ -542,12 +567,10 @@ func TestClient_FindPeers(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - var ( - clientOpts []Option - serverOpts []server.Option - onRespReceived []func(*http.Response) - onReqReceived []func(*http.Request) - ) + var clientOpts []Option + var serverOpts []server.Option + var onRespReceived []func(*http.Response) + var onReqReceived []func(*http.Request) if c.serverStreamingDisabled { serverOpts = append(serverOpts, server.WithStreamingResultsDisabled()) @@ -560,6 +583,10 @@ func TestClient_FindPeers(t *testing.T) { }) } + if c.filterProtocols != nil { + clientOpts = append(clientOpts, WithProtocolFilter(c.filterProtocols)) + } + if c.expStreamingResponse { onRespReceived = append(onRespReceived, func(r *http.Response) { assert.Equal(t, mediaTypeNDJSON, r.Header.Get("Content-Type")) @@ -603,7 +630,7 @@ func TestClient_FindPeers(t *testing.T) { resultIter, err := client.FindPeers(ctx, pid) c.expErrContains.errContains(t, err) - results := iter.ReadAll[iter.Result[*types.PeerRecord]](resultIter) + results := iter.ReadAll(resultIter) assert.Equal(t, c.expResult, results) }) } diff --git a/routing/http/server/filters.go b/routing/http/filters/filters.go similarity index 78% rename from routing/http/server/filters.go rename to routing/http/filters/filters.go index bb5dfa0d5..ae7aad18f 100644 --- a/routing/http/server/filters.go +++ b/routing/http/filters/filters.go @@ -1,24 +1,48 @@ -package server +package filters import ( + "net/url" "reflect" "slices" "strings" "github.com/ipfs/boxo/routing/http/types" "github.com/ipfs/boxo/routing/http/types/iter" + logging "github.com/ipfs/go-log/v2" "github.com/multiformats/go-multiaddr" ) -// filters implements IPIP-0484 +var logger = logging.Logger("routing/http/filters") -func parseFilter(param string) []string { +// Package filters implements IPIP-0484 + +func ParseFilter(param string) []string { if param == "" { return nil } return strings.Split(strings.ToLower(param), ",") } +func AddFiltersToURL(baseURL string, protocolFilter, addrFilter []string) string { + parsedURL, err := url.Parse(baseURL) + if err != nil { + return baseURL + } + + query := parsedURL.Query() + + if len(protocolFilter) > 0 { + query.Set("filter-protocols", strings.Join(protocolFilter, ",")) + } + + if len(addrFilter) > 0 { + query.Set("filter-addrs", strings.Join(addrFilter, ",")) + } + + parsedURL.RawQuery = query.Encode() + return parsedURL.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. @@ -28,7 +52,7 @@ func parseFilter(param string) []string { // - 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] { +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 @@ -76,6 +100,30 @@ func applyFiltersToIter(recordsIter iter.ResultIter[types.Record], filterAddrs, return filteredIter } +func ApplyFiltersToPeerRecordIter(peerRecordIter iter.ResultIter[*types.PeerRecord], filterAddrs, filterProtocols []string) iter.ResultIter[*types.PeerRecord] { + // Convert PeerRecord to Record so that we can reuse the filtering logic from findProviders + mappedIter := iter.Map(peerRecordIter, 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) + + // Convert Record back to PeerRecord 🙃 + return 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} + }) +} + // 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 { diff --git a/routing/http/server/filters_test.go b/routing/http/filters/filters_test.go similarity index 82% rename from routing/http/server/filters_test.go rename to routing/http/filters/filters_test.go index 078e4aa96..ac6219bd7 100644 --- a/routing/http/server/filters_test.go +++ b/routing/http/filters/filters_test.go @@ -1,4 +1,4 @@ -package server +package filters import ( "testing" @@ -10,6 +10,59 @@ import ( "github.com/stretchr/testify/require" ) +func TestAddFiltersToURL(t *testing.T) { + testCases := []struct { + name string + baseURL string + protocolFilter []string + addrFilter []string + expected string + }{ + { + name: "No filters", + baseURL: "https://example.com/routing/v1/providers/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi", + protocolFilter: nil, + addrFilter: nil, + expected: "https://example.com/routing/v1/providers/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi", + }, + { + name: "Only protocol filter", + baseURL: "https://example.com/routing/v1/providers/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi", + protocolFilter: []string{"transport-bitswap", "transport-ipfs-gateway-http"}, + addrFilter: nil, + expected: "https://example.com/routing/v1/providers/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi?filter-protocols=transport-bitswap%2Ctransport-ipfs-gateway-http", + }, + { + name: "Only addr filter", + baseURL: "https://example.com/routing/v1/providers/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi", + protocolFilter: nil, + addrFilter: []string{"ip4", "ip6"}, + expected: "https://example.com/routing/v1/providers/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi?filter-addrs=ip4%2Cip6", + }, + { + name: "Both filters", + baseURL: "https://example.com/routing/v1/providers/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi", + protocolFilter: []string{"transport-bitswap", "transport-graphsync-filecoinv1"}, + addrFilter: []string{"ip4", "ip6"}, + expected: "https://example.com/routing/v1/providers/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi?filter-addrs=ip4%2Cip6&filter-protocols=transport-bitswap%2Ctransport-graphsync-filecoinv1", + }, + { + name: "URL with existing query parameters", + baseURL: "https://example.com/routing/v1/providers/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi?existing=param", + protocolFilter: []string{"transport-bitswap"}, + addrFilter: []string{"ip4"}, + expected: "https://example.com/routing/v1/providers/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi?existing=param&filter-addrs=ip4&filter-protocols=transport-bitswap", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := AddFiltersToURL(tc.baseURL, tc.protocolFilter, tc.addrFilter) + assert.Equal(t, tc.expected, result) + }) + } +} + func TestApplyAddrFilter(t *testing.T) { // Create some test multiaddrs addr1, _ := multiaddr.NewMultiaddr("/ip4/127.0.0.1/tcp/4001/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt") diff --git a/routing/http/server/server.go b/routing/http/server/server.go index 3cdcefee0..a7f9385b6 100644 --- a/routing/http/server/server.go +++ b/routing/http/server/server.go @@ -15,6 +15,7 @@ import ( "github.com/cespare/xxhash/v2" "github.com/gorilla/mux" "github.com/ipfs/boxo/ipns" + "github.com/ipfs/boxo/routing/http/filters" "github.com/ipfs/boxo/routing/http/internal/drjson" "github.com/ipfs/boxo/routing/http/types" "github.com/ipfs/boxo/routing/http/types/iter" @@ -196,8 +197,8 @@ func (s *server) findProviders(w http.ResponseWriter, httpReq *http.Request) { // Parse query parameters query := httpReq.URL.Query() - filterAddrs := parseFilter(query.Get("filter-addrs")) - filterProtocols := parseFilter(query.Get("filter-protocols")) + filterAddrs := filters.ParseFilter(query.Get("filter-addrs")) + filterProtocols := filters.ParseFilter(query.Get("filter-protocols")) mediaType, err := s.detectResponseType(httpReq) if err != nil { @@ -235,7 +236,7 @@ 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() - filteredIter := applyFiltersToIter(provIter, filterAddrs, filterProtocols) + filteredIter := filters.ApplyFiltersToIter(provIter, filterAddrs, filterProtocols) providers, err := iter.ReadAllResults(filteredIter) if err != nil { writeErr(w, "FindProviders", http.StatusInternalServerError, fmt.Errorf("delegate error: %w", err)) @@ -247,7 +248,7 @@ func (s *server) findProvidersJSON(w http.ResponseWriter, provIter iter.ResultIt }) } func (s *server) findProvidersNDJSON(w http.ResponseWriter, provIter iter.ResultIter[types.Record], filterAddrs, filterProtocols []string) { - filteredIter := applyFiltersToIter(provIter, filterAddrs, filterProtocols) + filteredIter := filters.ApplyFiltersToIter(provIter, filterAddrs, filterProtocols) writeResultsIterNDJSON(w, filteredIter) } @@ -285,8 +286,8 @@ func (s *server) findPeers(w http.ResponseWriter, r *http.Request) { } query := r.URL.Query() - filterAddrs := parseFilter(query.Get("filter-addrs")) - filterProtocols := parseFilter(query.Get("filter-protocols")) + filterAddrs := filters.ParseFilter(query.Get("filter-addrs")) + filterProtocols := filters.ParseFilter(query.Get("filter-protocols")) mediaType, err := s.detectResponseType(r) if err != nil { @@ -383,29 +384,9 @@ func (s *server) provide(w http.ResponseWriter, httpReq *http.Request) { func (s *server) findPeersJSON(w http.ResponseWriter, peersIter iter.ResultIter[*types.PeerRecord], filterAddrs, filterProtocols []string) { defer peersIter.Close() - // 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) - - // 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} - }) + peersIter = filters.ApplyFiltersToPeerRecordIter(peersIter, filterAddrs, filterProtocols) - peers, err := iter.ReadAllResults(finalIter) + peers, err := iter.ReadAllResults(peersIter) if err != nil { writeErr(w, "FindPeers", http.StatusInternalServerError, fmt.Errorf("delegate error: %w", err)) @@ -428,7 +409,7 @@ func (s *server) findPeersNDJSON(w http.ResponseWriter, peersIter iter.ResultIte return iter.Result[types.Record]{Val: record} }) - filteredIter := applyFiltersToIter(mappedIter, filterAddrs, filterProtocols) + filteredIter := filters.ApplyFiltersToIter(mappedIter, filterAddrs, filterProtocols) writeResultsIterNDJSON(w, filteredIter) } diff --git a/routing/http/server/server_test.go b/routing/http/server/server_test.go index 772f79999..bf84e4155 100644 --- a/routing/http/server/server_test.go +++ b/routing/http/server/server_test.go @@ -16,6 +16,7 @@ import ( "github.com/ipfs/boxo/ipns" "github.com/ipfs/boxo/path" + "github.com/ipfs/boxo/routing/http/filters" "github.com/ipfs/boxo/routing/http/types" "github.com/ipfs/boxo/routing/http/types/iter" "github.com/ipfs/go-cid" @@ -153,15 +154,7 @@ func TestProviders(t *testing.T) { router.On("FindProviders", mock.Anything, cid, limit).Return(results, nil) 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) - } - } + urlStr = filters.AddFiltersToURL(urlStr, strings.Split(filterProtocols, ","), strings.Split(filterAddrs, ",")) req, err := http.NewRequest(http.MethodGet, urlStr, nil) require.NoError(t, err) @@ -273,15 +266,8 @@ func TestPeers(t *testing.T) { t.Cleanup(server.Close) 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) - } - } + urlStr = filters.AddFiltersToURL(urlStr, strings.Split(filterProtocols, ","), strings.Split(filterAddrs, ",")) + req, err := http.NewRequest(http.MethodGet, urlStr, nil) require.NoError(t, err) if contentType != "" { From d62e0311889f44e00a49dcbd68a5041a706ba0fb Mon Sep 17 00:00:00 2001 From: Andrew Gillis <11790789+gammazero@users.noreply.github.com> Date: Wed, 2 Oct 2024 23:01:37 -0700 Subject: [PATCH 24/27] Update to latest go-libp2p (#681) * Update to latest go-libp2p --- examples/go.mod | 2 +- examples/go.sum | 4 ++-- go.mod | 2 +- go.sum | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/go.mod b/examples/go.mod index fd77bc07e..d7a4a8433 100644 --- a/examples/go.mod +++ b/examples/go.mod @@ -9,7 +9,7 @@ require ( github.com/ipfs/go-datastore v0.6.0 github.com/ipld/go-car/v2 v2.13.1 github.com/ipld/go-ipld-prime v0.21.0 - github.com/libp2p/go-libp2p v0.36.3 + github.com/libp2p/go-libp2p v0.36.4 github.com/libp2p/go-libp2p-routing-helpers v0.7.3 github.com/multiformats/go-multiaddr v0.13.0 github.com/multiformats/go-multicodec v0.9.0 diff --git a/examples/go.sum b/examples/go.sum index f8d2600ad..cf8e4f89a 100644 --- a/examples/go.sum +++ b/examples/go.sum @@ -271,8 +271,8 @@ github.com/libp2p/go-doh-resolver v0.4.0 h1:gUBa1f1XsPwtpE1du0O+nnZCUqtG7oYi7Bb+ github.com/libp2p/go-doh-resolver v0.4.0/go.mod h1:v1/jwsFusgsWIGX/c6vCRrnJ60x7bhTiq/fs2qt0cAg= github.com/libp2p/go-flow-metrics v0.1.0 h1:0iPhMI8PskQwzh57jB9WxIuIOQ0r+15PChFGkx3Q3WM= github.com/libp2p/go-flow-metrics v0.1.0/go.mod h1:4Xi8MX8wj5aWNDAZttg6UPmc0ZrnFNsMtpsYUClFtro= -github.com/libp2p/go-libp2p v0.36.3 h1:NHz30+G7D8Y8YmznrVZZla0ofVANrvBl2c+oARfMeDQ= -github.com/libp2p/go-libp2p v0.36.3/go.mod h1:4Y5vFyCUiJuluEPmpnKYf6WFx5ViKPUYs/ixe9ANFZ8= +github.com/libp2p/go-libp2p v0.36.4 h1:ZaKyKSHBFbzs6CnAYMhaMc5QgV1UoCN+9WXrg8SEwI4= +github.com/libp2p/go-libp2p v0.36.4/go.mod h1:4Y5vFyCUiJuluEPmpnKYf6WFx5ViKPUYs/ixe9ANFZ8= github.com/libp2p/go-libp2p-asn-util v0.4.1 h1:xqL7++IKD9TBFMgnLPZR6/6iYhawHKHl950SO9L6n94= github.com/libp2p/go-libp2p-asn-util v0.4.1/go.mod h1:d/NI6XZ9qxw67b4e+NgpQexCIiFYJjErASrYW4PFDN8= github.com/libp2p/go-libp2p-kad-dht v0.25.2 h1:FOIk9gHoe4YRWXTu8SY9Z1d0RILol0TrtApsMDPjAVQ= diff --git a/go.mod b/go.mod index a324e8b71..840bd3151 100644 --- a/go.mod +++ b/go.mod @@ -38,7 +38,7 @@ require ( github.com/jbenet/goprocess v0.1.4 github.com/libp2p/go-buffer-pool v0.1.0 github.com/libp2p/go-doh-resolver v0.4.0 - github.com/libp2p/go-libp2p v0.36.3 + github.com/libp2p/go-libp2p v0.36.4 github.com/libp2p/go-libp2p-kad-dht v0.25.2 github.com/libp2p/go-libp2p-record v0.2.0 github.com/libp2p/go-libp2p-routing-helpers v0.7.3 diff --git a/go.sum b/go.sum index b68267e3f..88c97bdd2 100644 --- a/go.sum +++ b/go.sum @@ -274,8 +274,8 @@ github.com/libp2p/go-doh-resolver v0.4.0 h1:gUBa1f1XsPwtpE1du0O+nnZCUqtG7oYi7Bb+ github.com/libp2p/go-doh-resolver v0.4.0/go.mod h1:v1/jwsFusgsWIGX/c6vCRrnJ60x7bhTiq/fs2qt0cAg= github.com/libp2p/go-flow-metrics v0.1.0 h1:0iPhMI8PskQwzh57jB9WxIuIOQ0r+15PChFGkx3Q3WM= github.com/libp2p/go-flow-metrics v0.1.0/go.mod h1:4Xi8MX8wj5aWNDAZttg6UPmc0ZrnFNsMtpsYUClFtro= -github.com/libp2p/go-libp2p v0.36.3 h1:NHz30+G7D8Y8YmznrVZZla0ofVANrvBl2c+oARfMeDQ= -github.com/libp2p/go-libp2p v0.36.3/go.mod h1:4Y5vFyCUiJuluEPmpnKYf6WFx5ViKPUYs/ixe9ANFZ8= +github.com/libp2p/go-libp2p v0.36.4 h1:ZaKyKSHBFbzs6CnAYMhaMc5QgV1UoCN+9WXrg8SEwI4= +github.com/libp2p/go-libp2p v0.36.4/go.mod h1:4Y5vFyCUiJuluEPmpnKYf6WFx5ViKPUYs/ixe9ANFZ8= github.com/libp2p/go-libp2p-asn-util v0.4.1 h1:xqL7++IKD9TBFMgnLPZR6/6iYhawHKHl950SO9L6n94= github.com/libp2p/go-libp2p-asn-util v0.4.1/go.mod h1:d/NI6XZ9qxw67b4e+NgpQexCIiFYJjErASrYW4PFDN8= github.com/libp2p/go-libp2p-kad-dht v0.25.2 h1:FOIk9gHoe4YRWXTu8SY9Z1d0RILol0TrtApsMDPjAVQ= From f61a371459d7823332c3eebeb5d5923fb23df11f Mon Sep 17 00:00:00 2001 From: gammazero <11790789+gammazero@users.noreply.github.com> Date: Wed, 2 Oct 2024 23:08:55 -0700 Subject: [PATCH 25/27] update version --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index 93d6ca712..ee0e5814d 100644 --- a/version.json +++ b/version.json @@ -1,3 +1,3 @@ { - "version": "v0.23.0" + "version": "v0.24.0" } From 8464618419a53d23ed2f13f0f2ce5dca73c08ad1 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Thu, 3 Oct 2024 19:08:22 +0200 Subject: [PATCH 26/27] docs(changelog): v0.24.0 --- CHANGELOG.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e6d468f4..7190373ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,18 @@ The following emojis are used to highlight certain changes: ### Added +### Changed + +### Removed + +### Fixed + +### Security + +## [v0.24.0] + +### Added + * `boxo/bitswap/server`: * A new [`WithWantHaveReplaceSize(n)`](https://pkg.go.dev/github.com/ipfs/boxo/bitswap/server/#WithWantHaveReplaceSize) option can be used with `bitswap.New` to fine-tune cost-vs-performance. It sets the maximum size of a block in bytes up to which the bitswap server will replace a WantHave with a WantBlock response. Setting this to 0 disables this WantHave replacement and means that block sizes are not read when processing WantHave requests. [#672](https://github.com/ipfs/boxo/pull/672) * `routing/http`: @@ -27,7 +39,8 @@ The following emojis are used to highlight certain changes: ### Removed ### Fixed -= `unixfs/hamt` Log error instead of panic if both link and shard are nil [#393](https://github.com/ipfs/boxo/pull/393) + +- `unixfs/hamt` Log error instead of panic if both link and shard are nil [#393](https://github.com/ipfs/boxo/pull/393) ### Security From eac6c25481c4645ec24f05576d6b8ef276864331 Mon Sep 17 00:00:00 2001 From: Andrew Gillis <11790789+gammazero@users.noreply.github.com> Date: Thu, 3 Oct 2024 11:52:55 -0700 Subject: [PATCH 27/27] chore: update go-multiaddr-dns (#684) (cherry picked from commit a2fc8e7f6e7026c4cc8870a9cdf4126f004b5792) --- examples/go.mod | 2 +- examples/go.sum | 4 ++-- go.mod | 2 +- go.sum | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/go.mod b/examples/go.mod index d7a4a8433..772c389a8 100644 --- a/examples/go.mod +++ b/examples/go.mod @@ -107,7 +107,7 @@ require ( github.com/mr-tron/base58 v1.2.0 // indirect github.com/multiformats/go-base32 v0.1.0 // indirect github.com/multiformats/go-base36 v0.2.0 // indirect - github.com/multiformats/go-multiaddr-dns v0.3.1 // indirect + github.com/multiformats/go-multiaddr-dns v0.4.0 // indirect github.com/multiformats/go-multiaddr-fmt v0.1.0 // indirect github.com/multiformats/go-multibase v0.2.0 // indirect github.com/multiformats/go-multihash v0.2.3 // indirect diff --git a/examples/go.sum b/examples/go.sum index cf8e4f89a..d212ad06c 100644 --- a/examples/go.sum +++ b/examples/go.sum @@ -331,8 +331,8 @@ github.com/multiformats/go-multiaddr v0.2.0/go.mod h1:0nO36NvPpyV4QzvTLi/lafl2y9 github.com/multiformats/go-multiaddr v0.13.0 h1:BCBzs61E3AGHcYYTv8dqRH43ZfyrqM8RXVPT8t13tLQ= github.com/multiformats/go-multiaddr v0.13.0/go.mod h1:sBXrNzucqkFJhvKOiwwLyqamGa/P5EIXNPLovyhQCII= github.com/multiformats/go-multiaddr-dns v0.3.0/go.mod h1:mNzQ4eTGDg0ll1N9jKPOUogZPoJ30W8a7zk66FQPpdQ= -github.com/multiformats/go-multiaddr-dns v0.3.1 h1:QgQgR+LQVt3NPTjbrLLpsaT2ufAA2y0Mkk+QRVJbW3A= -github.com/multiformats/go-multiaddr-dns v0.3.1/go.mod h1:G/245BRQ6FJGmryJCrOuTdB37AMA5AMOVuO6NY3JwTk= +github.com/multiformats/go-multiaddr-dns v0.4.0 h1:P76EJ3qzBXpUXZ3twdCDx/kvagMsNo0LMFXpyms/zgU= +github.com/multiformats/go-multiaddr-dns v0.4.0/go.mod h1:7hfthtB4E4pQwirrz+J0CcDUfbWzTqEzVyYKKIKpgkc= github.com/multiformats/go-multiaddr-fmt v0.1.0 h1:WLEFClPycPkp4fnIzoFoV9FVd49/eQsuaL3/CWe167E= github.com/multiformats/go-multiaddr-fmt v0.1.0/go.mod h1:hGtDIW4PU4BqJ50gW2quDuPVjyWNZxToGUh/HwTZYJo= github.com/multiformats/go-multibase v0.2.0 h1:isdYCVLvksgWlMW9OZRYJEa9pZETFivncJHmHnnd87g= diff --git a/go.mod b/go.mod index 840bd3151..4d4653804 100644 --- a/go.mod +++ b/go.mod @@ -48,7 +48,7 @@ require ( github.com/mr-tron/base58 v1.2.0 github.com/multiformats/go-base32 v0.1.0 github.com/multiformats/go-multiaddr v0.13.0 - github.com/multiformats/go-multiaddr-dns v0.3.1 + github.com/multiformats/go-multiaddr-dns v0.4.0 github.com/multiformats/go-multibase v0.2.0 github.com/multiformats/go-multicodec v0.9.0 github.com/multiformats/go-multihash v0.2.3 diff --git a/go.sum b/go.sum index 88c97bdd2..da17cedae 100644 --- a/go.sum +++ b/go.sum @@ -334,8 +334,8 @@ github.com/multiformats/go-multiaddr v0.2.0/go.mod h1:0nO36NvPpyV4QzvTLi/lafl2y9 github.com/multiformats/go-multiaddr v0.13.0 h1:BCBzs61E3AGHcYYTv8dqRH43ZfyrqM8RXVPT8t13tLQ= github.com/multiformats/go-multiaddr v0.13.0/go.mod h1:sBXrNzucqkFJhvKOiwwLyqamGa/P5EIXNPLovyhQCII= github.com/multiformats/go-multiaddr-dns v0.3.0/go.mod h1:mNzQ4eTGDg0ll1N9jKPOUogZPoJ30W8a7zk66FQPpdQ= -github.com/multiformats/go-multiaddr-dns v0.3.1 h1:QgQgR+LQVt3NPTjbrLLpsaT2ufAA2y0Mkk+QRVJbW3A= -github.com/multiformats/go-multiaddr-dns v0.3.1/go.mod h1:G/245BRQ6FJGmryJCrOuTdB37AMA5AMOVuO6NY3JwTk= +github.com/multiformats/go-multiaddr-dns v0.4.0 h1:P76EJ3qzBXpUXZ3twdCDx/kvagMsNo0LMFXpyms/zgU= +github.com/multiformats/go-multiaddr-dns v0.4.0/go.mod h1:7hfthtB4E4pQwirrz+J0CcDUfbWzTqEzVyYKKIKpgkc= github.com/multiformats/go-multiaddr-fmt v0.1.0 h1:WLEFClPycPkp4fnIzoFoV9FVd49/eQsuaL3/CWe167E= github.com/multiformats/go-multiaddr-fmt v0.1.0/go.mod h1:hGtDIW4PU4BqJ50gW2quDuPVjyWNZxToGUh/HwTZYJo= github.com/multiformats/go-multibase v0.2.0 h1:isdYCVLvksgWlMW9OZRYJEa9pZETFivncJHmHnnd87g=