Issue 7269 - Restrict URL rewrites to GETs only

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 180 matching lines @@
   let [filter, thirdParty] = matchRequest(url, type, docDomain,
                                           sitekey, specificOnly);
 
   let result;
   let rewrittenUrl;
 
   if (filter instanceof BlockingFilter)
   {
     if (typeof filter.rewrite == "string")
     {
-      rewrittenUrl = filter.rewriteUrl(details.url);
-      // If no rewrite happened (error, different origin), we'll
-      // return undefined in order to avoid an "infinite" loop.
-      if (rewrittenUrl != details.url)
-        result = {redirectUrl: rewrittenUrl};
+      if (details.method == "GET")

[Sebastian Noack, 2019/02/08 22:07:59]

Nit: Any reason for introducing another block? Can't you just combine it with
the above check?

  if (typeof filter.rewrite == "string" && details.method == "GET")
  {
    ...
  }

[a.giammarchi, 2019/02/09 13:27:20]

I think if we go for `if (details.method == "OPTIONS") return;` we can move all
together this check to the top most part of the function.

+      {
+        rewrittenUrl = filter.rewriteUrl(details.url);
+        // If no rewrite happened (error, different origin), we'll
+        // return undefined in order to avoid an "infinite" loop.
+        if (rewrittenUrl != details.url)
+          result = {redirectUrl: rewrittenUrl};

[a.giammarchi, 2019/02/08 12:10:27]

Minor aesthetic improvement: how about we rename `rewrittenUrl` as `redirectUrl`
so that `result = {redirectUrl}` since it's `redirectUrl` that we want to set at
the end, and never `reqrittenUrl` ?

[hub, 2019/02/08 17:05:26]

it is named this way because it is the $rewrite option.

+      }
     }
     else
       result = {cancel: true};
   }
 
   getRelatedTabIds(details).then(tabIds =>
   {
     logRequest(
       tabIds,
       {
@@ skipping 114 matching lines @@
 }
 
 filterNotifier.on("subscription.added", onFilterChange);
 filterNotifier.on("subscription.removed", arg => onFilterChange(arg, false));
 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);