From c9df436c350a338b572da9756155c3c81e022293 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] 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}},
 		},
 	}