Issue 6969 - Implement abort-on-property-read snippet

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 475 matching lines @@
     {
       event.preventDefault();
     }
   },
   true);
 }
 
 exports["prevent-inline-scripts"] = preventInlineScripts;
 
 /**
- * Generates an alphanumeric ID consisting of 5 randomly 6 base-36 digits
- * from the range 100000..zzzzzz (both inclusive) concatenated.
+ * Generates a random alphanumeric ID consisting of 6 base-36 digits
+ * from the range 100000..zzzzzz (both inclusive).
  *
  * @returns {string} The random ID.
  */
 function randomId()

[Manish Jethani, 2018/10/25 22:22:52]

Do you like this version of the function?

I don't understand why we're using base-36 here. If we need a random number of
up to N bits, let's say N being 64, then we can just do this:

  Math.floor(Math.random() * (2 ** 32)).toString(16).padStart(8, "0") +
  Math.floor(Math.random() * (2 ** 32)).toString(16).padStart(8, "0")

This would be straightforward to document, as in "Generates a 64-bit
psuedo-random number in hexadecimal format". There's no reason to even mention a
range, the range is whatever can fit in 64 bits (16 hexadecimal digits).

Anyway, if you don't like the idea then let's just revert to the previous
version, because now the JSDoc and the implementation are both getting weird. I
think you'll agree that it was better before.

[Manish Jethani, 2018/10/25 22:59:57]

Just to clarify, I'm sorry about leading you down this path and I now realize
that it was not such a great idea. Let's revert to the previous version of
randomId() (and accept the tiny chance of a collision with a genuine error
message).

[hub, 2018/10/26 15:19:05]

Done.

 {
-  let id = "";
-  //
   // 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
   //
-  for (let i = 0; i < 5; i++)
-    id += Math.floor(Math.random() * 2116316160 + 60466176).toString(36);
-
-  return id;
+  return Math.floor(Math.random() * 2116316160 + 60466176).toString(36);
 }
 
 function wrapPropertyAccess(object, property, descriptor)
 {
   let currentDescriptor = Object.getOwnPropertyDescriptor(object, property);
-  if (!currentDescriptor)
-    currentDescriptor = {};
-  if (typeof descriptor.get != "undefined")
-    currentDescriptor.get = descriptor.get;
-  if (typeof descriptor.set != "undefined")
-    currentDescriptor.set = descriptor.set;
-  Object.defineProperty(object, property, currentDescriptor);
-}
-
-/**
- * Will patch a property on the window object to abort when read.
- * It will intercept the onerror callback and block it if tagged.
+  if (currentDescriptor && !currentDescriptor.configurable)
+    return false;
+
+  Object.defineProperty(object, property, descriptor);
+  return true;
+}
+
+/**
+ * Patches a property on the window object to abort execution when the
+ * property is read.
+ *
+ * No error is be printed to the console.
+ *
  * The idea originates from
- * https://github.com/uBlockOrigin/uAssets/blob/80b195436f8f8d78ba713237bfc268ecfc9d9d2b/filters/resources.txt#L1703
- *
- * @todo handle properties of properties.
- *
- * @param {string} property the name of the property.
+ * {@link https://github.com/uBlockOrigin/uAssets/blob/80b195436f8f8d78ba713237bfc268ecfc9d9d2b/filters/resources.txt#L1703 uBlock Origin}.
+ *
+ * @param {string} property The name of the property.
  */
 function abortOnPropertyRead(property)
 {
   if (!property)
     return;
 
   let rid = randomId();
 
   function abort()
   {
     throw new ReferenceError(rid);
   }
 
   let {onerror} = window;
-  window.onerror = (message, ...rest) =>
-  {
-    if (typeof message == "string" && message.includes(rid))
-      return true;
-    if (typeof onerror == "function")
-      return onerror.call(this, message, ...rest);

[Manish Jethani, 2018/10/25 22:22:52]

OK, this one I already explained why there's no guarantee that there will be a
call method on the object.

[hub, 2018/10/26 15:19:06]

Done.

-  };
-
-  wrapPropertyAccess(window, property, {get: abort, set() {}});
+  if (wrapPropertyAccess(window, property, {get: abort, set() {}}))
+  {
+    window.onerror = (message, ...rest) =>
+    {
+      if (typeof message == "string" && message.includes(rid))
+        return true;
+      if (typeof onerror == "function")
+        return (() => {}).call.call(onerror, this, message, ...rest);
+    };
+  }
 }
 
 exports["abort-on-property-read"] = makeInjector(abortOnPropertyRead,
                                                  wrapPropertyAccess,
                                                  randomId);