Reviewers: kpreid2,

Description:
The X_IGNORES_Y matrix of tests was not testing for seal violations
correctly. The splice test was not creating a potential seal violation.

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

Affected files:
  M     src/com/google/caja/ses/repairES5.js


Index: src/com/google/caja/ses/repairES5.js
===================================================================
--- src/com/google/caja/ses/repairES5.js        (revision 5559)
+++ src/com/google/caja/ses/repairES5.js        (working copy)
@@ -3322,23 +3322,33 @@
   // more practical to define them programmatically.

   function arrayMutatorKludge(destination, prop, testArgs) {
-    function testArrayMutator(op) {
-      return function() {
-        var x = [2, 1];  // disordered to detect sort()
-        Object[op](x);
-        try {
-          x[prop].apply(x, testArgs);
-        } catch (e) {
-          if (x.length === 2 && x[0] === 2 && x[1] === 1) { return false; }
-        }
+    function test_METHOD_IGNORES_SEALED() {
+      var x = [2, 1];  // disordered to detect sort()
+      Object.seal(x);
+      try {
+        x[prop].apply(x, testArgs);
+      } catch (e) {
         // It is actually still a non-conformance if the array was not
+        // badly mutated but the method did not throw, but not an
+        // UNSAFE_SPEC_VIOLATION.
+      }
+      return !(x.length === 2 && ('0' in x) && ('1' in x) && !('2' in x));
+    }
+
+    function test_METHOD_IGNORES_FROZEN() {
+      var x = [2, 1];  // disordered to detect sort()
+      Object.freeze(x);
+      try {
+        x[prop].apply(x, testArgs);
+      } catch (e) {
+        // It is actually still a non-conformance if the array was not
         // mutated but the method did not throw, but not an
         // UNSAFE_SPEC_VIOLATION.
-        return (x.length !== 2 || x[0] !== 2 || x[1] !== 1);
-      };
+      }
+      return !(x.length === 2 && x[0] === 2 && x[1] === 1 && !('2' in x));
     }

-    function repairArrayMutator() {
+    function repair_METHOD_IGNORES_SEALED() {
       var originalMethod = Array.prototype[prop];
       var isSealed = Object.isSealed;
       Object.defineProperty(Array.prototype, prop, {
@@ -3356,8 +3366,8 @@
     destination.push({
       id: (prop + '_IGNORES_SEALED').toUpperCase(),
       description: 'Array.prototype.' + prop + ' ignores sealing',
-      test: testArrayMutator('seal'),
-      repair: repairArrayMutator,
+      test: test_METHOD_IGNORES_SEALED,
+      repair: repair_METHOD_IGNORES_SEALED,
       preSeverity: severities.UNSAFE_SPEC_VIOLATION,
       canRepair: false,  // does not protect individual properties, only
           // fully sealed objects
@@ -3370,8 +3380,8 @@
     destination.push({
       id: (prop + '_IGNORES_FROZEN').toUpperCase(),
       description: 'Array.prototype.' + prop + ' ignores freezing',
-      test: testArrayMutator('freeze'),
-      repair: repairArrayMutator,
+      test: test_METHOD_IGNORES_FROZEN,
+      repair: repair_METHOD_IGNORES_SEALED,
       preSeverity: severities.UNSAFE_SPEC_VIOLATION,
       canRepair: true,
       urls: [
@@ -4274,7 +4284,7 @@
   // SHIFT_IGNORES_SEALED
   // SHIFT_IGNORES_FROZEN
   arrayMutatorKludge(supportedKludges, 'unshift', ['foo']);
-  arrayMutatorKludge(supportedKludges, 'splice', [0, 1, 'foo']);
+  arrayMutatorKludge(supportedKludges, 'splice', [0, 0, 'foo']);
   arrayMutatorKludge(supportedKludges, 'shift', []);
   // Array.prototype.{push,pop,sort} are also subject to the problem
   // arrayMutatorKludge handles, but are handled separately and more


--

--- 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