Added $to= option matching. - #716
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| None, | ||
| ) | ||
| .unwrap() | ||
| } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[opengrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon
Please consider an alternative approach that avoids this security concern, or request a review from the sec-team on slack.
| ::flatbuffers::Vector<'a, ::flatbuffers::ForwardsUOffset<NetworkFilter>>, | ||
| >>(NetworkFilterList::VT_OPT_TO_DOMAINS_MAP_VALUES, None) | ||
| .unwrap() | ||
| } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[opengrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon
Please consider an alternative approach that avoids this security concern, or request a review from the sec-team on slack.
There was a problem hiding this comment.
Pull request overview
This PR adds full support for $to= option matching by bucketing filters on destination-hostname hashes (in addition to existing $domain=/$from= source-domain bucketing), and updates the serialized data format accordingly.
Changes:
- Implement
$to=token selection/bucketing and apply$to=constraints during request matching (including negated$to). - Extend the FlatBuffers schema and on-disk data format (version bump) to store
$to=-bucketed filter maps. - Add/adjust unit tests to cover
$to=tokenization and matching behavior, and update serialization hash expectations.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/filters/network.rs | Adds unit tests validating $to= tokenization and token-bucketing precedence. |
| tests/unit/filters/network_matchers.rs | Updates matcher tests to assert $to= is enforced against destination hostnames (incl. negation and $from+$to). |
| tests/unit/engine.rs | Updates expected serialization hashes due to schema/data changes. |
| src/request.rs | Adds destination-hostname hash list (hostname_hashes) and shared label-hash helper for source/destination. |
| src/optimizer.rs | Prevents pattern-based optimization for filters with $to= constraints. |
| src/network_filter_list.rs | Adds $to= token map lookup path before falling back to pattern buckets; adjusts match token plumbing. |
| src/flatbuffers/fb_network_filter.fbs | Extends NetworkFilterList with required $to= map index/values vectors. |
| src/flatbuffers/fb_network_filter_generated.rs | Regenerates FlatBuffers bindings to include $to= map fields (required). |
| src/filters/network.rs | Adds FilterTokens::OptToDomains and $to= participation in token selection and filter IDs. |
| src/filters/network_matchers.rs | Generalizes domain-inclusion/exclusion checks to accept hostname-hash slices and reuses them for $to=. |
| src/filters/fb_network.rs | Plumbs $to= include/exclude domain checks into FlatBuffer-backed filter matching. |
| src/filters/fb_network_builder.rs | Adds $to= bucket map builder and includes it in serialized NetworkFilterList. |
| src/data_format/mod.rs | Bumps data format version to 8 for the new schema/layout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Rust Benchmark
Details
| Benchmark suite | Current: 3845835 | Previous: dc4d694 | Ratio |
|---|---|---|---|
rule-match-browserlike/brave-list |
2751849481 ns/iter (± 21399772) |
2099334199 ns/iter (± 17991214) |
1.31 |
rule-match-first-request/brave-list |
2894477 ns/iter (± 13160) |
1421104 ns/iter (± 10951) |
2.04 |
blocker_new/brave-list |
116393196 ns/iter (± 424771) |
112417642 ns/iter (± 561176) |
1.04 |
blocker_new/brave-list-deserialize |
29692058 ns/iter (± 466063) |
32505677 ns/iter (± 1294449) |
0.91 |
memory-usage-final/brave-list-initial |
9599810 B/iter (± 0) |
9561338 B/iter (± 0) |
1.00 |
memory-usage-final/brave-list-1000-requests |
4634920 B/iter (± 0) |
3183886 B/iter (± 0) |
1.46 |
memory-usage-max/brave-list-initial/max |
32108754 B/iter (± 0) |
32070026 B/iter (± 0) |
1.00 |
memory-usage-alloc-count/brave-list-initial/alloc-count |
284588 allocs/iter (± 0) |
284089 allocs/iter (± 0) |
1.00 |
memory-usage-alloc-count/brave-list-1000-requests/alloc-count |
114719 allocs/iter (± 0) |
80628 allocs/iter (± 0) |
1.42 |
url_cosmetic_resources/brave-list |
185550 ns/iter (± 703) |
184145 ns/iter (± 1771) |
1.01 |
cosmetic-class-id-match/brave-list |
3298491 ns/iter (± 881688) |
3240078 ns/iter (± 845876) |
1.02 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.10.
| Benchmark suite | Current: 3845835 | Previous: dc4d694 | Ratio |
|---|---|---|---|
rule-match-browserlike/brave-list |
2751849481 ns/iter (± 21399772) |
2099334199 ns/iter (± 17991214) |
1.31 |
rule-match-first-request/brave-list |
2894477 ns/iter (± 13160) |
1421104 ns/iter (± 10951) |
2.04 |
memory-usage-final/brave-list-1000-requests |
4634920 B/iter (± 0) |
3183886 B/iter (± 0) |
1.46 |
memory-usage-alloc-count/brave-list-1000-requests/alloc-count |
114719 allocs/iter (± 0) |
80628 allocs/iter (± 0) |
1.42 |
This comment was automatically generated by workflow using github-action-benchmark.
Fix #365