Skip to content

Commit 988c9bf

Browse files
Oleg Labudkoartemiv4nov
Oleg Labudko
authored and
artemiv4nov
committed
Pull request #94: AG-23378 Do not split rules with many domains in the unless-domain and if-domain
Merge in ADGUARD-IOS/safari-converter from fix/AG-23378 to master * commit '1a6e72e94833327d504f7bbc199c7f42801b64bc': added logging for domains limit exceeded case added test to the test list discarded rules with regexp in if-domain or unless-domain added github issue link removed redundants fixed tests refactored handling domains limit
2 parents be622c5 + 1a6e72e commit 988c9bf

File tree

5 files changed

+77
-65
lines changed

5 files changed

+77
-65
lines changed

‎Sources/ContentBlockerConverter/Compiler/BlockerEntryFactory.swift

+20-3
Original file line numberDiff line numberDiff line change
@@ -386,10 +386,27 @@ class BlockerEntryFactory {
386386
}
387387
};
388388

389-
private func addDomainOptions(rule: Rule, trigger: inout BlockerEntry.Trigger) throws -> Void {
390-
let included = resolveTopLevelDomainWildcards(domains: rule.permittedDomains);
389+
/**
390+
* Throws error if provided rule contains regexp in permitted or restricted domains
391+
*/
392+
private func excludeRegexpDomainRule(_ rule: Rule) throws -> Void {
393+
let domains = rule.restrictedDomains + rule.permittedDomains
394+
try domains.forEach { item in
395+
if item.hasPrefix("/") && item.hasSuffix("/") {
396+
throw ConversionError.invalidDomains(message: "Safari does not support regular expressions in permitted or restricted domains");
397+
}
398+
}
399+
}
400+
401+
private func addDomainOptions(rule: Rule, trigger: inout BlockerEntry.Trigger) throws -> Void {
402+
var excludedDomains = rule.restrictedDomains
403+
let includedDomains = rule.permittedDomains
391404

392-
var excludedDomains = rule.restrictedDomains;
405+
// discard rules that contains regexp in if-domain or unless-domain
406+
// https://github.com/AdguardTeam/SafariConverterLib/issues/53
407+
try excludeRegexpDomainRule(rule);
408+
409+
let included = resolveTopLevelDomainWildcards(domains: includedDomains);
393410
addUnlessDomainForThirdParty(rule: rule, domains: &excludedDomains);
394411
let excluded = resolveTopLevelDomainWildcards(domains: excludedDomains);
395412

‎Sources/ContentBlockerConverter/Distributor.swift

+19-34
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import Foundation
2+
import Shared
23

34
/**
45
* Maximum domains amount for css blocking rule
56
*/
6-
private let MAX_DOMAINS_FOR_RULE = 250;
7+
private let MAX_DOMAINS_FOR_RULE = 10000
78

89
/**
910
* Distributor class
@@ -69,46 +70,30 @@ class Distributor {
6970
);
7071
}
7172

72-
/**
73-
* Checks the if-domain and unless-domain amount and splits entry if it's over limit
74-
*/
75-
private func handleDomainLimit(entry: BlockerEntry) -> [BlockerEntry] {
76-
var result = [BlockerEntry]();
77-
let ifDomainsNum = entry.trigger.ifDomain?.count ?? 0;
78-
let unlessDomainsNum = entry.trigger.unlessDomain?.count ?? 0;
79-
if ifDomainsNum > MAX_DOMAINS_FOR_RULE {
80-
let chunkedIfDomains = [[String]]?(entry.trigger.ifDomain!.chunked(into: MAX_DOMAINS_FOR_RULE));
81-
for chunk in chunkedIfDomains! {
82-
var newEntry = entry;
83-
newEntry.trigger.ifDomain = Array(chunk);
84-
result.append(newEntry);
85-
}
86-
return result;
87-
} else if unlessDomainsNum > MAX_DOMAINS_FOR_RULE {
88-
let chunkedUnlessDomains = [[String]]?(entry.trigger.unlessDomain!.chunked(into: MAX_DOMAINS_FOR_RULE));
89-
for chunk in chunkedUnlessDomains! {
90-
var newEntry = entry;
91-
newEntry.trigger.unlessDomain = Array(chunk);
92-
result.append(newEntry);
93-
}
94-
return result;
95-
} else {
96-
return [entry];
97-
}
98-
}
99-
10073
/**
10174
* Updates if-domain and unless-domain fields.
10275
* Adds wildcard to every rule and splits rules contains over limit domains
10376
*/
10477
func updateDomains(entries: [BlockerEntry]) -> [BlockerEntry] {
105-
var result = [BlockerEntry]();
78+
var result = [BlockerEntry]()
10679
for var entry in entries {
107-
entry.trigger.ifDomain = addWildcard(domains: entry.trigger.ifDomain);
108-
entry.trigger.unlessDomain = addWildcard(domains: entry.trigger.unlessDomain);
109-
result += handleDomainLimit(entry: entry);
80+
entry.trigger.ifDomain = addWildcard(domains: entry.trigger.ifDomain)
81+
entry.trigger.unlessDomain = addWildcard(domains: entry.trigger.unlessDomain)
82+
83+
let ifDomainsAmount = entry.trigger.ifDomain?.count ?? 0
84+
let unlessDomainsAmount = entry.trigger.unlessDomain?.count ?? 0
85+
86+
// discard rules that exceed domains limit
87+
// https://github.com/AdguardTeam/SafariConverterLib/issues/51
88+
if ifDomainsAmount > MAX_DOMAINS_FOR_RULE
89+
|| unlessDomainsAmount > MAX_DOMAINS_FOR_RULE {
90+
Logger.log("Domains limit exceeded: \(ifDomainsAmount > MAX_DOMAINS_FOR_RULE ? ifDomainsAmount : unlessDomainsAmount)")
91+
continue
92+
}
93+
94+
result += [entry]
11095
}
111-
return result;
96+
return result
11297
};
11398

11499
private func addWildcard(domains: [String]?) -> [String]? {

‎Tests/ContentBlockerConverterTests/ContentBlockerConverterTests.swift

+30-15
Original file line numberDiff line numberDiff line change
@@ -776,40 +776,32 @@ final class ContentBlockerConverterTests: XCTestCase {
776776

777777
func testTldWildcardRules() {
778778
var result = converter.convertArray(rules: ["surge.*,testcases.adguard.*###case-5-wildcard-for-tld > .test-banner"]);
779-
XCTAssertEqual(result.convertedCount, 2);
779+
XCTAssertEqual(result.convertedCount, 1);
780780

781781
var decoded = try! parseJsonString(json: result.converted);
782-
XCTAssertEqual(decoded.count, 2);
782+
XCTAssertEqual(decoded.count, 1);
783783
XCTAssertEqual(decoded[0].trigger.urlFilter, URL_FILTER_CSS_RULES);
784784
XCTAssertEqual(decoded[0].trigger.ifDomain?[0], "*surge.com");
785785
XCTAssertEqual(decoded[0].trigger.ifDomain?[1], "*surge.ru");
786786
XCTAssertEqual(decoded[0].trigger.ifDomain?[2], "*surge.net");
787-
XCTAssertEqual(decoded[0].trigger.ifDomain?.count, 250);
788-
789-
XCTAssertEqual(decoded[1].trigger.urlFilter, URL_FILTER_CSS_RULES);
790-
XCTAssertNotNil(decoded[1].trigger.ifDomain?[0]);
791-
XCTAssertEqual(decoded[1].trigger.ifDomain?.count, 150);
787+
XCTAssertEqual(decoded[0].trigger.ifDomain?.count, 400);
792788

793789

794790
result = converter.convertArray(rules: ["||*/test-files/adguard.png$domain=surge.*|testcases.adguard.*"]);
795-
XCTAssertEqual(result.convertedCount, 2);
791+
XCTAssertEqual(result.convertedCount, 1);
796792

797793
decoded = try! parseJsonString(json: result.converted);
798-
XCTAssertEqual(decoded.count, 2);
794+
XCTAssertEqual(decoded.count, 1);
799795
XCTAssertEqual(decoded[0].trigger.urlFilter, START_URL_UNESCAPED + ".*\\/test-files\\/adguard\\.png");
800796
XCTAssertEqual(decoded[0].trigger.ifDomain?[0], "*surge.com");
801797
XCTAssertEqual(decoded[0].trigger.ifDomain?[1], "*surge.ru");
802798
XCTAssertEqual(decoded[0].trigger.ifDomain?[2], "*surge.net");
803-
XCTAssertEqual(decoded[0].trigger.ifDomain?.count, 250);
799+
XCTAssertEqual(decoded[0].trigger.ifDomain?.count, 400);
804800

805801
var regex = try! NSRegularExpression(pattern: decoded[0].trigger.urlFilter!);
806802
XCTAssertTrue(SimpleRegex.isMatch(regex: regex, target: "https://test.com/test-files/adguard.png"));
807803

808-
XCTAssertEqual(decoded[1].trigger.urlFilter, START_URL_UNESCAPED + ".*\\/test-files\\/adguard\\.png");
809-
XCTAssertNotNil(decoded[1].trigger.ifDomain?[0]);
810-
XCTAssertEqual(decoded[1].trigger.ifDomain?.count, 150);
811-
812-
regex = try! NSRegularExpression(pattern: decoded[1].trigger.urlFilter!);
804+
regex = try! NSRegularExpression(pattern: decoded[0].trigger.urlFilter!);
813805
XCTAssertTrue(SimpleRegex.isMatch(regex: regex, target: "https://test.com/test-files/adguard.png"));
814806

815807
result = converter.convertArray(rules: ["|http$script,domain=forbes.*"]);
@@ -2474,6 +2466,28 @@ final class ContentBlockerConverterTests: XCTestCase {
24742466
XCTAssertEqual(decoded[0].action.type, "script")
24752467
XCTAssertEqual(decoded[0].action.script, "window.adv_id = null;")
24762468
}
2469+
2470+
func testExcludingRulesWithRegex() {
2471+
var rules = ["||example.org$domain=/test\\.com/"]
2472+
2473+
var result = converter.convertArray(rules: rules, safariVersion: .safari15)
2474+
XCTAssertEqual(result.converted, ConversionResult.EMPTY_RESULT_JSON);
2475+
2476+
rules = [
2477+
"/example1\\.org/#%#alert('1');",
2478+
"||example2.org$domain=/test2\\.com/",
2479+
"||example3.org$domain=test3.com/",
2480+
"/example4\\.org/##.adv"
2481+
]
2482+
2483+
result = converter.convertArray(rules: rules, safariVersion: .safari15)
2484+
let decoded = try! parseJsonString(json: result.converted)
2485+
XCTAssertEqual(decoded.count, 1)
2486+
2487+
XCTAssertEqual(decoded[0].trigger.ifDomain, ["*test3.com/"])
2488+
XCTAssertEqual(decoded[0].trigger.urlFilter, "^[htpsw]+:\\/\\/([a-z0-9-]+\\.)?example3\\.org")
2489+
XCTAssertEqual(decoded[0].action.type, "block")
2490+
}
24772491

24782492
static var allTests = [
24792493
("testEmpty", testEmpty),
@@ -2538,5 +2552,6 @@ final class ContentBlockerConverterTests: XCTestCase {
25382552
("testXpathRules", testXpathRules),
25392553
("testApplyMultidomainCosmeticExclusions", testApplyMultidomainCosmeticExclusions),
25402554
("testApplyMultidomainAdvancedExclusions", testApplyMultidomainAdvancedExclusions),
2555+
("testExcludingRulesWithRegex", testExcludingRulesWithRegex),
25412556
]
25422557
}

‎Tests/ContentBlockerConverterTests/DistributorTests.swift

+4-9
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,8 @@ final class DistributorTests: XCTestCase {
9292

9393
let result = builder.updateDomains(entries: entries);
9494
XCTAssertNotNil(result);
95-
XCTAssertEqual(result.count, 3);
96-
XCTAssertEqual(result[0].trigger.ifDomain!.count, 250);
97-
XCTAssertEqual(result[1].trigger.ifDomain!.count, 250);
98-
XCTAssertEqual(result[2].trigger.ifDomain!.count, 51);
95+
XCTAssertEqual(result.count, 1);
96+
XCTAssertEqual(result[0].trigger.ifDomain!.count, 551);
9997
}
10098

10199
func testHandleUnlessDomainsLimit() {
@@ -134,13 +132,10 @@ final class DistributorTests: XCTestCase {
134132

135133
let result = builder.updateDomains(entries: entries);
136134
XCTAssertNotNil(result);
137-
XCTAssertEqual(result.count, 3);
138-
XCTAssertEqual(result[0].trigger.unlessDomain!.count, 250);
139-
XCTAssertEqual(result[1].trigger.unlessDomain!.count, 250);
140-
XCTAssertEqual(result[2].trigger.unlessDomain!.count, 51);
135+
XCTAssertEqual(result.count, 1);
136+
XCTAssertEqual(result[0].trigger.unlessDomain!.count, 551);
141137
}
142138

143-
144139
static var allTests = [
145140
("testEmpty", testEmpty),
146141
("testApplyWildcards", testApplyWildcards),

‎Tests/ContentBlockerConverterTests/GeneralTests.swift

+4-4
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,8 @@ final class GeneralTests: XCTestCase {
345345
let conversionResult = ContentBlockerConverter().convertArray(rules: rules)
346346
NSLog(conversionResult.message)
347347

348-
XCTAssertEqual(conversionResult.totalConvertedCount, 20193)
349-
XCTAssertEqual(conversionResult.convertedCount, 20193)
348+
XCTAssertEqual(conversionResult.totalConvertedCount, 19999)
349+
XCTAssertEqual(conversionResult.convertedCount, 19999)
350350
XCTAssertEqual(conversionResult.errorsCount, 128)
351351
XCTAssertEqual(conversionResult.overLimit, false)
352352
}
@@ -363,8 +363,8 @@ final class GeneralTests: XCTestCase {
363363
let conversionResult = ContentBlockerConverter().convertArray(rules: rules)
364364
NSLog(conversionResult.message)
365365

366-
XCTAssertEqual(conversionResult.totalConvertedCount, 20193)
367-
XCTAssertEqual(conversionResult.convertedCount, 20193)
366+
XCTAssertEqual(conversionResult.totalConvertedCount, 19999)
367+
XCTAssertEqual(conversionResult.convertedCount, 19999)
368368
XCTAssertEqual(conversionResult.errorsCount, 128)
369369
XCTAssertEqual(conversionResult.overLimit, false)
370370
}

0 commit comments

Comments
 (0)