Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/filterClasses.js

Issue 30045566: Fixed #4 - Disable rewrite option for all but internal redirect (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Reworked patch Created April 16, 2019, 4:55 a.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/filterClasses.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/filterClasses.js
===================================================================
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -699,60 +699,55 @@
* @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 (regexpSource.length >= 2 &&
regexpSource[0] == "/" &&
regexpSource[regexpSource.length - 1] == "/")
{
// The filter is a regular expression - convert it immediately to
// catch syntax errors
let regexp = new RegExp(regexpSource.substr(1, regexpSource.length - 2),
this.matchCase ? "" : "i");
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)
+ if (this.rewrite)
Manish Jethani 2019/04/16 05:52:58 This actually has a somewhat opposite of the inten
hub 2019/04/16 12:23:06 Good catch.
regexpSource = regexpSource.replace(/^\*+/, "").replace(/\*+$/, "");
if (!this.matchCase && isLiteralPattern(regexpSource))
regexpSource = regexpSource.toLowerCase();
// No need to convert this filter to regular expression yet, do it on demand
this.pattern = regexpSource;
}
@@ -782,22 +777,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),
- this.matchCase ? "" : "i");
- }
+ let {pattern} = this;
+ if (!isLiteralPattern(pattern))
+ value = new RegExp(filterToRegExp(pattern), this.matchCase ? "" : "i");
Object.defineProperty(this, "regexp", {value});
return value;
},
/**
* Content types the filter applies to, combination of values from
* RegExpFilter.typeMap
* @type {number}
@@ -837,30 +829,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
Manish Jethani 2019/04/16 05:52:58 I think we can replace `<code>rewrite</code> prope
hub 2019/04/16 12:23:06 Done.
* <code>abp-resource:blank-html</code>, this should be
* <code>blank-html</code>.
* @type {?string}
*/
- resourceName: null,
+ rewrite: null,
/**
* Tests whether the URL matches this filter
* @param {string} location URL to be tested
* @param {number} typeMask bitmask of content / request types to match
* @param {string} [docDomain] domain name of the document that loads the URL
* @param {boolean} [thirdParty] should be true if the URL is a third-party
* request
@@ -989,17 +974,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.substr(0, match.index);
for (let option of options)
{
@@ -1055,50 +1039,35 @@
collapse = !inverse;
break;
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.substr("abp-resource:".length);
+ if (!value || !value.startsWith("abp-resource:"))
Manish Jethani 2019/04/16 05:52:58 It doesn't matter _that_ much, but I'd just like t
hub 2019/04/16 12:23:06 I think I'm gonna go the safe route. Change was in
+ return new InvalidFilter(origText, "filter_invalid_rewrite");
+ rewrite = value.substr("abp-resource:".length);
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] == "*")
{
@@ -1107,17 +1076,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");
@@ -1168,36 +1137,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
* <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;
@@ -1220,30 +1186,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 »

Powered by Google App Engine
This is Rietveld