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

Unified Diff: lib/filterClasses.js

Issue 29760704: Issue 6592 - Implement $rewrite filter option (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Check origin on rewrite. Created May 2, 2018, 10:13 p.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/.eslintrc.json » ('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
@@ -766,16 +766,17 @@
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 options;
let match = (text.indexOf("$") >= 0 ? Filter.optionsRegExp.exec(text) : null);
if (match)
{
options = match[1].split(",");
text = match.input.substr(0, match.index);
for (let option of options)
{
@@ -813,30 +814,32 @@
else if (option == "~THIRD_PARTY")
thirdParty = false;
else if (option == "COLLAPSE")
collapse = true;
else if (option == "~COLLAPSE")
collapse = false;
else if (option == "SITEKEY" && value)
sitekeys = value.toUpperCase();
+ else if (option == "REWRITE" && value)
Manish Jethani 2018/05/03 02:42:36 Note that for the rewrite option an empty string s
hub 2018/05/03 18:40:04 There can't be an empty string. The `optionsRegExp
Manish Jethani 2018/05/04 22:50:48 Well that's a shame, because otherwise foo$rewrite
hub 2018/05/07 20:11:55 Acknowledged.
+ rewrite = value;
else
return new InvalidFilter(origText, "filter_unknown_option");
}
}
try
{
if (blocking)
{
if (csp && Filter.invalidCSPRegExp.test(csp))
return new InvalidFilter(origText, "filter_invalid_csp");
return new BlockingFilter(origText, text, contentType, matchCase, domains,
- thirdParty, sitekeys, collapse, csp);
+ thirdParty, sitekeys, collapse, csp, rewrite);
}
return new WhitelistFilter(origText, text, contentType, matchCase, domains,
thirdParty, sitekeys);
}
catch (e)
{
return new InvalidFilter(origText, "filter_invalid_regexp");
}
@@ -889,27 +892,30 @@
* @param {boolean} matchCase see RegExpFilter()
* @param {string} domains see RegExpFilter()
* @param {boolean} thirdParty see RegExpFilter()
* @param {string} sitekeys see RegExpFilter()
* @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
+ * @param {string} [rewrite]
+ * The rewrite expression
* @constructor
* @augments RegExpFilter
*/
function BlockingFilter(text, regexpSource, contentType, matchCase, domains,
- thirdParty, sitekeys, collapse, csp)
+ thirdParty, sitekeys, collapse, csp, rewrite)
{
RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains,
thirdParty, sitekeys);
this.collapse = collapse;
this.csp = csp;
+ this.rewrite = rewrite;
}
exports.BlockingFilter = BlockingFilter;
BlockingFilter.prototype = extend(RegExpFilter, {
type: "blocking",
/**
* Defines whether the filter should collapse blocked content.
@@ -917,17 +923,47 @@
* @type {boolean}
*/
collapse: null,
/**
* Content Security Policy to inject for matching requests.
* @type {string}
*/
- csp: null
+ csp: null,
+
+ /**
+ * The rewrite expression
+ * @type {string}
+ */
+ rewrite: null,
+
+ /**
+ * Perform the URL rewrite and check the origin.
+ * @param {string} urlString the string URL to rewrite
+ * @returns {string?} the rewritten URL, or null if it doesn't match.
+ */
+ rewriteUrl(urlString)
+ {
+ let rewritten = urlString.replace(this.regexp, this.rewrite);
+ if (rewritten != urlString)
Sebastian Noack 2018/05/02 22:25:03 Why do we need that check? Seems like a premature
hub 2018/05/03 00:57:58 I consider new URL() costly (and we do it twice).
Sebastian Noack 2018/05/03 01:14:52 This code path is only hit for $rewrite filters an
hub 2018/05/03 18:40:04 yup. Will fix it.
+ {
+ try
+ {
+ if (new URL(rewritten).origin != new URL(urlString).origin)
+ rewritten = urlString;
Manish Jethani 2018/05/03 02:42:36 We could return rewritten here and return urlStrin
hub 2018/05/03 18:40:04 Done.
+ }
+ catch (e)
+ {
+ rewritten = urlString;
+ }
+ }
+
+ return rewritten;
+ }
});
/**
* Class for whitelist filters
* @param {string} text see Filter()
* @param {string} regexpSource see RegExpFilter()
* @param {number} contentType see RegExpFilter()
* @param {boolean} matchCase see RegExpFilter()
« no previous file with comments | « no previous file | test/.eslintrc.json » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld