Skip to content

Commit f85860d

Browse files
antonok-edmantonok-edm
authored
Merge pull request #371 from brave/removeparam-types
Only apply removeparam to document/subdocument/xhr by default
2 parents 859e7de + da3d470 commit f85860d

File tree

2 files changed

+50
-30
lines changed

2 files changed

+50
-30
lines changed

‎src/blocker.rs

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,7 +1616,7 @@ mod blocker_tests {
16161616
"*$removeparam=fbclid",
16171617
"/script.js$redirect-rule=noopjs",
16181618
"^block^$important",
1619-
"$removeparam=testCase,~image",
1619+
"$removeparam=testCase,~xhr",
16201620
];
16211621

16221622
let (network_filters, _) = parse_filters(&filters, true, Default::default());
@@ -1630,73 +1630,89 @@ mod blocker_tests {
16301630

16311631
resources.add_resource(Resource::simple("noopjs", crate::resources::MimeType::ApplicationJavascript, "(() => {})()")).unwrap();
16321632

1633-
let result = blocker.check(&Request::new("https://example.com?q=1&test=2#blue", "https://antonok.com", "script").unwrap(), &resources);
1633+
let result = blocker.check(&Request::new("https://example.com?q=1&test=2#blue", "https://antonok.com", "xhr").unwrap(), &resources);
16341634
assert_eq!(result.rewritten_url, Some("https://example.com?q=1#blue".into()));
16351635
assert!(!result.matched);
16361636

1637-
let result = blocker.check(&Request::new("https://example.com?test=2&q=1#blue", "https://antonok.com", "script").unwrap(), &resources);
1637+
let result = blocker.check(&Request::new("https://example.com?test=2&q=1#blue", "https://antonok.com", "xhr").unwrap(), &resources);
16381638
assert_eq!(result.rewritten_url, Some("https://example.com?q=1#blue".into()));
16391639
assert!(!result.matched);
16401640

1641-
let result = blocker.check(&Request::new("https://example.com?test=2#blue", "https://antonok.com", "script").unwrap(), &resources);
1641+
let result = blocker.check(&Request::new("https://example.com?test=2#blue", "https://antonok.com", "xhr").unwrap(), &resources);
16421642
assert_eq!(result.rewritten_url, Some("https://example.com#blue".into()));
16431643
assert!(!result.matched);
16441644

1645-
let result = blocker.check(&Request::new("https://example.com?q=1#blue", "https://antonok.com", "script").unwrap(), &resources);
1645+
let result = blocker.check(&Request::new("https://example.com?q=1#blue", "https://antonok.com", "xhr").unwrap(), &resources);
16461646
assert_eq!(result.rewritten_url, None);
16471647
assert!(!result.matched);
16481648

1649-
let result = blocker.check(&Request::new("https://example.com?q=1&test=2", "https://antonok.com", "script").unwrap(), &resources);
1649+
let result = blocker.check(&Request::new("https://example.com?q=1&test=2", "https://antonok.com", "xhr").unwrap(), &resources);
16501650
assert_eq!(result.rewritten_url, Some("https://example.com?q=1".into()));
16511651
assert!(!result.matched);
16521652

1653-
let result = blocker.check(&Request::new("https://example.com?test=2&q=1", "https://antonok.com", "script").unwrap(), &resources);
1653+
let result = blocker.check(&Request::new("https://example.com?test=2&q=1", "https://antonok.com", "xhr").unwrap(), &resources);
16541654
assert_eq!(result.rewritten_url, Some("https://example.com?q=1".into()));
16551655
assert!(!result.matched);
16561656

1657-
let result = blocker.check(&Request::new("https://example.com?test=2", "https://antonok.com", "script").unwrap(), &resources);
1657+
let result = blocker.check(&Request::new("https://example.com?test=2", "https://antonok.com", "xhr").unwrap(), &resources);
16581658
assert_eq!(result.rewritten_url, Some("https://example.com".into()));
16591659
assert!(!result.matched);
16601660

1661-
let result = blocker.check(&Request::new("https://example.com?q=1", "https://antonok.com", "script").unwrap(), &resources);
1661+
let result = blocker.check(&Request::new("https://example.com?test=2", "https://antonok.com", "image").unwrap(), &resources);
16621662
assert_eq!(result.rewritten_url, None);
16631663
assert!(!result.matched);
16641664

1665-
let result = blocker.check(&Request::new("https://example.com?q=fbclid", "https://antonok.com", "script").unwrap(), &resources);
1665+
let result = blocker.check(&Request::new("https://example.com?q=1", "https://antonok.com", "xhr").unwrap(), &resources);
16661666
assert_eq!(result.rewritten_url, None);
16671667
assert!(!result.matched);
16681668

1669-
let result = blocker.check(&Request::new("https://example.com?fbclid=10938&q=1&test=2", "https://antonok.com", "script").unwrap(), &resources);
1669+
let result = blocker.check(&Request::new("https://example.com?q=fbclid", "https://antonok.com", "xhr").unwrap(), &resources);
1670+
assert_eq!(result.rewritten_url, None);
1671+
assert!(!result.matched);
1672+
1673+
let result = blocker.check(&Request::new("https://example.com?fbclid=10938&q=1&test=2", "https://antonok.com", "xhr").unwrap(), &resources);
16701674
assert_eq!(result.rewritten_url, Some("https://example.com?q=1".into()));
16711675
assert!(!result.matched);
16721676

1673-
let result = blocker.check(&Request::new("https://test.com?fbclid=10938&q=1&test=2", "https://antonok.com", "script").unwrap(), &resources);
1677+
let result = blocker.check(&Request::new("https://test.com?fbclid=10938&q=1&test=2", "https://antonok.com", "xhr").unwrap(), &resources);
16741678
assert_eq!(result.rewritten_url, Some("https://test.com?q=1&test=2".into()));
16751679
assert!(!result.matched);
16761680

1677-
let result = blocker.check(&Request::new("https://example.com?q1=1&q2=2&q3=3&test=2&q4=4&q5=5&fbclid=39", "https://antonok.com", "script").unwrap(), &resources);
1681+
let result = blocker.check(&Request::new("https://example.com?q1=1&q2=2&q3=3&test=2&q4=4&q5=5&fbclid=39", "https://antonok.com", "xhr").unwrap(), &resources);
16781682
assert_eq!(result.rewritten_url, Some("https://example.com?q1=1&q2=2&q3=3&q4=4&q5=5".into()));
16791683
assert!(!result.matched);
16801684

1681-
let result = blocker.check(&Request::new("https://example.com?q1=1&q1=2&test=2&test=3", "https://antonok.com", "script").unwrap(), &resources);
1685+
let result = blocker.check(&Request::new("https://example.com?q1=1&q1=2&test=2&test=3", "https://antonok.com", "xhr").unwrap(), &resources);
16821686
assert_eq!(result.rewritten_url, Some("https://example.com?q1=1&q1=2".into()));
16831687
assert!(!result.matched);
16841688

1685-
let result = blocker.check(&Request::new("https://example.com/script.js?test=2#blue", "https://antonok.com", "script").unwrap(), &resources);
1689+
let result = blocker.check(&Request::new("https://example.com/script.js?test=2#blue", "https://antonok.com", "xhr").unwrap(), &resources);
16861690
assert_eq!(result.rewritten_url, Some("https://example.com/script.js#blue".into()));
16871691
assert_eq!(result.redirect, Some("data:application/javascript;base64,KCgpID0+IHt9KSgp".into()));
16881692
assert!(!result.matched);
16891693

1690-
let result = blocker.check(&Request::new("https://example.com/block/script.js?test=2", "https://antonok.com", "script").unwrap(), &resources);
1694+
let result = blocker.check(&Request::new("https://example.com/block/script.js?test=2", "https://antonok.com", "xhr").unwrap(), &resources);
16911695
assert_eq!(result.rewritten_url, None);
16921696
assert_eq!(result.redirect, Some("data:application/javascript;base64,KCgpID0+IHt9KSgp".into()));
16931697
assert!(result.matched);
16941698

1695-
let result = blocker.check(&Request::new("https://example.com/Path/?Test=ABC&testcase=AbC&testCase=aBc", "https://antonok.com", "script").unwrap(), &resources);
1699+
let result = blocker.check(&Request::new("https://example.com/Path/?Test=ABC&testcase=AbC&testCase=aBc", "https://antonok.com", "xhr").unwrap(), &resources);
1700+
assert_eq!(result.rewritten_url, None);
1701+
assert!(!result.matched);
1702+
1703+
let result = blocker.check(&Request::new("https://example.com/Path/?Test=ABC&testcase=AbC&testCase=aBc", "https://antonok.com", "image").unwrap(), &resources);
1704+
assert_eq!(result.rewritten_url, None);
1705+
assert!(!result.matched);
1706+
1707+
let result = blocker.check(&Request::new("https://example.com/Path/?Test=ABC&testcase=AbC&testCase=aBc", "https://antonok.com", "subdocument").unwrap(), &resources);
1708+
assert_eq!(result.rewritten_url, Some("https://example.com/Path/?Test=ABC&testcase=AbC".into()));
1709+
assert!(!result.matched);
1710+
1711+
let result = blocker.check(&Request::new("https://example.com/Path/?Test=ABC&testcase=AbC&testCase=aBc", "https://antonok.com", "document").unwrap(), &resources);
16961712
assert_eq!(result.rewritten_url, Some("https://example.com/Path/?Test=ABC&testcase=AbC".into()));
16971713
assert!(!result.matched);
16981714

1699-
let result = blocker.check(&Request::new("https://example.com?Test=ABC?123&test=3#&test=4#b", "https://antonok.com", "script").unwrap(), &resources);
1715+
let result = blocker.check(&Request::new("https://example.com?Test=ABC?123&test=3#&test=4#b", "https://antonok.com", "document").unwrap(), &resources);
17001716
assert_eq!(result.rewritten_url, Some("https://example.com?Test=ABC?123#&test=4#b".into()));
17011717
assert!(!result.matched);
17021718

@@ -1774,7 +1790,7 @@ mod blocker_tests {
17741790
let resources = ResourceStorage::default();
17751791

17761792
for (original, expected) in testcases.into_iter() {
1777-
let result = blocker.check(&Request::new(original, "https://example.net", "script").unwrap(), &resources);
1793+
let result = blocker.check(&Request::new(original, "https://example.net", "xhr").unwrap(), &resources);
17781794
let expected = if original == expected {
17791795
None
17801796
} else {
@@ -1799,7 +1815,7 @@ fn test_removeparam_same_tokens() {
17991815

18001816
let blocker = Blocker::new(network_filters, &blocker_options);
18011817

1802-
let result = blocker.check(&Request::new("https://example.com?example1_=1&example1-=2", "https://example.com", "script").unwrap(), &Default::default());
1818+
let result = blocker.check(&Request::new("https://example.com?example1_=1&example1-=2", "https://example.com", "xhr").unwrap(), &Default::default());
18031819
assert_eq!(result.rewritten_url, Some("https://example.com".into()));
18041820
assert!(!result.matched);
18051821
}

‎src/filters/network.rs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -644,12 +644,22 @@ impl NetworkFilter {
644644

645645
// If any negated "network" types were set, then implicitly enable all network types.
646646
// The negated types will be applied later.
647-
if (cpt_mask_negative & NetworkFilterMask::FROM_NETWORK_TYPES) != NetworkFilterMask::NONE {
647+
//
648+
// This doesn't apply to removeparam filters.
649+
if !mask.contains(NetworkFilterMask::IS_REMOVEPARAM)
650+
&& (cpt_mask_negative & NetworkFilterMask::FROM_NETWORK_TYPES) != NetworkFilterMask::NONE {
648651
mask |= NetworkFilterMask::FROM_NETWORK_TYPES;
649652
}
650653
// If no positive types were set, then the filter should apply to all network types.
651654
if (cpt_mask_positive & NetworkFilterMask::FROM_ALL_TYPES).is_empty() {
652-
mask |= NetworkFilterMask::FROM_NETWORK_TYPES;
655+
// Removeparam is again a special case.
656+
if mask.contains(NetworkFilterMask::IS_REMOVEPARAM) {
657+
mask |= NetworkFilterMask::FROM_DOCUMENT
658+
| NetworkFilterMask::FROM_SUBDOCUMENT
659+
| NetworkFilterMask::FROM_XMLHTTPREQUEST;
660+
} else {
661+
mask |= NetworkFilterMask::FROM_NETWORK_TYPES;
662+
}
653663
}
654664

655665
match parsed.pattern.left_anchor {
@@ -833,13 +843,6 @@ impl NetworkFilter {
833843
return Err(NetworkFilterError::RemoveparamWithException);
834844
}
835845

836-
// `removeparam` rules apply to all request types by default, including document.
837-
if mask.contains(NetworkFilterMask::IS_REMOVEPARAM)
838-
&& (cpt_mask_positive & NetworkFilterMask::FROM_ALL_TYPES).is_empty()
839-
{
840-
mask |= NetworkFilterMask::FROM_ALL_TYPES;
841-
}
842-
843846
// uBlock Origin would block main document `https://example.com` requests with all of the
844847
// following filters:
845848
// - ||example.com
@@ -854,7 +857,8 @@ impl NetworkFilter {
854857
(cpt_mask_negative & NetworkFilterMask::FROM_ALL_TYPES).is_empty() &&
855858
mask.contains(NetworkFilterMask::IS_HOSTNAME_ANCHOR) &&
856859
mask.contains(NetworkFilterMask::IS_RIGHT_ANCHOR) &&
857-
!end_url_anchor {
860+
!end_url_anchor &&
861+
!mask.contains(NetworkFilterMask::IS_REMOVEPARAM) {
858862
mask |= NetworkFilterMask::FROM_ALL_TYPES;
859863
}
860864
// Finally, apply any explicitly negated request types

0 commit comments

Comments
 (0)