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 682 matching lines @@
   // 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)
 {
-  let dot = property.indexOf(".");

[Manish Jethani, 2019/03/07 14:12:22]

Nit: When finding the index of a dot in a string (domain names), we have used
the name `dotIndex` in other places. Would be nice to be consistent, but up to
you.

[hub, 2019/03/07 21:32:27]

Done.

-  if (dot == -1)
+  let dotIndex = property.indexOf(".");
+  if (dotIndex == -1)
   {
     // simple property case.

[Manish Jethani, 2019/03/07 14:12:22]

Nit: I don't know if this comment adds much value, but it's up to you.

[hub, 2019/03/07 21:32:27]

Acknowledged.

     let currentDescriptor = Object.getOwnPropertyDescriptor(object, property);
     if (currentDescriptor && !currentDescriptor.configurable)
       return;
 
     Object.defineProperty(object, property, descriptor);
     return;
   }
-  let name = property.slice(0, dot);

[Manish Jethani, 2019/03/07 14:12:22]

Nit: A blank line after the `if` block would make it more readable IMO.

[hub, 2019/03/07 21:32:27]

Done.

-  property = property.slice(dot + 1);
+
+  let name = property.slice(0, dotIndex);
+  property = property.slice(dotIndex + 1);
   let value = object[name];
   if (value && typeof value == "object")

[Manish Jethani, 2019/03/07 14:12:22]

Suggestion: It might be better if we defined this formally as:

  let isObject = value => value && typeof value == "object";

But it's your call, I'm not too keen on this either.

[hub, 2019/03/07 21:32:27]

I'll leave it as is

[Manish Jethani, 2019/03/12 07:00:08]

Acknowledged.

     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);
   };
+
   Object.defineProperty(object, name, {get: () => value, set: setter});

[Manish Jethani, 2019/03/07 14:12:23]

Nit: Blank line before this would be nice.

[hub, 2019/03/07 21:32:27]

Done.

 }
 
 /**
  * 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 89 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)))
@@ skipping 45 matching lines @@
       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);