Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 61 additions & 9 deletions src/filters/abstract_network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,16 +107,68 @@ impl AbstractNetworkFilter {
exception = true;
}

let maybe_options_index: Option<usize> = 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("||") {
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/blocker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
Expand Down
34 changes: 34 additions & 0 deletions tests/unit/filters/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}