Repository: chaijs/chai
Pull request #823 (2016-10-03 21:05:31)

Do not suppress `TypeError`s thrown on integrity checks
Current implementation will report false-positive result if `TypeError` is thrown in some `Proxy` traps (`isExtensible`, `ownKeys`, `getOwnPropertyDescriptor`).


Patch:
diff --git a/MAINTAINERS b/MAINTAINERS
index fe9092f..ddbd6be 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3,3 +3,4 @@ lucasfcosta
 astorije
 meeber
 vieiralucas
+shvaikalesh
diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js
index c93e713..5471b37 100644
--- a/lib/chai/core/assertions.js
+++ b/lib/chai/core/assertions.js
@@ -2166,7 +2166,7 @@ module.exports = function (chai, _) {
    * ### .extensible
    *
    * Asserts that the target is extensible (can have new properties added to
-   * it).
+   * it). Returns `false` for primitives.
    *
    *     var nonExtensibleObject = Object.preventExtensions({});
    *     var sealedObject = Object.seal({});
@@ -2176,6 +2176,7 @@ module.exports = function (chai, _) {
    *     expect(nonExtensibleObject).to.not.be.extensible;
    *     expect(sealedObject).to.not.be.extensible;
    *     expect(frozenObject).to.not.be.extensible;
+   *     expect("").to.not.be.extensible;
    *
    * @name extensible
    * @namespace BDD
@@ -2185,19 +2186,12 @@ module.exports = function (chai, _) {
   Assertion.addProperty('extensible', function() {
     var obj = flag(this, 'object');
 
-    // In ES5, if the argument to this method is not an object (a primitive), then it will cause a TypeError.
+    // In ES5, if the argument to this method is a primitive, then it will cause a TypeError.
     // In ES6, a non-object argument will be treated as if it was a non-extensible ordinary object, simply return false.
     // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/isExtensible
-    // The following provides ES6 behavior when a TypeError is thrown under ES5.
+    // The following provides ES6 behavior for ES5 environments.
 
-    var isExtensible;
-
-    try {
-      isExtensible = Object.isExtensible(obj);
-    } catch (err) {
-      if (err instanceof TypeError) isExtensible = false;
-      else throw err;
-    }
+    var isExtensible = obj === Object(obj) && Object.isExtensible(obj);
 
     this.assert(
       isExtensible
@@ -2210,7 +2204,7 @@ module.exports = function (chai, _) {
    * ### .sealed
    *
    * Asserts that the target is sealed (cannot have new properties added to it
-   * and its existing properties cannot be removed).
+   * and its existing properties cannot be removed). Returns `true` for primitives.
    *
    *     var sealedObject = Object.seal({});
    *     var frozenObject = Object.freeze({});
@@ -2218,6 +2212,7 @@ module.exports = function (chai, _) {
    *     expect(sealedObject).to.be.sealed;
    *     expect(frozenObject).to.be.sealed;
    *     expect({}).to.not.be.sealed;
+   *     expect(false).to.be.sealed;
    *
    * @name sealed
    * @namespace BDD
@@ -2227,19 +2222,12 @@ module.exports = function (chai, _) {
   Assertion.addProperty('sealed', function() {
     var obj = flag(this, 'object');
 
-    // In ES5, if the argument to this method is not an object (a primitive), then it will cause a TypeError.
+    // In ES5, if the argument to this method is a primitive, then it will cause a TypeError.
     // In ES6, a non-object argument will be treated as if it was a sealed ordinary object, simply return true.
     // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/isSealed
-    // The following provides ES6 behavior when a TypeError is thrown under ES5.
-
-    var isSealed;
+    // The following provides ES6 behavior for ES5 environments.
 
-    try {
-      isSealed = Object.isSealed(obj);
-    } catch (err) {
-      if (err instanceof TypeError) isSealed = true;
-      else throw err;
-    }
+    var isSealed = obj === Object(obj) ? Object.isSealed(obj) : true;
 
     this.assert(
       isSealed
@@ -2252,12 +2240,13 @@ module.exports = function (chai, _) {
    * ### .frozen
    *
    * Asserts that the target is frozen (cannot have new properties added to it
-   * and its existing properties cannot be modified).
+   * and its existing properties cannot be modified). Returns `true` for primitives.
    *
    *     var frozenObject = Object.freeze({});
    *
    *     expect(frozenObject).to.be.frozen;
    *     expect({}).to.not.be.frozen;
+   *     expect(1).to.be.frozen;
    *
    * @name frozen
    * @namespace BDD
@@ -2267,19 +2256,12 @@ module.exports = function (chai, _) {
   Assertion.addProperty('frozen', function() {
     var obj = flag(this, 'object');
 
-    // In ES5, if the argument to this method is not an object (a primitive), then it will cause a TypeError.
+    // In ES5, if the argument to this method is a primitive, then it will cause a TypeError.
     // In ES6, a non-object argument will be treated as if it was a frozen ordinary object, simply return true.
     // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/isFrozen
-    // The following provides ES6 behavior when a TypeError is thrown under ES5.
-
-    var isFrozen;
+    // The following provides ES6 behavior for ES5 environments.
 
-    try {
-      isFrozen = Object.isFrozen(obj);
-    } catch (err) {
-      if (err instanceof TypeError) isFrozen = true;
-      else throw err;
-    }
+    var isFrozen = obj === Object(obj) ? Object.isFrozen(obj) : true;
 
     this.assert(
       isFrozen


Test patch:
diff --git a/test/assert.js b/test/assert.js
index d57beeb..ed4b453 100644
--- a/test/assert.js
+++ b/test/assert.js
@@ -1874,6 +1874,19 @@ describe('assert', function () {
       err(function() {
         assert[isExtensible](undefined);
       }, 'expected undefined to be extensible');
+
+      if (typeof Proxy === 'function') {
+        var proxy = new Proxy({}, {
+          isExtensible: function() {
+            throw new TypeError();
+          }
+        });
+
+        err(function() {
+          // isExtensible should not suppress errors, thrown in proxy traps
+          assert[isExtensible](proxy);
+        }, { name: 'TypeError' });
+      }
     });
   });
 
@@ -1898,6 +1911,19 @@ describe('assert', function () {
       if (typeof Symbol === 'function') {
         assert[isNotExtensible](Symbol());
       }
+
+      if (typeof Proxy === 'function') {
+        var proxy = new Proxy({}, {
+          isExtensible: function() {
+            throw new TypeError();
+          }
+        });
+
+        err(function() {
+          // isNotExtensible should not suppress errors, thrown in proxy traps
+          assert[isNotExtensible](proxy);
+        }, { name: 'TypeError' });
+      }
     });
   });
 
@@ -1922,6 +1948,22 @@ describe('assert', function () {
       if (typeof Symbol === 'function') {
         assert[isSealed](Symbol());
       }
+
+      if (typeof Proxy === 'function') {
+        var proxy = new Proxy({}, {
+          ownKeys: function() {
+            throw new TypeError();
+          }
+        });
+
+        // Object.isSealed will call ownKeys trap only if object is not extensible
+        Object.preventExtensions(proxy);
+
+        err(function() {
+          // isSealed should not suppress errors, thrown in proxy traps
+          assert[isSealed](proxy);
+        }, { name: 'TypeError' });
+      }
     });
   });
 
@@ -1956,6 +1998,22 @@ describe('assert', function () {
       err(function() {
         assert[isNotSealed](undefined);
       }, 'expected undefined to not be sealed');
+
+      if (typeof Proxy === 'function') {
+        var proxy = new Proxy({}, {
+          ownKeys: function() {
+            throw new TypeError();
+          }
+        });
+
+        // Object.isSealed will call ownKeys trap only if object is not extensible
+        Object.preventExtensions(proxy);
+
+        err(function() {
+          // isNotSealed should not suppress errors, thrown in proxy traps
+          assert[isNotSealed](proxy);
+        }, { name: 'TypeError' });
+      }
     });
   });
 
@@ -1980,6 +2038,22 @@ describe('assert', function () {
       if (typeof Symbol === 'function') {
         assert[isFrozen](Symbol());
       }
+
+      if (typeof Proxy === 'function') {
+        var proxy = new Proxy({}, {
+          ownKeys: function() {
+            throw new TypeError();
+          }
+        });
+
+        // Object.isFrozen will call ownKeys trap only if object is not extensible
+        Object.preventExtensions(proxy);
+
+        err(function() {
+          // isFrozen should not suppress errors, thrown in proxy traps
+          assert[isFrozen](proxy);
+        }, { name: 'TypeError' });
+      }
     });
   });
 
@@ -2014,6 +2088,22 @@ describe('assert', function () {
       err(function() {
         assert[isNotFrozen](undefined);
       }, 'expected undefined to not be frozen');
+
+      if (typeof Proxy === 'function') {
+        var proxy = new Proxy({}, {
+          ownKeys: function() {
+            throw new TypeError();
+          }
+        });
+
+        // Object.isFrozen will call ownKeys trap only if object is not extensible
+        Object.preventExtensions(proxy);
+
+        err(function() {
+          // isNotFrozen should not suppress errors, thrown in proxy traps
+          assert[isNotFrozen](proxy);
+        }, { name: 'TypeError' });
+      }
     });
   });
 
diff --git a/test/expect.js b/test/expect.js
index 7b1a75f..1ca00e7 100644
--- a/test/expect.js
+++ b/test/expect.js
@@ -2369,6 +2369,19 @@ describe('expect', function () {
     err(function() {
       expect(undefined).to.be.extensible;
     }, 'expected undefined to be extensible');
+
+    if (typeof Proxy === 'function') {
+      var proxy = new Proxy({}, {
+        isExtensible: function() {
+          throw new TypeError();
+        }
+      });
+
+      err(function() {
+        // .extensible should not suppress errors, thrown in proxy traps
+        expect(proxy).to.be.extensible;
+      }, { name: 'TypeError' });
+    }
   });
 
   it('sealed', function() {
@@ -2416,6 +2429,22 @@ describe('expect', function () {
     err(function() {
       expect(undefined).to.not.be.sealed;
     }, 'expected undefined to not be sealed');
+
+    if (typeof Proxy === 'function') {
+      var proxy = new Proxy({}, {
+        ownKeys: function() {
+          throw new TypeError();
+        }
+      });
+
+      // Object.isSealed will call ownKeys trap only if object is not extensible
+      Object.preventExtensions(proxy);
+
+      err(function() {
+        // .sealed should not suppress errors, thrown in proxy traps
+        expect(proxy).to.be.sealed;
+      }, { name: 'TypeError' });
+    }
   });
 
   it('frozen', function() {
@@ -2463,5 +2492,21 @@ describe('expect', function () {
     err(function() {
       expect(undefined).to.not.be.frozen;
     }, 'expected undefined to not be frozen');
+
+    if (typeof Proxy === 'function') {
+      var proxy = new Proxy({}, {
+        ownKeys: function() {
+          throw new TypeError();
+        }
+      });
+
+      // Object.isFrozen will call ownKeys trap only if object is not extensible
+      Object.preventExtensions(proxy);
+
+      err(function() {
+        // .frozen should not suppress errors, thrown in proxy traps
+        expect(proxy).to.be.frozen;
+      }, { name: 'TypeError' });
+    }
   });
 });
diff --git a/test/should.js b/test/should.js
index d715bbc..085ebe4 100644
--- a/test/should.js
+++ b/test/should.js
@@ -2204,6 +2204,19 @@ describe('should', function() {
      err(function() {
        false.should.be.extensible;
      }, 'expected false to be extensible');
+
+    if (typeof Proxy === 'function') {
+      var proxy = new Proxy({}, {
+        isExtensible: function() {
+          throw new TypeError();
+        }
+      });
+
+      err(function() {
+        // .extensible should not suppress errors, thrown in proxy traps
+        proxy.should.be.extensible;
+      }, { name: 'TypeError' });
+    }
   });
 
   it('sealed', function() {
@@ -2241,6 +2254,22 @@ describe('should', function() {
     err(function() {
       false.should.not.be.sealed;
     }, 'expected false to not be sealed');
+
+    if (typeof Proxy === 'function') {
+      var proxy = new Proxy({}, {
+        ownKeys: function() {
+          throw new TypeError();
+        }
+      });
+
+      // Object.isSealed will call ownKeys trap only if object is not extensible
+      Object.preventExtensions(proxy);
+
+      err(function() {
+        // .sealed should not suppress errors, thrown in proxy traps
+        proxy.should.be.sealed;
+      }, { name: 'TypeError' });
+    }
   });
 
   it('frozen', function() {
@@ -2278,5 +2307,21 @@ describe('should', function() {
     err(function() {
       false.should.not.be.frozen;
     }, 'expected false to not be frozen');
+
+    if (typeof Proxy === 'function') {
+      var proxy = new Proxy({}, {
+        ownKeys: function() {
+          throw new TypeError();
+        }
+      });
+
+      // Object.isFrozen will call ownKeys trap only if object is not extensible
+      Object.preventExtensions(proxy);
+
+      err(function() {
+        // .frozen should not suppress errors, thrown in proxy traps
+        proxy.should.be.frozen;
+      }, { name: 'TypeError' });
+    }
   });
 });