Skip to content

Commit cef6a53

Browse files
bershanskiybershanskiy
authored
Refactor: filter nonstandard URL patterns in background (darkreader#10810)
1 parent e4a3a74 commit cef6a53

3 files changed

Lines changed: 148 additions & 58 deletions

File tree

‎src/generators/utils/parse.ts‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {isFullyQualifiedDomain, isFullyQualifiedDomainWildcard, fullyQualifiedDomainMatchesWildcard, isURLInList} from '../../utils/url';
1+
import {isFullyQualifiedDomain, isFullyQualifiedDomainWildcard, fullyQualifiedDomainMatchesWildcard, isURLInList, isURLMatched} from '../../utils/url';
22
import {parseArray} from '../../utils/text';
33

44
declare const __TEST__: boolean;
@@ -25,7 +25,7 @@ export interface SitePropsIndex<SiteFix extends SiteProps> {
2525
interface ConfigIndex {
2626
domains: { [domain: string]: number[] };
2727
domainLabels: { [domainLabel: string]: number[] };
28-
nonstandard: number[];
28+
nonstandard: number[] | null;
2929
}
3030

3131
export interface SiteListIndex {
@@ -235,8 +235,21 @@ export function indexSitesFixesConfig<T extends SiteProps>(text: string): SitePr
235235
return {offsets: encodeOffsets(offsets), domains, domainLabels, nonstandard, cacheDomainIndex: {}, cacheSiteFix: {}, cacheCleanupTimer: null};
236236
}
237237

238-
function lookupConfigURLs(url: string, index: ConfigIndex, getAllRecordURLs: (id: number) => string[]): number[] {
239-
const domain = getDomain(url);
238+
function lookupConfigURLsInDomainLabels(domain: string, recordIds: number[], currRecordIds: number[], getAllRecordURLs: (id: number) => string[]) {
239+
for (const recordId of currRecordIds) {
240+
const recordURLs = getAllRecordURLs(recordId);
241+
for (const ruleUrl of recordURLs) {
242+
const wildcard = getDomain(ruleUrl);
243+
if (isFullyQualifiedDomainWildcard(wildcard) && fullyQualifiedDomainMatchesWildcard(wildcard, domain)) {
244+
recordIds.push(recordId);
245+
} else {
246+
// Skip this rule, since the label match must have come from a different URL
247+
}
248+
}
249+
}
250+
}
251+
252+
function lookupConfigURLs(domain: string, index: ConfigIndex, getAllRecordURLs: (id: number) => string[]): number[] {
240253
const labels = domain.split('.');
241254
let recordIds: number[] = [];
242255

@@ -250,17 +263,7 @@ function lookupConfigURLs(url: string, index: ConfigIndex, getAllRecordURLs: (id
250263
// We need to use in operator because ids are 0-based and 0 is falsy
251264
if (label in index.domainLabels) {
252265
const currRecordIds = index.domainLabels[label];
253-
for (const recordId of currRecordIds) {
254-
const recordURLs = getAllRecordURLs(recordId);// getSiteFix<T>(text, index, options, recordId);
255-
for (const ruleUrl of recordURLs) {
256-
const wildcard = getDomain(ruleUrl);
257-
if (isFullyQualifiedDomainWildcard(wildcard) && fullyQualifiedDomainMatchesWildcard(wildcard, domain)) {
258-
recordIds.push(recordId);
259-
} else {
260-
// Skip this rule, since the label match must have come from a different URL
261-
}
262-
}
263-
}
266+
lookupConfigURLsInDomainLabels(domain, recordIds, currRecordIds, getAllRecordURLs);
264267
}
265268
}
266269

@@ -270,12 +273,22 @@ function lookupConfigURLs(url: string, index: ConfigIndex, getAllRecordURLs: (id
270273
recordIds = recordIds.concat(index.domains[substring]);
271274
}
272275
if (substring in index.domainLabels) {
273-
recordIds = recordIds.concat(index.domainLabels[substring]);
276+
const currRecordIds = index.domainLabels[substring];
277+
lookupConfigURLsInDomainLabels(domain, recordIds, currRecordIds, getAllRecordURLs);
274278
}
275279
}
276-
// Backwards compatibility: send over nonstandard patterns, which will be filtered out
280+
281+
// Backwards compatibility: check for nonssend over nonstandard patterns, which will be filtered out
277282
// via regex in content script
278-
recordIds = recordIds.concat(index.nonstandard);
283+
if (index.nonstandard) {
284+
for (const currRecordId of index.nonstandard) {
285+
const urls = getAllRecordURLs(currRecordId);
286+
if (urls.some((url) => isURLMatched(domain, getDomain(url)))) {
287+
recordIds.push(currRecordId);
288+
continue;
289+
}
290+
}
291+
}
279292

280293
// Deduplicate array elements
281294
recordIds = Array.from(new Set(recordIds));

‎tests/browser/e2e/devtools.tests.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ async function expectStyles(styles: StyleExpectations) {
2626
}
2727

2828
describe('Modifying config via Developer tools', () => {
29+
// TODO: remove flakes and remove this line
30+
jest.retryTimes(10, {logErrorsBeforeRetry: true});
31+
2932
it('Modifying config', async () => {
3033
await loadBasicPage();
3134

‎tests/unit/generators/utils/parse.tests.ts‎

Lines changed: 114 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -450,49 +450,123 @@ describe('Explicit wildcard domain patterns', () => {
450450

451451
describe('Backwards compatibility', () => {
452452
describe('Nonstandard patterns', () => {
453-
interface TestFix {
454-
url: string[];
455-
directive: string;
456-
}
457-
458-
const directiveMap: { [key: string]: keyof TestFix } = {
459-
DIRECTIVE: 'directive',
460-
};
461-
462-
const config = [
463-
'*',
464-
'',
465-
'DIRECTIVE',
466-
'hello world',
467-
'',
468-
'====================',
469-
'',
470-
'example*.com',
471-
'',
472-
'DIRECTIVE',
473-
'one',
474-
''
475-
].join('\n');
453+
it('Clearly non-matching pattern', () => {
454+
interface TestFix {
455+
url: string[];
456+
directive: string;
457+
}
476458

477-
const options: SitesFixesParserOptions<TestFix> = {
478-
commands: Object.keys(directiveMap),
479-
getCommandPropName: (command) => directiveMap[command],
480-
parseCommandValue: (_, value) => value.trim(),
481-
};
482-
const index = indexSitesFixesConfig<TestFix>(config);
459+
const directiveMap: { [key: string]: keyof TestFix } = {
460+
DIRECTIVE: 'directive',
461+
};
462+
463+
const config = [
464+
'*',
465+
'',
466+
'DIRECTIVE',
467+
'hello world',
468+
'',
469+
'====================',
470+
'',
471+
'example*.com',
472+
'',
473+
'DIRECTIVE',
474+
'one',
475+
''
476+
].join('\n');
477+
478+
const options: SitesFixesParserOptions<TestFix> = {
479+
commands: Object.keys(directiveMap),
480+
getCommandPropName: (command) => directiveMap[command],
481+
parseCommandValue: (_, value) => value.trim(),
482+
};
483+
const index = indexSitesFixesConfig<TestFix>(config);
484+
485+
const fixes = getSitesFixesFor<TestFix>('other.net', config, index, options);
486+
expect(fixes).toEqual([
487+
{
488+
'url': ['*'],
489+
'directive': 'hello world'
490+
}
491+
]);
492+
});
483493

484-
const fixes = getSitesFixesFor<TestFix>('other.net', config, index, options);
485-
expect(fixes).toEqual([
486-
{
487-
'url': ['*'],
488-
'directive': 'hello world'
489-
}, {
490-
'url': [
491-
'example*.com',
492-
],
493-
'directive': 'one'
494+
it('Legacy matching pattern', () => {
495+
interface TestFix {
496+
url: string[];
497+
directive: string;
494498
}
495-
]);
499+
500+
const directiveMap: { [key: string]: keyof TestFix } = {
501+
DIRECTIVE: 'directive',
502+
};
503+
504+
const config = [
505+
'*',
506+
'',
507+
'DIRECTIVE',
508+
'hello world',
509+
'',
510+
'====================',
511+
'',
512+
'example*',
513+
'',
514+
'DIRECTIVE',
515+
'one',
516+
''
517+
].join('\n');
518+
519+
const options: SitesFixesParserOptions<TestFix> = {
520+
commands: Object.keys(directiveMap),
521+
getCommandPropName: (command) => directiveMap[command],
522+
parseCommandValue: (_, value) => value.trim(),
523+
};
524+
const index = indexSitesFixesConfig<TestFix>(config);
525+
526+
const fixes = getSitesFixesFor<TestFix>('example.com', config, index, options);
527+
expect(fixes).toEqual([
528+
{
529+
'url': ['*'],
530+
'directive': 'hello world'
531+
},
532+
{
533+
'url': ['example*'],
534+
'directive': 'one'
535+
}
536+
]);
537+
538+
const fixes2 = getSitesFixesFor<TestFix>('example.deep.com', config, index, options);
539+
expect(fixes2).toEqual([
540+
{
541+
'url': ['*'],
542+
'directive': 'hello world'
543+
},
544+
{
545+
'url': ['example*'],
546+
'directive': 'one'
547+
}
548+
]);
549+
550+
const fixes3 = getSitesFixesFor<TestFix>('nonexample.com', config, index, options);
551+
expect(fixes3).toEqual([
552+
{
553+
'url': ['*'],
554+
'directive': 'hello world'
555+
}
556+
]);
557+
558+
const fixes4 = getSitesFixesFor<TestFix>('deep.example.com', config, index, options);
559+
expect(fixes4).toEqual([
560+
{
561+
'url': ['*'],
562+
'directive': 'hello world'
563+
},
564+
{
565+
'url': ['example*'],
566+
'directive': 'one'
567+
}
568+
]);
569+
});
496570
});
497571
});
498572

0 commit comments

Comments
 (0)