diff --git a/src/content_blocking.rs b/src/content_blocking.rs index 128d202f..8224ebcb 100644 --- a/src/content_blocking.rs +++ b/src/content_blocking.rs @@ -427,53 +427,52 @@ impl TryFrom 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); diff --git a/tests/unit/content_blocking.rs b/tests/unit/content_blocking.rs index a95d048b..67d876b4 100644 --- a/tests/unit/content_blocking.rs +++ b/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(