diff --git a/src/filters/abstract_network.rs b/src/filters/abstract_network.rs index b97c9b33..01d69162 100644 --- a/src/filters/abstract_network.rs +++ b/src/filters/abstract_network.rs @@ -107,16 +107,68 @@ impl AbstractNetworkFilter { exception = true; } - let maybe_options_index: Option = find_char_reverse(b'$', line.as_bytes()); - + // Find the `$` that separates the pattern from filter options. + // Regex filters (delimited by `/`) may contain `$` as an end-of-string + // anchor. When a candidate `$` yields an options-parse error and the + // pattern to its left looks like a regex, try the next `$` to the left. let mut options = None; - if let Some(options_index) = maybe_options_index { - filter_index_end = options_index; - - // slicing here is safe; the first byte after '$' will be a character boundary - let raw_options = &line[filter_index_end + 1..]; - - options = Some(parse_filter_options(raw_options)?); + { + let bytes = line.as_bytes(); + let mut search_end = bytes.len(); + let mut last_err = None; + loop { + let maybe_dollar = find_char_reverse(b'$', &bytes[..search_end]); + match maybe_dollar { + Some(idx) if idx >= filter_index_start => { + let raw_options = &line[idx + 1..]; + match parse_filter_options(raw_options) { + Ok(parsed) => { + filter_index_end = idx; + options = Some(parsed); + last_err = None; + break; + } + Err(e) => { + // Check if the pattern to the left of this `$` looks + // like a regex (delimited by `/`). Only then is it + // plausible that this `$` is a regex anchor rather than + // the options separator. + let pattern_before = &line[filter_index_start..idx]; + let after_dollar = &line[idx + 1..]; + // The `$` may be a regex end-anchor if the overall + // pattern is regex-delimited (`/pattern$/`). Check + // whether the left side starts with `/` and either + // already ends with `/`, or the text right after `$` + // starts with `/` (the closing regex delimiter). + let looks_like_regex = pattern_before.starts_with('/') + && (pattern_before.ends_with('/') + || pattern_before.ends_with("$/") + || after_dollar.starts_with('/')); + if looks_like_regex { + // Try the next `$` to the left. + last_err = Some(e); + search_end = idx; + } else { + // Not a regex pattern — this `$` must be the + // options separator; propagate the error. + return Err(e); + } + } + } + } + _ => { + // No more `$` candidates. If we had a previous error from a + // regex-looking pattern, that error was for the wrong `$` — + // the filter simply has no options section. + if last_err.is_some() { + // No valid options separator found; treat the entire + // line (after @@) as the pattern with no options. + filter_index_end = line.len(); + } + break; + } + } + } } let left_anchor = if line[filter_index_start..].starts_with("||") { diff --git a/tests/unit/blocker.rs b/tests/unit/blocker.rs index 1cd0b962..653ad786 100644 --- a/tests/unit/blocker.rs +++ b/tests/unit/blocker.rs @@ -1360,7 +1360,7 @@ mod legacy_rule_parsing_tests { // * not handling document/subdocument options; // * the optimizer that merges multiple rules into one; const EASY_LIST: ListCounts = ListCounts { - filters: 53691 - 678, + filters: 53695 - 678, cosmetic_filters: if cfg!(feature = "css-validation") { 23808 } else { diff --git a/tests/unit/engine.rs b/tests/unit/engine.rs index e3ab2b1b..dbfd3005 100644 --- a/tests/unit/engine.rs +++ b/tests/unit/engine.rs @@ -237,9 +237,9 @@ mod tests { ); } let expected_hash: u64 = if cfg!(feature = "css-validation") { - 7671576097086431151 + 15675484747929189047 } else { - 7220740598028920672 + 18054822511375548505 }; assert_eq!(hash(&data), expected_hash, "{HASH_MISMATCH_MSG}"); diff --git a/tests/unit/filters/network.rs b/tests/unit/filters/network.rs index 5ffc295e..b80e622a 100644 --- a/tests/unit/filters/network.rs +++ b/tests/unit/filters/network.rs @@ -1257,4 +1257,38 @@ mod parse_tests { FilterTokens::Other(&[utils::fast_hash("some"), utils::fast_hash("primewire")]) ); } + + #[test] + fn test_regex_with_dollar_end_anchor_issue_257() { + // https://github.com/brave/adblock-rust/issues/257 + // Regex filter with $ end-anchor AND options should parse correctly + let filter_str = r"/^https?:\/\/[a-z]{8,15}\.top\/[a-z]{4,}\.json$/$xhr,3p,match-case"; + let result = NetworkFilter::parse(filter_str, true, Default::default()); + assert!(result.is_ok(), "Filter with regex $ anchor and options should parse: {:?}", result); + let filter = result.unwrap(); + // The pattern should include the $ anchor character + let pattern = filter.filter.string_view().unwrap(); + assert!(pattern.contains("json$"), "Regex pattern should contain $ anchor, got: {}", pattern); + assert!(filter.match_case(), "match-case option should be set"); + assert!(filter.is_regex(), "filter should be detected as regex"); + } + + #[test] + fn test_regex_with_dollar_anchor_no_options() { + // Regex filter with $ anchor but no options + let filter_str = r"/\.json$/"; + let result = NetworkFilter::parse(filter_str, true, Default::default()); + assert!(result.is_ok(), "Regex with $ anchor and no options should parse: {:?}", result); + let filter = result.unwrap(); + let pattern = filter.filter.string_view().unwrap(); + assert!(pattern.contains('$'), "Pattern should contain $ anchor, got: {}", pattern); + } + + #[test] + fn test_regex_without_dollar_anchor_with_options() { + // Normal regex with options but no $ anchor - should still work + let filter_str = r"/\.json/$script,3p"; + let result = NetworkFilter::parse(filter_str, true, Default::default()); + assert!(result.is_ok(), "Normal regex with options should parse: {:?}", result); + } }