Issue 6622 - Implement $rewrite filter option

lib/requestBlocker.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 52 matching lines @@
   if (!(browser.webRequest.ResourceType))
     return;
 
   for (let type in browser.webRequest.ResourceType)
     yield resourceTypes.get(browser.webRequest.ResourceType[type]) || "OTHER";
 
   // WEBRTC gets addressed through a workaround, even if the webRequest API is
   // lacking support to block this kind of a request.
   yield "WEBRTC";
 
-  // POPUP, CSP and ELEMHIDE filters aren't mapped to resource types.
+  // These filter types aren't mapped to resource types.
   yield "POPUP";
   yield "ELEMHIDE";
   yield "CSP";
 }());
 
 function getDocumentInfo(page, frame, originUrl)
 {
   return [
     extractHostFromFrame(frame, originUrl),
     getKey(page, frame, originUrl),
@@ skipping 32 matching lines @@
     url = details.initiator + "/*";  // is equivalent to "originUrl" on Firefox
                                      // except that its not a full URL but just
                                      // an origin (proto + host).
   else
     return Promise.resolve([]);
 
   return browser.tabs.query({url}).then(tabs => tabs.map(tab => tab.id));
 }
 
 function logRequest(tabIds, url, type, docDomain, thirdParty,
-                    sitekey, specificOnly, filter)
+                    sitekey, specificOnly, filter, rewrittenTo)
 {
   if (filter)
     FilterNotifier.emit("filter.hitCount", filter, 0, 0, tabIds);
 
   devtools.logRequest(
     tabIds, url, type, docDomain,
     thirdParty, sitekey,
-    specificOnly, filter
+    specificOnly, filter, rewrittenTo
   );
 }
 
 browser.webRequest.onBeforeRequest.addListener(details =>
 {
   // Never block top-level documents.
   if (details.type == "main_frame")
     return;
 
   // Filter out requests from non web protocols. Ideally, we'd explicitly
@@ skipping 42 matching lines @@
 
   if (checkWhitelisted(page, frame, originUrl))
     return;
 
   let type = resourceTypes.get(details.type) || "OTHER";
   let [docDomain, sitekey, specificOnly] = getDocumentInfo(page, frame,
                                                            originUrl);
   let [filter, urlString, thirdParty] = matchRequest(url, type, docDomain,
                                                      sitekey, specificOnly);
 
+  let result;
+  let rewrittenTo;
+
+  if (filter instanceof BlockingFilter)
+  {
+    if (filter.rewrite)
+    {
+      let rewritten = filter.rewriteUrl(urlString);

[Manish Jethani, 2018/05/04 20:49:47]

We don't need rewritten, we can just assign rewrittenTo here.

[hub, 2018/05/04 20:51:49]

already done

+      if (rewritten == urlString)

[Manish Jethani, 2018/05/04 20:49:47]

Nit: since the if block spans multiple lines, we should add braces around it.

[hub, 2018/05/04 20:51:49]

Acknowledged.

+        // we couldn't do the rewrite, so just let it through.

[Sebastian Noack, 2018/05/04 19:46:36]

The request should still be logged.

Just for clarification, bailing here is necessary because the onBeforeRequest
listener would be called infinite times if we keep redirecting to the same URL?

[Manish Jethani, 2018/05/04 20:06:12]

Agreed.
I checked on both the latest versions of Chrome and Firefox. If the redirect URL
is the same as the current URL, Chrome will do nothing (simply let the URL
through) while Firefox will actually do a redirection to the same URL, but only
a fixed number of times.

Naturally the number is 42.

[Manish Jethani, 2018/05/04 20:27:24]

To be clear, I'm OK with the check here even though it would be unusual in
practice to come up with a rewrite that doesn't change anything. I could see it
happening with, for example, "/(?foo=)\d+/$rewrite=$10" if the matched substring
is already "?foo=0" (which it will be after the first rewrite).

If we're going to add the check then I think we should also not log the hit,
because otherwise in the preceding example we would have two entries, one for
"?foo=123" and one for "?foo=0". Also in some cases the rewrite will fail
because of the same-origin policy we have.

We could update the comment to something like:

  // Do not redirect to the same URL, lest we end up in a potentially infinite
redirect loop.

[hub, 2018/05/04 20:48:08]

Ok, I'll log.

[hub, 2018/05/04 20:48:08]

Last time I checked, Chrome (66) did that too.

[hub, 2018/05/04 20:48:08]

Done this.

+        return;
+
+      rewrittenTo = rewritten;
+      result = {redirectUrl: rewritten};
+    }
+    else
+      result = {cancel: true};
+  }
+
   getRelatedTabIds(details).then(tabIds =>
   {
     logRequest(tabIds, urlString, type, docDomain,
-               thirdParty, sitekey, specificOnly, filter);
+               thirdParty, sitekey, specificOnly, filter, rewrittenTo);
   });
 
-  if (filter instanceof BlockingFilter)
-    return {cancel: true};
+  return result;
 }, {urls: ["<all_urls>"]}, ["blocking"]);
 
 port.on("filters.collapse", (message, sender) =>
 {
   let {page, frame} = sender;
 
   if (checkWhitelisted(page, frame))
     return false;
 
   let blocked = false;
@@ skipping 94 matching lines @@
 }
 
 FilterNotifier.on("subscription.added", onFilterChange);
 FilterNotifier.on("subscription.removed", onFilterChange);
 FilterNotifier.on("subscription.updated", onFilterChange);
 FilterNotifier.on("subscription.disabled", arg => onFilterChange(arg, true));
 FilterNotifier.on("filter.added", onFilterChange);
 FilterNotifier.on("filter.removed", onFilterChange);
 FilterNotifier.on("filter.disabled", arg => onFilterChange(arg, true));
 FilterNotifier.on("load", onFilterChange);