Reviewers: MarkM,

Description:
* IE 11 has a native WeakMap, but it silently fails to accept frozen
  keys. Our DoubleWeakMap patch can handle that, but had to be taught
  about the exact problem.
* IE 11's typed arrays' prototypes have zero-length ArrayBuffers in
  .buffer data properties. This is harmless but was rejected; we now
  have a 'maybeAccessor' whitelist symbol. ('accessor' is gone because
  it has no uses; we can restore it in the future if needed).

There are still Caja test failures in IE 11; these changes merely
allow SES startup to succeed.

Please review this at https://codereview.appspot.com/44260044/

Affected files (+62, -35 lines):
  M     src/com/google/caja/ses/WeakMap.js
  M     src/com/google/caja/ses/startSES.js
  M     src/com/google/caja/ses/whitelist.js


Index: src/com/google/caja/ses/WeakMap.js
===================================================================
--- src/com/google/caja/ses/WeakMap.js  (revision 5649)
+++ src/com/google/caja/ses/WeakMap.js  (working copy)
@@ -125,6 +125,10 @@
     ses.weakMapPermitHostObjects = weakMapPermitHostObjects;
   }

+  // IE 11 has no Proxy but has a broken WeakMap such that we need to patch
+  // it using DoubleWeakMap; this flag tells DoubleWeakMap so.
+  var doubleWeakMapCheckSilentFailure = false;
+
// Check if there is already a good-enough WeakMap implementation, and if so
   // exit without replacing it.
   var HostWeakMap = WeakMap;
@@ -150,7 +154,16 @@

       // Fall through to installing our WeakMap.
     } else {
-      return;
+      // IE 11 bug: WeakMaps silently fail to store frozen objects.
+      var testMap = new HostWeakMap();
+      var testObject = Object.freeze({});
+      testMap.set(testObject, 1);
+      if (testMap.get(testObject) !== 1) {
+        doubleWeakMapCheckSilentFailure = true;
+        // Fall through to installing our WeakMap.
+      } else {
+       return;
+      }
     }
   }

@@ -517,6 +530,13 @@
       // that it may refuse to store some key types. Therefore, make a map
       // implementation which makes use of both as possible.

