From aac0bc3665a9e9943e65b1cd263ae166cdd977cf Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Sat, 7 Mar 2020 14:06:39 +0100 Subject: [PATCH 1/7] Add verification tests for #2226 --- test/issues/issues-test.js | 46 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/issues/issues-test.js b/test/issues/issues-test.js index c5343224f..56c62984b 100644 --- a/test/issues/issues-test.js +++ b/test/issues/issues-test.js @@ -580,9 +580,11 @@ describe("issues", function() { function ClassWithoutProps() { return; } + function AnotherClassWithoutProps() { return; } + ClassWithoutProps.prototype.constructor = ClassWithoutProps; AnotherClassWithoutProps.prototype.constructor = AnotherClassWithoutProps; var arg1 = new ClassWithoutProps(); //arg1.constructor.name === ClassWithoutProps @@ -640,6 +642,7 @@ describe("issues", function() { function Foo() { return; } + // eslint-disable-next-line mocha/no-setup-in-describe Foo.prototype.testMethod = function() { return; @@ -679,6 +682,7 @@ describe("issues", function() { function Foo() { return; } + // eslint-disable-next-line mocha/no-setup-in-describe Foo.prototype.testMethod = function() { return 1; @@ -709,4 +713,46 @@ describe("issues", function() { refute.equals(fake.lastArg, 3); }); }); + + describe("#2226 - props on prototype are not restored correctly", function() { + function createObjectWithPropFromPrototype() { + var proto = {}; + var obj = {}; + + Object.setPrototypeOf(obj, proto); + Object.defineProperty(proto, "test", { writable: true, value: 1 }); + return obj; + } + + it("should restore fakes shadowing prototype props correctly", function() { + var obj = createObjectWithPropFromPrototype(); + + var originalPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + sinon.replace(obj, "test", 2); + var replacedPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + sinon.restore(); + var restoredPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + assert.isUndefined(originalPropertyDescriptor); + refute.isUndefined(replacedPropertyDescriptor); + assert.isUndefined(restoredPropertyDescriptor); + }); + + it("should restore stubs shadowing prototype props correctly", function() { + var obj = createObjectWithPropFromPrototype(); + var originalPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + sinon.stub(obj, "test").value(2); + var replacedPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + sinon.restore(); + var restoredPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + assert.isUndefined(originalPropertyDescriptor); + refute.isUndefined(replacedPropertyDescriptor); + assert.isUndefined(restoredPropertyDescriptor); + }); + }); }); From 4b55c62b494ed3405409e48f4d614a227659e0bf Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Sat, 7 Mar 2020 14:07:06 +0100 Subject: [PATCH 2/7] [fix] sandbox.restore handles protype props --- lib/sinon/sandbox.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index 9e1e580af..aa5a32fe3 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -189,9 +189,14 @@ function Sandbox() { function getFakeRestorer(object, property) { var descriptor = getPropertyDescriptor(object, property); + var shadowsPrototypeProp = typeof Object.getOwnPropertyDescriptor(object, property) === "undefined"; function restorer() { - Object.defineProperty(object, property, descriptor); + if (!shadowsPrototypeProp) { + Object.defineProperty(object, property, descriptor); + } else { + delete object[property]; + } } restorer.object = object; restorer.property = property; From 49abbad12eb644c2ed55601f60ca085208258213 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Mon, 9 Mar 2020 11:41:41 +0100 Subject: [PATCH 3/7] Rename util --- lib/sinon/stub.js | 6 +++--- lib/sinon/util/core/is-non-existent-own-property.js | 7 ------- lib/sinon/util/core/is-non-existent-property.js | 12 ++++++++++++ test/sandbox-test.js | 2 +- 4 files changed, 16 insertions(+), 11 deletions(-) delete mode 100644 lib/sinon/util/core/is-non-existent-own-property.js create mode 100644 lib/sinon/util/core/is-non-existent-property.js diff --git a/lib/sinon/stub.js b/lib/sinon/stub.js index a24d140ea..3ddca0a99 100644 --- a/lib/sinon/stub.js +++ b/lib/sinon/stub.js @@ -6,7 +6,7 @@ var behaviors = require("./default-behaviors"); var createProxy = require("./proxy"); var functionName = require("@sinonjs/commons").functionName; var hasOwnProperty = require("@sinonjs/commons").prototypes.object.hasOwnProperty; -var isNonExistentOwnProperty = require("./util/core/is-non-existent-own-property"); +var isNonExistentProperty = require("./util/core/is-non-existent-property"); var spy = require("./spy"); var extend = require("./util/core/extend"); var getPropertyDescriptor = require("./util/core/get-property-descriptor"); @@ -69,8 +69,8 @@ function stub(object, property) { throwOnFalsyObject.apply(null, arguments); - if (isNonExistentOwnProperty(object, property)) { - throw new TypeError("Cannot stub non-existent own property " + valueToString(property)); + if (isNonExistentProperty(object, property)) { + throw new TypeError("Cannot stub non-existent property " + valueToString(property)); } var actualDescriptor = getPropertyDescriptor(object, property); diff --git a/lib/sinon/util/core/is-non-existent-own-property.js b/lib/sinon/util/core/is-non-existent-own-property.js deleted file mode 100644 index 2df12609a..000000000 --- a/lib/sinon/util/core/is-non-existent-own-property.js +++ /dev/null @@ -1,7 +0,0 @@ -"use strict"; - -function isNonExistentOwnProperty(object, property) { - return object && typeof property !== "undefined" && !(property in object); -} - -module.exports = isNonExistentOwnProperty; diff --git a/lib/sinon/util/core/is-non-existent-property.js b/lib/sinon/util/core/is-non-existent-property.js new file mode 100644 index 000000000..f09dc3650 --- /dev/null +++ b/lib/sinon/util/core/is-non-existent-property.js @@ -0,0 +1,12 @@ +"use strict"; + +/** + * @param {*} object + * @param {String} property + * @returns whether a prop exists in the prototype chain + */ +function isNonExistentProperty(object, property) { + return object && typeof property !== "undefined" && !(property in object); +} + +module.exports = isNonExistentProperty; diff --git a/test/sandbox-test.js b/test/sandbox-test.js index 37c0c2eb6..c4e406ace 100644 --- a/test/sandbox-test.js +++ b/test/sandbox-test.js @@ -562,7 +562,7 @@ describe("Sandbox", function() { function() { sandbox.stub(object, Symbol()); }, - { message: "Cannot stub non-existent own property Symbol()" }, + { message: "Cannot stub non-existent property Symbol()" }, TypeError ); From 3031027894ce259d06c551f956c2c7bab414c6fa Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Mon, 9 Mar 2020 13:39:39 +0100 Subject: [PATCH 4/7] Add custom prop isOwn to descriptor Makes it possible to see if the descriptor originates from the object or its prototype chain --- lib/sinon/util/core/get-property-descriptor.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/sinon/util/core/get-property-descriptor.js b/lib/sinon/util/core/get-property-descriptor.js index 8dd3dc830..9625fd355 100644 --- a/lib/sinon/util/core/get-property-descriptor.js +++ b/lib/sinon/util/core/get-property-descriptor.js @@ -3,9 +3,15 @@ module.exports = function getPropertyDescriptor(object, property) { var proto = object; var descriptor; + var isOwn = object && Object.getOwnPropertyDescriptor(object, property); while (proto && !(descriptor = Object.getOwnPropertyDescriptor(proto, property))) { proto = Object.getPrototypeOf(proto); } + + if (descriptor) { + descriptor.isOwn = isOwn; + } + return descriptor; }; From 1fe433e1e36749a32e73f1cee6b7fd442d046977 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Mon, 9 Mar 2020 14:12:01 +0100 Subject: [PATCH 5/7] [fix] stubs restore prototype props correctly closes #2226 --- lib/sinon/default-behaviors.js | 2 +- lib/sinon/stub.js | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/sinon/default-behaviors.js b/lib/sinon/default-behaviors.js index 96a4b416d..d7387f7a9 100644 --- a/lib/sinon/default-behaviors.js +++ b/lib/sinon/default-behaviors.js @@ -258,7 +258,7 @@ var defaultBehaviors = { Object.defineProperty(rootStub.rootObj, rootStub.propName, { value: newVal, enumerable: true, - configurable: isPropertyConfigurable(rootStub.rootObj, rootStub.propName) + configurable: rootStub.shadowsPropOnPrototype || isPropertyConfigurable(rootStub.rootObj, rootStub.propName) }); return fake; diff --git a/lib/sinon/stub.js b/lib/sinon/stub.js index 3ddca0a99..9dde8858e 100644 --- a/lib/sinon/stub.js +++ b/lib/sinon/stub.js @@ -94,11 +94,14 @@ function stub(object, property) { var func = typeof actualDescriptor.value === "function" ? actualDescriptor.value : null; var s = createStub(func); + var propIsOwn = Boolean(actualDescriptor.isOwn); + extend.nonEnum(s, { rootObj: object, propName: property, + shadowsPropOnPrototype: !propIsOwn, restore: function restore() { - if (actualDescriptor !== undefined) { + if (actualDescriptor !== undefined && propIsOwn) { Object.defineProperty(object, property, actualDescriptor); return; } From 25311e4956f8898ae0ead6c164cdf71af426b507 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Mon, 9 Mar 2020 14:24:12 +0100 Subject: [PATCH 6/7] Simplify sandbox fix by reusing the fix for stubs --- lib/sinon/sandbox.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index aa5a32fe3..dd66f346e 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -189,10 +189,9 @@ function Sandbox() { function getFakeRestorer(object, property) { var descriptor = getPropertyDescriptor(object, property); - var shadowsPrototypeProp = typeof Object.getOwnPropertyDescriptor(object, property) === "undefined"; function restorer() { - if (!shadowsPrototypeProp) { + if (descriptor.isOwn) { Object.defineProperty(object, property, descriptor); } else { delete object[property]; From 92dc087eace469b0db31f36b3521f2f5c697dfcf Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Mon, 9 Mar 2020 22:22:11 +0100 Subject: [PATCH 7/7] Remove needless intermediary --- lib/sinon/stub.js | 6 ++---- lib/sinon/util/core/get-property-descriptor.js | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/sinon/stub.js b/lib/sinon/stub.js index 9dde8858e..4624b9847 100644 --- a/lib/sinon/stub.js +++ b/lib/sinon/stub.js @@ -94,14 +94,12 @@ function stub(object, property) { var func = typeof actualDescriptor.value === "function" ? actualDescriptor.value : null; var s = createStub(func); - var propIsOwn = Boolean(actualDescriptor.isOwn); - extend.nonEnum(s, { rootObj: object, propName: property, - shadowsPropOnPrototype: !propIsOwn, + shadowsPropOnPrototype: !actualDescriptor.isOwn, restore: function restore() { - if (actualDescriptor !== undefined && propIsOwn) { + if (actualDescriptor !== undefined && actualDescriptor.isOwn) { Object.defineProperty(object, property, actualDescriptor); return; } diff --git a/lib/sinon/util/core/get-property-descriptor.js b/lib/sinon/util/core/get-property-descriptor.js index 9625fd355..8efafc2e2 100644 --- a/lib/sinon/util/core/get-property-descriptor.js +++ b/lib/sinon/util/core/get-property-descriptor.js @@ -3,7 +3,7 @@ module.exports = function getPropertyDescriptor(object, property) { var proto = object; var descriptor; - var isOwn = object && Object.getOwnPropertyDescriptor(object, property); + var isOwn = Boolean(object && Object.getOwnPropertyDescriptor(object, property)); while (proto && !(descriptor = Object.getOwnPropertyDescriptor(proto, property))) { proto = Object.getPrototypeOf(proto);