Fix NetworkFilter keys (#555)

Author: atuchin-m

src/cosmetic_filter_cache.rs

@@ -262,7 +262,7 @@ impl CosmeticFilterCache {
         for hash in hashes.iter() {
             // Handle top-level hide selectors
             if let Some(hide_iterator) = hostname_hide_view.get(**hash) {
-                for (_, hide_selector) in hide_iterator {
+                for hide_selector in hide_iterator {
                     if !exceptions.contains(hide_selector) {
                         specific_hide_selectors.insert(hide_selector.to_owned());
                     }
@@ -271,7 +271,7 @@ impl CosmeticFilterCache {
 
             // Handle top-level inject scripts with encoded permissions
             if let Some(script_iterator) = hostname_inject_script_view.get(**hash) {
-                for (_, encoded_script) in script_iterator {
+                for encoded_script in script_iterator {
                     let (permission, script) = decode_script_with_permission(encoded_script);
                     script_injections
                         .entry(script)

src/filters/fb_network.rs

@@ -74,12 +74,15 @@ impl<'a> FlatNetworkFilter<'a> {
     #[inline(always)]
     pub fn new(
         filter: &'a fb::NetworkFilter<'a>,
-        index: usize,
         filter_data_context: &'a FilterDataContext,
     ) -> Self {
+        // Use the flatbuffer relative location as key, it's unique for
+        // each filter regardless of the filter list it belongs to.
+        let key = filter._tab.loc() as u64;
+
         Self {
+            key,
             fb_filter: filter,
-            key: index as u64,
             mask: NetworkFilterMask::from_bits_retain(filter.mask()),
             filter_data_context,
         }

src/flatbuffers/containers/flat_multimap.rs

@@ -71,12 +71,12 @@ where
     V: Follow<'a>,
     Keys: SortedIndex<I>,
 {
-    type Item = (usize, <V as Follow<'a>>::Inner);
+    type Item = <V as Follow<'a>>::Inner;
 
     fn next(&mut self) -> Option<Self::Item> {
         if self.index < self.keys.len() && self.keys.get(self.index) == self.key {
             self.index += 1;
-            Some((self.index - 1, self.values.get(self.index - 1)))
+            Some(self.values.get(self.index - 1))
         } else {
             None
         }

src/network_filter_list.rs

@@ -90,9 +90,8 @@ impl NetworkFilterList<'_> {
 
         for token in request.get_tokens_for_match() {
             if let Some(iter) = filter_map.get(to_short_hash(*token)) {
-                for (index, fb_filter) in iter {
-                    let filter =
-                        FlatNetworkFilter::new(&fb_filter, index, self.filter_data_context);
+                for fb_filter in iter {
+                    let filter = FlatNetworkFilter::new(&fb_filter, self.filter_data_context);
 
                     // if matched, also needs to be tagged with an active tag (or not tagged at all)
                     if filter.matches(request, regex_manager)
@@ -133,9 +132,8 @@ impl NetworkFilterList<'_> {
 
         for token in request.get_tokens_for_match() {
             if let Some(iter) = filter_map.get(to_short_hash(*token)) {
-                for (index, fb_filter) in iter {
-                    let filter =
-                        FlatNetworkFilter::new(&fb_filter, index, self.filter_data_context);
+                for fb_filter in iter {
+                    let filter = FlatNetworkFilter::new(&fb_filter, self.filter_data_context);
 
                     // if matched, also needs to be tagged with an active tag (or not tagged at all)
                     if filter.matches(request, regex_manager)

tests/unit/flatbuffers/containers/flat_multimap.rs

@@ -46,7 +46,7 @@ mod tests {
 
         // Test existing key
         let mut iter = map.get(1).unwrap();
-        assert_eq!(iter.next(), Some((0, 100)));
+        assert_eq!(iter.next(), Some(100));
         assert_eq!(iter.next(), None);
 
         // Test non-existing key
@@ -65,14 +65,14 @@ mod tests {
 
         // Test key with single value
         let mut iter = map.get(3).unwrap();
-        assert_eq!(iter.next(), Some((5, 60)));
+        assert_eq!(iter.next(), Some(60));
         assert_eq!(iter.next(), None);
 
         // Test key with multiple values
         let mut iter = map.get(2).unwrap();
-        assert_eq!(iter.next(), Some((2, 30)));
-        assert_eq!(iter.next(), Some((3, 40)));
-        assert_eq!(iter.next(), Some((4, 50)));
+        assert_eq!(iter.next(), Some(30));
+        assert_eq!(iter.next(), Some(40));
+        assert_eq!(iter.next(), Some(50));
         assert_eq!(iter.next(), None);
 
         // Test non-existing key
@@ -89,9 +89,9 @@ mod tests {
         assert_eq!(map.total_size(), 3);
 
         let mut iter = map.get(5).unwrap();
-        assert_eq!(iter.next(), Some((0, 100)));
-        assert_eq!(iter.next(), Some((1, 200)));
-        assert_eq!(iter.next(), Some((2, 300)));
+        assert_eq!(iter.next(), Some(100));
+        assert_eq!(iter.next(), Some(200));
+        assert_eq!(iter.next(), Some(300));
         assert_eq!(iter.next(), None);
     }
 
@@ -104,9 +104,9 @@ mod tests {
 
         assert_eq!(map.total_size(), 3);
 
-        assert_eq!(map.get(1).unwrap().next(), Some((0, 10)));
-        assert_eq!(map.get(3).unwrap().next(), Some((1, 30)));
-        assert_eq!(map.get(5).unwrap().next(), Some((2, 50)));
+        assert_eq!(map.get(1).unwrap().next(), Some(10));
+        assert_eq!(map.get(3).unwrap().next(), Some(30));
+        assert_eq!(map.get(5).unwrap().next(), Some(50));
         assert!(map.get(2).is_none());
         assert!(map.get(4).is_none());
     }
@@ -149,11 +149,8 @@ mod tests {
         );
 
         assert_eq!(map.total_size(), 3);
-        assert_eq!(map.get(1).unwrap().collect::<Vec<_>>(), vec![(0, 10)]);
-        assert_eq!(
-            map.get(2).unwrap().collect::<Vec<_>>(),
-            vec![(1, 20), (2, 30)]
-        );
+        assert_eq!(map.get(1).unwrap().collect::<Vec<_>>(), vec![10]);
+        assert_eq!(map.get(2).unwrap().collect::<Vec<_>>(), vec![20, 30]);
         assert!(map.get(0).is_none());
         assert!(map.get(3).is_none());
     }
@@ -191,11 +188,8 @@ mod tests {
         let map = FlatMultiMapView::new(flat_map.keys(), flat_map.values());
 
         assert_eq!(map.total_size(), 3);
-        assert_eq!(map.get("a").unwrap().collect::<Vec<_>>(), vec![(0, "10")]);
-        assert_eq!(
-            map.get("b").unwrap().collect::<Vec<_>>(),
-            vec![(1, "20"), (2, "30")]
-        );
+        assert_eq!(map.get("a").unwrap().collect::<Vec<_>>(), vec!["10"]);
+        assert_eq!(map.get("b").unwrap().collect::<Vec<_>>(), vec!["20", "30"]);
         assert!(map.get("c").is_none());
         assert!(map.get("d").is_none());
     }