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

lib/content/snippets.js

Index: lib/content/snippets.js
===================================================================
--- a/lib/content/snippets.js
+++ b/lib/content/snippets.js
@@ -693,23 +693,53 @@
   // 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)
+function wrapPropertyAccess(object, property, descriptor, magic)
 {
-  let currentDescriptor = Object.getOwnPropertyDescriptor(object, property);
-  if (currentDescriptor && !currentDescriptor.configurable)
-    return false;
+  let dot = property.indexOf(".");
+  if (dot == -1)
+  {
+    // simple property case.
+    let currentDescriptor = Object.getOwnPropertyDescriptor(object, property);
+    if (currentDescriptor && !currentDescriptor.configurable)
+      return false;
+
+    Object.defineProperty(object, property, descriptor);
+    return true;
+  }
+  let name = property.slice(0, dot);
+  property = property.slice(dot + 1);
+  let value = object[name];
+  if (value && typeof value == "object")
+    return wrapPropertyAccess(value, property, descriptor, magic);
 
-  Object.defineProperty(object, property, descriptor);
+  let currentDescriptor = Object.getOwnPropertyDescriptor(object, name);
+  if (currentDescriptor)
+  {
+    if (currentDescriptor.set && currentDescriptor.set.hasOwnProperty(magic))
+      return true;
+    if (!currentDescriptor.configurable)
+      return false;
+  }
+
+  let v;
+  let setter = a =>
+  {
+    v = a;
+    if (a && typeof a == "object")
+      wrapPropertyAccess(a, property, descriptor, magic);
+  };
+  setter[magic] = undefined;

[Manish Jethani, 2019/02/08 10:49:48]

I do not quite understand why this tag is necessary. Can you give an example of
a case where this would help?

[hub, 2019/02/08 14:14:03]

See above line 723 where we check if it's here and in that case move on.

[Manish Jethani, 2019/02/12 16:12:38]

Yes, but my question is more about why we need the tag at all. i.e. what if we
didn't tag it? Would it break something?

[hub, 2019/02/12 18:11:28]

so we don't try to set it again if it has already been, in case of recursion.
better be safe than sorry.

[Manish Jethani, 2019/02/15 14:45:24]

Well in any case it's redundant because configurable will be false for our own
descriptor. There's no need to set the magic property, nor to check for it.

[hub, 2019/02/19 15:36:48]

How do we know we are the one who set it?

[Manish Jethani, 2019/02/19 18:13:07]

We don't know, but if it's not configurable we can't set it anyway. (The code
will never reach the point where we check for the magic property.)

[hub, 2019/02/19 20:54:52]

Done.

+  Object.defineProperty(object, name, {get: () => v, set: setter});
   return true;
 }
 
 /**
  * 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.
@@ -744,17 +774,17 @@
 
   let rid = randomId();
 
   function abort()
   {
     throw new ReferenceError(rid);
   }
 
-  if (wrapPropertyAccess(window, property, {get: abort, set() {}}))
+  if (wrapPropertyAccess(window, property, {get: abort, set() {}}, rid))
     overrideOnError(rid);
 }
 
 exports["abort-on-property-read"] = makeInjector(abortOnPropertyRead,
                                                  wrapPropertyAccess,
                                                  overrideOnError,
                                                  randomId);
 
@@ -776,17 +806,17 @@
 
   let rid = randomId();
 
   function abort()
   {
     throw new ReferenceError(rid);
   }
 
-  if (wrapPropertyAccess(window, property, {set: abort}))
+  if (wrapPropertyAccess(window, property, {set: abort}, rid))
     overrideOnError(rid);
 }
 
 exports["abort-on-property-write"] = makeInjector(abortOnPropertyWrite,
                                                   wrapPropertyAccess,
                                                   overrideOnError,
                                                   randomId);
 
@@ -839,15 +869,15 @@
     },
     set(value)
     {
       abort();
       currentValue = value;
     }
   };
 
-  if (wrapPropertyAccess(object, name, descriptor))
+  if (wrapPropertyAccess(object, name, descriptor, rid))
     overrideOnError(rid);
 }
 
 exports["abort-current-inline-script"] =
   makeInjector(abortCurrentInlineScript, wrapPropertyAccess, toRegExp,
                overrideOnError, regexEscape, randomId);