Fix processing bad_filter in into_content_blocking (#559)

Author: atuchin-m

src/content_blocking.rs

@@ -316,9 +316,10 @@ impl TryFrom<NetworkFilter> for CbRuleEquivalent {
             if v.mask.contains(NetworkFilterMask::GENERIC_HIDE) {
                 return Err(CbRuleCreationFailure::NetworkGenerichideUnsupported);
             }
-            if v.mask.contains(NetworkFilterMask::BAD_FILTER) {
-                return Err(CbRuleCreationFailure::NetworkBadFilterUnsupported);
-            }
+            debug_assert!(
+                !v.mask.contains(NetworkFilterMask::BAD_FILTER),
+                "BAD_FILTER should be filtered out"
+            );
             if v.is_csp() {
                 return Err(CbRuleCreationFailure::NetworkCspUnsupported);
             }

src/lists.rs

@@ -285,17 +285,31 @@ impl FilterSet {
         self,
     ) -> Result<(Vec<crate::content_blocking::CbRule>, Vec<String>), ()> {
         use crate::content_blocking;
+        use crate::filters::network::NetworkFilterMaskHelper;
+        use std::collections::HashSet;
 
         if !self.debug {
             return Err(());
         }
 
+        // Store bad filter id to skip them later.
+        let mut bad_filter_ids = HashSet::new();
+        for filter in self.network_filters.iter() {
+            if filter.is_badfilter() {
+                bad_filter_ids.insert(filter.get_id_without_badfilter());
+            }
+        }
+
         let mut ignore_previous_rules = vec![];
         let mut other_rules = vec![];
 
         let mut filters_used = vec![];
 
         self.network_filters.into_iter().for_each(|filter| {
+            // Don't process bad filter rules or matching bad filter rules.
+            if bad_filter_ids.contains(&filter.get_id()) || filter.is_badfilter() {
+                return;
+            }
             let original_rule = *filter
                 .raw_line
                 .clone()

tests/unit/content_blocking.rs

@@ -15,6 +15,17 @@ mod ab2cb_tests {
         );
     }
 
+    fn test_from_abp_multi(abp_rules: &[&str], cb: &str) {
+        let mut filter_set = crate::lists::FilterSet::new(true);
+        filter_set.add_filters(abp_rules, Default::default());
+        let (cb_rules, _) = filter_set.into_content_blocking().unwrap();
+        assert_eq!(
+            cb_rules,
+            serde_json::from_str::<Vec<CbRule>>(cb)
+                .expect("content blocking rules under test could not be deserialized")
+        );
+    }
+
     #[test]
     fn ad_tests() {
         test_from_abp(
@@ -653,6 +664,38 @@ mod ab2cb_tests {
         }]"####,
         );
     }
+
+    #[test]
+    fn badfilter_cancels_matching_rules() {
+        // Test that BAD_FILTER rules cancel out matching rules
+        // Input: regular rule, BAD_FILTER rule, and differently modified rule
+        // Output: only the differently modified rule should remain (BAD_FILTER cancels the exact match)
+        test_from_abp_multi(
+            &[
+                "||example.com^",
+                "||example.com^$badfilter",
+                "||example.com^$third-party",
+            ],
+            r####"[{
+            "action": {
+                "type": "block"
+            },
+            "trigger": {
+                "url-filter": "^[^:]+:(//)?([^/]+\\.)?example\\.com",
+                "load-type": ["third-party"]
+            }
+        }, {
+            "action": {
+                "type": "ignore-previous-rules"
+            },
+            "trigger": {
+                "url-filter": ".*",
+                "resource-type": ["document"],
+                "load-type": ["first-party"]
+            }
+        }]"####,
+        );
+    }
 }
 
 #[cfg(test)]