Issue 7236 - Handle sub properties in abort-on-property snippets

lib/content/snippets.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 680 matching lines @@
 function randomId()
 {
   // 2176782336 is 36^6 which mean 6 chars [a-z0-9]
   // 60466176 is 36^5
   // 2176782336 - 60466176 = 2116316160. This ensure to always have 6
   // chars even if Math.random() returns its minimum value 0.0
   //
   return Math.floor(Math.random() * 2116316160 + 60466176).toString(36);
 }
 
-function wrapPropertyAccess(object, property, descriptor, magic)
-{
-  let dot = property.indexOf(".");
-  if (dot == -1)
+function wrapPropertyAccess(object, property, descriptor)
+{
+  let dotIndex = property.indexOf(".");
+  if (dotIndex == -1)
   {
     // simple property case.
     let currentDescriptor = Object.getOwnPropertyDescriptor(object, property);
     if (currentDescriptor && !currentDescriptor.configurable)
-      return false;
+      return;
 
     Object.defineProperty(object, property, descriptor);
-    return true;
+    return;
   }
-  let prop = property.slice(0, dot);

[Manish Jethani, 2019/02/03 14:30:38]

We could just call this `name` instead of `prop`.

[hub, 2019/02/04 15:48:02]

Done.

-  property = property.slice(dot + 1);
-  let value = object[prop];
-  if (typeof value != "undefined")

[Manish Jethani, 2019/02/03 14:30:38]

What if `value` is `null`? I think we want to check `value != null`.

[hub, 2019/02/04 15:48:02]

Actually I think we want `value intanceof Object` here. Fixing the inconsistency
with below.

-    return wrapPropertyAccess(value, property, descriptor);
-
-  let currentDescriptor = Object.getOwnPropertyDescriptor(object, prop);
-  if (currentDescriptor && currentDescriptor.set &&
-      magic && currentDescriptor.set.hasOwnProperty(magic))
-  {
-    return true;
-  }
-
-  let v;
-  let setter = a =>
-  {
-    v = a;
-    if (a instanceof Object)

[Manish Jethani, 2019/02/03 14:30:38]

This check is inconsistent with the one above. I think we can just check `a !=
null`. All other values will get coerced to object.

Oh wait, but `Object.defineProperty` will fail. Well then let's just check for
`a && typeof a == "object"`?

[hub, 2019/02/04 15:48:02]

`a instanceof Object` is false if a is null. Unlike `typeof a` that will be
`object` even for null.

[Manish Jethani, 2019/02/04 17:56:33]

We've been here before:

https://hg.adblockplus.org/adblockpluscore/file/91cd83c4c272/lib/content/snip...

instanceof is more expensive which is why we avoid it in Core. It doesn't matter
for snippets, but for consistency I think we can continue using `a && typeof a
== "object"`.

[hub, 2019/02/04 19:54:36]

So be it. And this actually make the test pass on Firefox.

-      wrapPropertyAccess(a, property, descriptor, magic);
+
+  let name = property.slice(0, dotIndex);
+  property = property.slice(dotIndex + 1);
+  let value = object[name];
+  if (value && typeof value == "object")
+    wrapPropertyAccess(value, property, descriptor);
+
+  let currentDescriptor = Object.getOwnPropertyDescriptor(object, name);
+  if (currentDescriptor && !currentDescriptor.configurable)
+    return;
+
+  let setter = newValue =>
+  {
+    value = newValue;
+    if (newValue && typeof newValue == "object")
+      wrapPropertyAccess(newValue, property, descriptor);
   };
-  setter[magic] = undefined;

[Manish Jethani, 2019/02/03 14:30:38]

If we just want to tag the setter, I think it's better to set it to `null`
rather than `undefined`.

[hub, 2019/02/04 15:48:03]

undefined is good. We check it with hasOwnProperty(). 

-  Object.defineProperty(object, prop, {get: () => v, set: setter});
+
+  Object.defineProperty(object, name, {get: () => value, set: setter});
 }
 
 /**
  * Overrides the <code>onerror</code> handler to discard tagged error messages
  * from our property wrapping.
  *
  * @param {string} magic The magic string that tags the error message.
  */
 function overrideOnError(magic)
 {
@@ skipping 23 matching lines @@
   if (!property)
     return;
 
   let rid = randomId();
 
   function abort()
   {
     throw new ReferenceError(rid);
   }
 
-  if (wrapPropertyAccess(window, property, {get: abort, set() {}}, rid))
-    overrideOnError(rid);
+  wrapPropertyAccess(window, property, {get: abort, set() {}});
+  overrideOnError(rid);
 }
 
 exports["abort-on-property-read"] = makeInjector(abortOnPropertyRead,
                                                  wrapPropertyAccess,
                                                  overrideOnError,
                                                  randomId);
 
 /**
  * Patches a property on the window object to abort execution when the
  * property is written.
@@ skipping 10 matching lines @@
   if (!property)
     return;
 
   let rid = randomId();
 
   function abort()
   {
     throw new ReferenceError(rid);
   }
 
-  if (wrapPropertyAccess(window, property, {set: abort}, rid))
-    overrideOnError(rid);
+  wrapPropertyAccess(window, property, {set: abort});
+  overrideOnError(rid);
 }
 
 exports["abort-on-property-write"] = makeInjector(abortOnPropertyWrite,
                                                   wrapPropertyAccess,
                                                   overrideOnError,
                                                   randomId);
 
 /**
  * Aborts the execution of an inline script.
  *
@@ skipping 12 matching lines @@
 
 
   let object = window;
   let path = api.split(".");
   let name = path.pop();
 
   for (let node of path)
   {
     object = object[node];
 
-    if (!object || typeof object != "object")
+    if (!object || !(typeof object == "object" || typeof object == "function"))
       return;
   }
 
   let currentValue = object[name];
 
   let abort = () =>
   {
     let element = document.currentScript;
     if (element instanceof HTMLScriptElement && element.src == "" &&
         element != us && (!re || re.test(element.textContent)))
     {
       throw new ReferenceError(rid);
     }
   };
 
   let descriptor = {
     get()
     {
       abort();
       return currentValue;
     },
     set(value)
     {
       abort();
       currentValue = value;
     }
   };
 
-  if (wrapPropertyAccess(object, name, descriptor, rid))
-    overrideOnError(rid);
+  wrapPropertyAccess(object, name, descriptor);
+  overrideOnError(rid);
 }
 
 exports["abort-current-inline-script"] =
   makeInjector(abortCurrentInlineScript, wrapPropertyAccess, toRegExp,
                overrideOnError, regexEscape, randomId);
+
+/**
+ * Strips a query string parameter from <code>fetch()</code> calls.
+ *
+ * @param {string} name The name of the parameter.
+ * @param {?string} [urlPattern] An optional pattern that the URL must match.
+ */
+function stripFetchQueryParameter(name, urlPattern = null)
+{
+  let fetch_ = window.fetch;
+  if (typeof fetch_ != "function")
+    return;
+
+  let urlRegExp = urlPattern ? toRegExp(urlPattern) : null;
+  window.fetch = function fetch(...args)
+  {
+    let [source] = args;
+    if (typeof source == "string" &&
+        (!urlRegExp || urlRegExp.test(source)))
+    {
+      let url = new URL(source);
+      url.searchParams.delete(name);
+      args[0] = url.href;
+    }
+    return fetch_.apply(this, args);
+  };
+}
+
+exports["strip-fetch-query-parameter"] = makeInjector(stripFetchQueryParameter,
+                                                      toRegExp, regexEscape);