Skip to content

Commit

Permalink
Don't assume obj.__proto__ === Object in assertion helpers
Browse files Browse the repository at this point in the history
Previously, we assumed that whatever object the user passed to the
object assertion helpers had its prototype set to Object and would
thus have things like a hasOwnProperty property.

However, it turns out that this is not a valid assumption. This isn't
limited to just corner cases; Node 6.x changed the behavior of its
`querystring` module (which is also used by its `url` module) to
explicitly set the prototype of querystring objects to `null` in order
to prevent key collisions if someone sent a URL query string parameter
with the same key as a property on the prototype. See nodejs/node#6055
for where this change was made.

Therefore, we use Object.prototype.hasOwnProperty, which is guaranteed
to always exist, and call it on the object the user passed in. We do
the same for propertyIsEnumerable as applicable.
  • Loading branch information
strugee committed Apr 7, 2017
1 parent 9bbb0fe commit 53c0c36
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions lib/assert/macros.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ assert.isEnumerable = function (obj, key, message) {
assertMissingArguments(arguments, assert.isEnumerable);
assertAnyTypeOf(obj, ["function", "object"], null, assert.isEnumerable);
assertTypeOf(key, "string", null, assert.isEnumerable);
if (!obj.hasOwnProperty(key)) {
if (!Object.prototype.hasOwnProperty.call(obj, key)) {
assert.fail(null, null, "key was never defined.", "isEnumerable", assert.isEnumerable);
}
if (!obj.propertyIsEnumerable(key)) {
if (!Object.prototype.propertyIsEnumerable.call(obj, key)) {
assert.fail(null, null, message || "expected key '" + key + "' to be enumerable", "isEnumerable", assert.isEnumerable);
}
};
Expand All @@ -106,10 +106,10 @@ assert.isNotEnumerable = function (obj, key, message) {
assertMissingArguments(arguments, assert.isNotEnumerable);
assertAnyTypeOf(obj, ["function", "object"], null, assert.isNotEnumerable);
assertTypeOf(key, "string", null, assert.isNotEnumerable);
if (!obj.hasOwnProperty(key)) {
if (!Object.prototype.hasOwnProperty.call(obj, key)) {
assert.fail(null, null, "key was never defined.", "isNotEnumerable", assert.isNotEnumerable);
}
if (obj.propertyIsEnumerable(key)) {
if (Object.prototype.propertyIsEnumerable.call(obj, key)) {
return assert.fail(null, null, message || "expected key '" + key + "' to not be enumerable", "isNotEnumerable", assert.isNotEnumerable);
}
};
Expand Down

0 comments on commit 53c0c36

Please sign in to comment.