Handle malformed content-blocking domain metadata

src/content_blocking.rs

@@ -427,53 +427,52 @@ impl TryFrom<NetworkFilter> for CbRuleEquivalent {
                 .to_string(),
             };
 
-            let (if_domain, unless_domain) = if v.opt_domains.is_some()
-                || v.opt_not_domains.is_some()
-            {
-                let mut if_domain = vec![];
-                let mut unless_domain = vec![];
-
-                // Unwraps are okay here - any rules with opt_domains or opt_not_domains must have
-                // an options section delimited by a '$' character, followed by a `domain=` option.
-                let opts = &raw_line[find_char(b'$', raw_line.as_bytes()).unwrap() + "$".len()..];
-                let domain_start_index =
-                    if let Some(index) = memmem::find(opts.as_bytes(), b"domain=") {
-                        index
-                    } else {
+            let (if_domain, unless_domain) =
+                if v.opt_domains.is_some() || v.opt_not_domains.is_some() {
+                    let mut if_domain = vec![];
+                    let mut unless_domain = vec![];
+
+                    let Some(options_index) = find_char(b'$', raw_line.as_bytes()) else {
                         return Err(CbRuleCreationFailure::FromNotSupported);
                     };
-                let domains_start = &opts[domain_start_index + "domain=".len()..];
-                let domains = if let Some(comma) = find_char(b',', domains_start.as_bytes()) {
-                    &domains_start[..comma]
-                } else {
-                    domains_start
-                }
-                .split('|');
-
-                domains.for_each(|domain| {
-                    let (collection, domain) =
-                        if let Some(domain_stripped) = domain.strip_prefix('~') {
-                            (&mut unless_domain, domain_stripped)
+                    let opts = &raw_line[options_index + "$".len()..];
+                    let domain_start_index =
+                        if let Some(index) = memmem::find(opts.as_bytes(), b"domain=") {
+                            index
                         } else {
-                            (&mut if_domain, domain)
+                            return Err(CbRuleCreationFailure::FromNotSupported);
                         };
-
-                    let lowercase = domain.to_lowercase();
-                    let normalized_domain = if lowercase.is_ascii() {
-                        lowercase
+                    let domains_start = &opts[domain_start_index + "domain=".len()..];
+                    let domains = if let Some(comma) = find_char(b',', domains_start.as_bytes()) {
+                        &domains_start[..comma]
                     } else {
-                        // The network filter has already parsed successfully, so this should be
-                        // safe
-                        idna::domain_to_ascii(&lowercase).unwrap()
-                    };
+                        domains_start
+                    }
+                    .split('|');
+
+                    for domain in domains {
+                        let (collection, domain) =
+                            if let Some(domain_stripped) = domain.strip_prefix('~') {
+                                (&mut unless_domain, domain_stripped)
+                            } else {
+                                (&mut if_domain, domain)
+                            };
+
+                        let lowercase = domain.to_lowercase();
+                        let normalized_domain = if lowercase.is_ascii() {
+                            lowercase
+                        } else {
+                            idna::domain_to_ascii(&lowercase)
+                                .map_err(|_| CbRuleCreationFailure::RuleContainsNonASCII)?
+                        };
 
-                    collection.push(format!("*{normalized_domain}"));
-                });
+                        collection.push(format!("*{normalized_domain}"));
+                    }
 
-                (non_empty(if_domain), non_empty(unless_domain))
-            } else {
-                (None, None)
-            };
+                    (non_empty(if_domain), non_empty(unless_domain))
+                } else {
+                    (None, None)
+                };
 
             if if_domain.is_some() && unless_domain.is_some() {
                 return Err(CbRuleCreationFailure::UnlessAndIfDomainTogetherUnsupported);

tests/unit/content_blocking.rs

@@ -26,6 +26,26 @@ mod ab2cb_tests {
         );
     }
 
+    #[test]
+    fn domain_metadata_without_parseable_options_is_unsupported() {
+        let filter = crate::lists::parse_filter(
+            "||example.com^$script,domain=test.example",
+            true,
+            Default::default(),
+        )
+        .expect("Rule under test could not be parsed");
+
+        let crate::lists::ParsedFilter::Network(mut filter) = filter else {
+            panic!("Expected a network filter");
+        };
+        filter.raw_line = Some(Box::new("||example.com^".to_string()));
+
+        assert!(matches!(
+            CbRuleEquivalent::try_from(crate::lists::ParsedFilter::Network(filter)),
+            Err(CbRuleCreationFailure::FromNotSupported)
+        ));
+    }
+
     #[test]
     fn ad_tests() {
         test_from_abp(