From 5fd94515cb2ae5540da63530ee14af8111b0ce00 Mon Sep 17 00:00:00 2001 From: Vladimir Kurchatkin Date: Tue, 27 Jan 2015 15:36:08 +0300 Subject: [PATCH] assert: check object prototype in `deepEqual` Objects with different prototypes should not be considered deep equal, e.g. `assert.deepEqual({}, [])` should throw. Previously `deepEqual` was implemented as per CommonJS Unit Testing/1.0 spec. It states that objects should have 'an identical "prototype" property' to be considered deep equal, which is incorrect. Instead object prototypes should be compared, i.e. `Object.getPrototypeOf` or `__proto__`. Fixes: https://github.com/iojs/io.js/issues/620 --- lib/assert.js | 5 +++-- test/parallel/test-assert.js | 16 +++++++++++----- test/parallel/test-url.js | 6 +++--- .../parallel/test-vm-create-context-accessors.js | 3 ++- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index aa3f0a3c831591..c17d72fc285b2d 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -199,11 +199,12 @@ function isArguments(object) { function objEquiv(a, b) { if (util.isNullOrUndefined(a) || util.isNullOrUndefined(b)) return false; - // an identical 'prototype' property. - if (a.prototype !== b.prototype) return false; // if one is a primitive, the other must be same if (util.isPrimitive(a) || util.isPrimitive(b)) return a === b; + // an identical prototype. + if (Object.getPrototypeOf(a) !== Object.getPrototypeOf(b)) + return false; var aIsArgs = isArguments(a), bIsArgs = isArguments(b); if ((aIsArgs && !bIsArgs) || (!aIsArgs && bIsArgs)) diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 5d5e70ea7ecbc1..4eb46224c01575 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -91,7 +91,7 @@ assert.doesNotThrow(makeBlock(a.deepEqual, {a: 4, b: '2'}, {a: 4, b: '2'})); assert.doesNotThrow(makeBlock(a.deepEqual, [4], ['4'])); assert.throws(makeBlock(a.deepEqual, {a: 4}, {a: 4, b: true}), a.AssertionError); -assert.doesNotThrow(makeBlock(a.deepEqual, ['a'], {0: 'a'})); +assert.throws(makeBlock(a.deepEqual, ['a'], {0: 'a'})); //(although not necessarily the same order), assert.doesNotThrow(makeBlock(a.deepEqual, {a: 4, b: '1'}, {b: '1', a: 4})); var a1 = [1, 2, 3]; @@ -144,10 +144,16 @@ if (typeof Symbol === 'symbol') { } // primitive wrappers and object -assert.doesNotThrow(makeBlock(a.deepEqual, new String('a'), ['a']), a.AssertionError); -assert.doesNotThrow(makeBlock(a.deepEqual, new String('a'), {0: 'a'}), a.AssertionError); -assert.doesNotThrow(makeBlock(a.deepEqual, new Number(1), {}), a.AssertionError); -assert.doesNotThrow(makeBlock(a.deepEqual, new Boolean(true), {}), a.AssertionError); +assert.throws(makeBlock(a.deepEqual, new String('a'), ['a']), + a.AssertionError); +assert.throws(makeBlock(a.deepEqual, new String('a'),{0: 'a'}), + a.AssertionError); +assert.throws(makeBlock(a.deepEqual, new Number(1), {}), a.AssertionError); +assert.throws(makeBlock(a.deepEqual, new Boolean(true), {}), a.AssertionError); + +// prototypes +assert.throws(makeBlock(assert.deepEqual, Object.create({}), {}), + a.AssertionError); // Testing the throwing function thrower(errorConstructor) { diff --git a/test/parallel/test-url.js b/test/parallel/test-url.js index d6732a91280ed5..ca37d7a40592f0 100644 --- a/test/parallel/test-url.js +++ b/test/parallel/test-url.js @@ -867,8 +867,8 @@ for (var u in parseTests) { } }); - assert.deepEqual(actual, expected); - assert.deepEqual(spaced, expected); + assert.deepEqual(JSON.parse(JSON.stringify(actual)), expected); + assert.deepEqual(JSON.parse(JSON.stringify(spaced)), expected); var expected = parseTests[u].href, actual = url.format(parseTests[u]); @@ -937,7 +937,7 @@ for (var u in parseTestsWithQueryString) { } } - assert.deepEqual(actual, expected); + assert.deepEqual(JSON.parse(JSON.stringify(actual)), expected); } // some extra formatting tests, just to verify diff --git a/test/parallel/test-vm-create-context-accessors.js b/test/parallel/test-vm-create-context-accessors.js index 23fc935389def8..591c6597be347c 100644 --- a/test/parallel/test-vm-create-context-accessors.js +++ b/test/parallel/test-vm-create-context-accessors.js @@ -23,4 +23,5 @@ Object.defineProperty(ctx, 'setter', { ctx = vm.createContext(ctx); var result = vm.runInContext('setter = "test";[getter,setter]', ctx); -assert.deepEqual(result, ['ok', 'ok=test']); +assert.equal(result[0], 'ok'); +assert.equal(result[1], 'ok=test');