Unified Diff: lib/filterClasses.js
Issue 30046558:
Fixed #4 - Disable $rewrite option for all but internal redirect (Closed)
Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set:
Created April 16, 2019, 1:32 p.m.
Use n/p to move between diff chunks;
N/P to move between comments.
« no previous file with comments
|
« no previous file
|
test/filterClasses.js » ('j')
|
no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')
| Index: lib/filterClasses.js |
| =================================================================== |
| --- a/lib/filterClasses.js |
| +++ b/lib/filterClasses.js |
| @@ -619,43 +619,38 @@ |
| * @param {string} [domains] |
| * Domains that the filter is restricted to, e.g. "foo.com|bar.com|~baz.com" |
| * @param {boolean} [thirdParty] |
| * Defines whether the filter should apply to third-party or first-party |
| * content only |
| * @param {string} [sitekeys] |
| * Public keys of websites that this filter should apply to |
| * @param {?string} [rewrite] |
| - * The (optional) rule specifying how to rewrite the URL. See |
| - * RegExpFilter.prototype.rewrite. |
| - * @param {?string} [resourceName] |
| * The name of the internal resource to which to rewrite the |
| * URL. e.g. if the value of the <code>rewrite</code> parameter is |
| * <code>abp-resource:blank-html</code>, this should be |
| * <code>blank-html</code>. |
| * @constructor |
| * @augments ActiveFilter |
| */ |
| function RegExpFilter(text, regexpSource, contentType, matchCase, domains, |
| - thirdParty, sitekeys, rewrite, resourceName) |
| + thirdParty, sitekeys, rewrite) |
| { |
| ActiveFilter.call(this, text, domains); |
| if (contentType != null) |
| this.contentType = contentType; |
| if (matchCase) |
| this.matchCase = matchCase; |
| if (thirdParty != null) |
| this.thirdParty = thirdParty; |
| if (sitekeys != null) |
| this.sitekeySource = sitekeys; |
| if (rewrite != null) |
| this.rewrite = rewrite; |
| - if (resourceName) |
| - this.resourceName = resourceName; |
| if (!this.matchCase) |
| regexpSource = regexpSource.toLowerCase(); |
| if (regexpSource.length >= 2 && |
| regexpSource[0] == "/" && |
| regexpSource[regexpSource.length - 1] == "/") |
| { |
| @@ -664,18 +659,17 @@ |
| let regexp = new RegExp(regexpSource.substring(1, regexpSource.length - 1)); |
| Object.defineProperty(this, "regexp", {value: regexp}); |
| } |
| else |
| { |
| // Patterns like /foo/bar/* exist so that they are not treated as regular |
| // expressions. We drop any superfluous wildcards here so our optimizations |
| // can kick in. |
| - if (this.rewrite == null || this.resourceName) |
| - regexpSource = regexpSource.replace(/^\*+/, "").replace(/\*+$/, ""); |
| + regexpSource = regexpSource.replace(/^\*+/, "").replace(/\*+$/, ""); |
| // No need to convert this filter to regular expression yet, do it on demand |
| this.pattern = regexpSource; |
| } |
| } |
| exports.RegExpFilter = RegExpFilter; |
| RegExpFilter.prototype = extend(ActiveFilter, { |
| @@ -701,19 +695,19 @@ |
| /** |
| * Regular expression to be used when testing against this filter |
| * @type {RegExp} |
| */ |
| get regexp() |
| { |
| let value = null; |
| - let {pattern, rewrite, resourceName} = this; |
| - if ((rewrite != null && !resourceName) || !isLiteralPattern(pattern)) |
| - value = new RegExp(filterToRegExp(pattern, rewrite != null)); |
| + let {pattern} = this; |
| + if (!isLiteralPattern(pattern)) |
| + value = new RegExp(filterToRegExp(pattern)); |
|
hub
2019/04/16 13:44:27
There was a conflict here (removal of the last arg
Manish Jethani
2019/04/16 14:06:23
Acknowledged.
|
| Object.defineProperty(this, "regexp", {value}); |
| return value; |
| }, |
| /** |
| * Content types the filter applies to, combination of values from |
| * RegExpFilter.typeMap |
| * @type {number} |
| @@ -753,30 +747,23 @@ |
| Object.defineProperty( |
| this, "sitekeys", {value: sitekeys, enumerable: true} |
| ); |
| return this.sitekeys; |
| }, |
| /** |
| - * The rule specifying how to rewrite the URL. |
| - * The syntax is similar to the one of String.prototype.replace(). |
| - * @type {?string} |
| - */ |
| - rewrite: null, |
| - |
| - /** |
| * The name of the internal resource to which to rewrite the |
| - * URL. e.g. if the value of the <code>rewrite</code> property is |
| + * URL. e.g. if the value of the <code>$rewrite</code> option is |
| * <code>abp-resource:blank-html</code>, this should be |
| * <code>blank-html</code>. |
| * @type {?string} |
| */ |
| - resourceName: null, |
| + rewrite: null, |
| /** |
| * Tests whether the URL request matches this filter |
| * @param {URLRequest} request URL request to be tested |
| * @param {number} typeMask bitmask of content / request types to match |
| * @param {string} [sitekey] public key provided by the document |
| * @return {boolean} true in case of a match |
| */ |
| @@ -899,17 +886,16 @@ |
| let contentType = null; |
| let matchCase = null; |
| let domains = null; |
| let sitekeys = null; |
| let thirdParty = null; |
| let collapse = null; |
| let csp = null; |
| let rewrite = null; |
| - let resourceName = null; |
| let options; |
| let match = text.includes("$") ? Filter.optionsRegExp.exec(text) : null; |
| if (match) |
| { |
| options = match[1].split(","); |
| text = match.input.substring(0, match.index); |
| for (let option of options) |
| { |
| @@ -967,48 +953,35 @@ |
| case "sitekey": |
| if (!value) |
| return new InvalidFilter(origText, "filter_unknown_option"); |
| sitekeys = value.toUpperCase(); |
| break; |
| case "rewrite": |
| if (value == null) |
| return new InvalidFilter(origText, "filter_unknown_option"); |
| - rewrite = value; |
| - if (value.startsWith("abp-resource:")) |
| - resourceName = value.substring("abp-resource:".length); |
| + if (!value.startsWith("abp-resource:")) |
| + return new InvalidFilter(origText, "filter_invalid_rewrite"); |
| + rewrite = value.substring("abp-resource:".length); |
|
hub
2019/04/16 13:44:27
There was another conflict here. We changed from s
Manish Jethani
2019/04/16 14:06:23
Acknowledged.
|
| break; |
| default: |
| return new InvalidFilter(origText, "filter_unknown_option"); |
| } |
| } |
| } |
| } |
| - // For security reasons, never match $rewrite filters |
| - // against requests that might load any code to be executed. |
| - // Unless it is to an internal resource. |
| - if (rewrite != null && !resourceName) |
| - { |
| - if (contentType == null) |
| - ({contentType} = RegExpFilter.prototype); |
| - contentType &= ~(RegExpFilter.typeMap.SCRIPT | |
| - RegExpFilter.typeMap.SUBDOCUMENT | |
| - RegExpFilter.typeMap.OBJECT | |
| - RegExpFilter.typeMap.OBJECT_SUBREQUEST); |
| - } |
| - |
| try |
| { |
| if (blocking) |
| { |
| if (csp && Filter.invalidCSPRegExp.test(csp)) |
| return new InvalidFilter(origText, "filter_invalid_csp"); |
| - if (resourceName) |
| + if (rewrite) |
| { |
| if (text[0] == "|" && text[1] == "|") |
| { |
| if (!domains && thirdParty != false) |
| return new InvalidFilter(origText, "filter_invalid_rewrite"); |
| } |
| else if (text[0] == "*") |
| { |
| @@ -1017,17 +990,17 @@ |
| } |
| else |
| { |
| return new InvalidFilter(origText, "filter_invalid_rewrite"); |
| } |
| } |
| return new BlockingFilter(origText, text, contentType, matchCase, domains, |
| - thirdParty, sitekeys, rewrite, resourceName, |
| + thirdParty, sitekeys, rewrite, |
| collapse, csp); |
| } |
| return new WhitelistFilter(origText, text, contentType, matchCase, domains, |
| thirdParty, sitekeys); |
| } |
| catch (e) |
| { |
| return new InvalidFilter(origText, "filter_invalid_regexp"); |
| @@ -1078,36 +1051,33 @@ |
| * @param {string} text see {@link Filter Filter()} |
| * @param {string} regexpSource see {@link RegExpFilter RegExpFilter()} |
| * @param {number} [contentType] see {@link RegExpFilter RegExpFilter()} |
| * @param {boolean} [matchCase] see {@link RegExpFilter RegExpFilter()} |
| * @param {string} [domains] see {@link RegExpFilter RegExpFilter()} |
| * @param {boolean} [thirdParty] see {@link RegExpFilter RegExpFilter()} |
| * @param {string} [sitekeys] see {@link RegExpFilter RegExpFilter()} |
| * @param {?string} [rewrite] |
| - * The (optional) rule specifying how to rewrite the URL. See |
| - * RegExpFilter.prototype.rewrite. |
| - * @param {?string} [resourceName] |
| * The name of the internal resource to which to rewrite the |
| * URL. e.g. if the value of the <code>rewrite</code> parameter is |
|
Manish Jethani
2019/04/16 13:54:06
We forgot this in master but let's do it here: $re
hub
2019/04/16 14:08:39
The function docs didn't get that fix. Doing both.
|
| * <code>abp-resource:blank-html</code>, this should be |
| * <code>blank-html</code>. |
| * @param {boolean} [collapse] |
| * defines whether the filter should collapse blocked content, can be null |
| * @param {string} [csp] |
| * Content Security Policy to inject when the filter matches |
| * @constructor |
| * @augments RegExpFilter |
| */ |
| function BlockingFilter(text, regexpSource, contentType, matchCase, domains, |
| - thirdParty, sitekeys, rewrite, resourceName, |
| + thirdParty, sitekeys, rewrite, |
| collapse, csp) |
| { |
| RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains, |
| - thirdParty, sitekeys, rewrite, resourceName); |
| + thirdParty, sitekeys, rewrite); |
| if (collapse != null) |
| this.collapse = collapse; |
| if (csp != null) |
| this.csp = csp; |
| } |
| exports.BlockingFilter = BlockingFilter; |
| @@ -1130,30 +1100,17 @@ |
| /** |
| * Rewrites an URL. |
| * @param {string} url the URL to rewrite |
| * @return {string} the rewritten URL, or the original in case of failure |
| */ |
| rewriteUrl(url) |
| { |
| - if (this.resourceName) |
| - return resourceMap.get(this.resourceName) || url; |
| - |
| - try |
| - { |
| - let rewrittenUrl = new URL(url.replace(this.regexp, this.rewrite), url); |
| - if (rewrittenUrl.origin == new URL(url).origin) |
| - return rewrittenUrl.href; |
| - } |
| - catch (e) |
| - { |
| - } |
| - |
| - return url; |
| + return resourceMap.get(this.rewrite) || url; |
| } |
| }); |
| /** |
| * Class for whitelist filters |
| * @param {string} text see {@link Filter Filter()} |
| * @param {string} regexpSource see {@link RegExpFilter RegExpFilter()} |
| * @param {number} [contentType] see {@link RegExpFilter RegExpFilter()} |
« no previous file with comments
|
« no previous file
|
test/filterClasses.js » ('j')
|
no next file with comments »