+ // In this mode we are always using double maps, so we are not proxy-safe. + // This combination does not occur in any known browser, but we had best
+      // be safe.
+ if (doubleWeakMapCheckSilentFailure && typeof Proxy !== 'undefined') {
+        Proxy = undefined;
+      }
+
       function DoubleWeakMap() {
if (!(this instanceof OurWeakMap)) { // approximate test for new ...()
           calledAsFunctionWarning();
@@ -536,6 +556,9 @@
// arbitrary WeakMaps to switch to using hidden properties, but only // those which need the ability, and unprivileged code is not allowed
         // to set the flag.
+        //
+        // (Except in doubleWeakMapCheckSilentFailure mode in which case we
+        // disable proxies.)
         var enableSwitching = false;

         function dget(key, opt_default) {
@@ -551,17 +574,28 @@
           return hmap.has(key) || (omap ? omap.has___(key) : false);
         }

-        function dset(key, value) {
-          if (enableSwitching) {
-            try {
-              hmap.set(key, value);
-            } catch (e) {
+        var dset;
+        if (doubleWeakMapCheckSilentFailure) {
+          dset = function(key, value) {
+            hmap.set(key, value);
+            if (!hmap.has(key)) {
               if (!omap) { omap = new OurWeakMap(); }
-              omap.set___(key, value);
+              omap.set(key, value);
             }
-          } else {
-            hmap.set(key, value);
-          }
+          };
+        } else {
+          dset = function(key, value) {
+            if (enableSwitching) {
+              try {
+                hmap.set(key, value);
+              } catch (e) {
+                if (!omap) { omap = new OurWeakMap(); }
+                omap.set___(key, value);
+              }
+            } else {
+              hmap.set(key, value);
+            }
+          };
         }

         function ddelete(key) {
Index: src/com/google/caja/ses/startSES.js
===================================================================
--- src/com/google/caja/ses/startSES.js (revision 5649)
+++ src/com/google/caja/ses/startSES.js (working copy)
@@ -1417,7 +1417,7 @@
    * process "*" inheritance using the whitelist, by walking actual
    * superclass chains.
    */
-  var whitelistSymbols = [true, '*', 'accessor'];
+  var whitelistSymbols = [true, '*', 'maybeAccessor'];
   var whiteTable = new WeakMap();
   function register(value, permit) {
     if (value !== Object(value)) { return; }
@@ -1434,8 +1434,8 @@
     whiteTable.set(value, permit);
     keys(permit).forEach(function(name) {
       // Use gopd to avoid invoking an accessor property.
-      // Mismatches between permit === 'accessor' and the property actually
-      // being an accessor property are caught later by clean().
+      // Accessor properties for which permit !== 'maybeAccessor'
+      // are caught later by clean().
       var desc = gopd(value, name);
       if (desc) {
         register(desc.value, permit[name]);
@@ -1618,20 +1618,13 @@
         if (hop.call(desc, 'value')) {
           // Is a data property
           var subValue = desc.value;
-          if (p === 'accessor') {
+          clean(subValue, path);
+        } else {
+          if (p !== 'maybeAccessor') {
             // We are not saying that it is safe for the prop to be
-            // unexpectedly not an accessor; rather, it will be deleted
+            // unexpectedly an accessor; rather, it will be deleted
             // and thus made safe.
             reportProperty(ses.severities.SAFE_SPEC_VIOLATION,
-                           'Not an accessor property', path);
-            cleanProperty(value, name, path);
-          } else {
-            clean(subValue, path);
-          }
-        } else {
-          // Is an accessor property (note symmetry with above case)
-          if (p !== 'accessor') {
-            reportProperty(ses.severities.SAFE_SPEC_VIOLATION,
                            'Not a data property', path);
             cleanProperty(value, name, path);
           } else {
Index: src/com/google/caja/ses/whitelist.js
===================================================================
--- src/com/google/caja/ses/whitelist.js        (revision 5649)
+++ src/com/google/caja/ses/whitelist.js        (working copy)
@@ -51,10 +51,10 @@
  *     If the property is an accessor property, it is not
  *     whitelisted (as invoking an accessor might not be meaningful,
  *     yet the accessor might return a value needing taming).
- * <li>"accessor", in which case this accessor property is simply
+ * <li>"maybeAccessor", in which case this accessor property is simply
  *     whitelisted and its getter and/or setter are tamed according to
- *     inheritance. If the property is not an accessor property, it is
- *     not whitelisted.
+ *     inheritance. If the property is not an accessor property, its
+ *     value is tamed according to inheritance.
  * <li>"*", in which case this property on this object is whitelisted,
  *     as is this property as inherited by all objects that inherit
  *     from this object. The values associated with all such properties
@@ -448,7 +448,7 @@
       name: t,  // ditto
       isView: t,
       prototype: {
-        byteLength: 'accessor',
+        byteLength: 'maybeAccessor',
         slice: t
       }
     },
@@ -457,10 +457,10 @@
       name: t,  // ditto
       BYTES_PER_ELEMENT: t,
       prototype: {
-        buffer: 'accessor',
-        byteOffset: 'accessor',
-        byteLength: 'accessor',
-        length: 'accessor',
+        buffer: 'maybeAccessor',
+        byteOffset: 'maybeAccessor',
+        byteLength: 'maybeAccessor',
+        length: 'maybeAccessor',
         BYTES_PER_ELEMENT: t,
         set: t,
         subarray: t
@@ -478,9 +478,9 @@
       length: t,  // does not inherit from Function.prototype on Chrome
       name: t,  // ditto
       prototype: {
-        buffer: 'accessor',
-        byteOffset: 'accessor',
-        byteLength: 'accessor',
+        buffer: 'maybeAccessor',
+        byteOffset: 'maybeAccessor',
+        byteLength: 'maybeAccessor',
         getInt8: t,
         getUint8: t,
         getInt16: t,


--

--- You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to