Skip to content

Commit

Permalink
AG-24787: fixed bad conversion of allowAllRequests rules
Browse files Browse the repository at this point in the history
Merge in ADGUARD-FILTERS/tsurlfilter from fix/AG-24787 to master

Squashed commit of the following:

commit ffbf6ca
Author: Dmitriy Seregin <d.seregin@adguard.com>
Date:   Thu Aug 10 17:50:05 2023 +0400

    remove unused type

commit 2b2026e
Author: Dmitriy Seregin <d.seregin@adguard.com>
Date:   Thu Aug 10 17:46:07 2023 +0400

    added changelog

commit cd39e37
Author: Dmitriy Seregin <d.seregin@adguard.com>
Date:   Thu Aug 10 17:45:30 2023 +0400

    AG-24787: fixed bad conversion of allowAllRequests rules
  • Loading branch information
105th committed Aug 10, 2023
1 parent 45069db commit 8fc2083
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 1 deletion.
3 changes: 3 additions & 0 deletions packages/tsurlfilter/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Support for `$to` modifier in the MV3 converter.
- Support for `$method` modifier in the MV3 converter.

### Fixed
- Bad conversion of `allowAllRequests` rules.


## [2.1.6] - 2023-08-04

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ export enum RuleActionType {
ALLOW = 'allow',
UPGRADE_SCHEME = 'upgradeScheme',
MODIFY_HEADERS = 'modifyHeaders',
/**
* For allowAllRequests rules {@link RuleCondition.resourceTypes} may only
* include the 'sub_frame' and 'main_frame' resource types.
*/
ALLOW_ALL_REQUESTS = 'allowAllRequests',
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,26 @@ export abstract class DeclarativeRuleConverter {
});
}

/**
* Checks if network rule can be converted to {@link RuleActionType.ALLOW_ALL_REQUESTS}.
*
* @param rule Network rule.
*
* @returns Is rule compatible with {@link RuleActionType.ALLOW_ALL_REQUESTS}.
*/
private static isCompatibleWithAllowAllRequests(rule: NetworkRule): boolean {
const types = DeclarativeRuleConverter.getResourceTypes(rule.getPermittedRequestTypes());

const allowedRequestTypes = [ResourceType.MainFrame, ResourceType.SubFrame];

// If found resource type which is incompatible with allowAllRequest field
if (types.some((type) => !allowedRequestTypes.includes(type))) {
return false;
}

return true;
}

/**
* Rule priority.
*
Expand Down Expand Up @@ -368,7 +388,7 @@ export abstract class DeclarativeRuleConverter {
*/
private getAction(rule: NetworkRule): RuleAction {
if (rule.isAllowlist()) {
if (rule.isFilteringDisabled()) {
if (rule.isFilteringDisabled() && DeclarativeRuleConverter.isCompatibleWithAllowAllRequests(rule)) {
return { type: RuleActionType.ALLOW_ALL_REQUESTS };
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
} from '../../../src/rules/declarative-converter/errors/converter-options-errors';
import { UnsupportedModifierError } from '../../../src/rules/declarative-converter/errors/conversion-errors';
import { NetworkRule } from '../../../src/rules/network-rule';
import { RuleActionType } from '../../../src/rules/declarative-converter/declarative-rule';

const createFilter = (
rules: string[],
Expand Down Expand Up @@ -529,4 +530,17 @@ describe('DeclarativeConverter', () => {
expect(errors[0]).toStrictEqual(err);
});
});

it('use only main_frame or sub_frame for allowAllRequests rules', async () => {
const rule = '@@||example.com/*/search?*&method=HEAD$xmlhttprequest,document';
const filter = createFilter([rule]);
const { ruleSets: [ruleSet] } = await converter.convert(
[filter],
);

const { declarativeRules } = await ruleSet.serialize();

expect(declarativeRules).toHaveLength(1);
expect(declarativeRules[0].action.type).not.toContain(RuleActionType.ALLOW_ALL_REQUESTS);
});
});

0 comments on commit 8fc2083

Please sign in to comment.