Skip to content

Commit

Permalink
feat(proxify): add length protection
Browse files Browse the repository at this point in the history
- When the `length` assertion is chained directly off of an uninvoked
  method, it references `function`'s built-in `length` property
  instead of Chai's `length` assertion. This commit enhances Chai's
  proxy protection to detect this problem and throw a helpful error
  message that advises the user to use `lengthOf` instead.

- Updates docs to reflect Chai's proxy protection detecting problems
  other than just a missing property.
  • Loading branch information
meeber committed Jan 2, 2017
1 parent 470ee42 commit 8fbe2c9
Show file tree
Hide file tree
Showing 8 changed files with 541 additions and 254 deletions.
54 changes: 40 additions & 14 deletions lib/chai/config.js
Expand Up @@ -55,16 +55,26 @@ module.exports = {
/**
* ### config.useProxy
*
* User configurable property, defines if chai will use a Proxy to throw
* an error when a non-existent property is read, which protects users
* from typos when using property-based assertions.
* User configurable property that defines if Chai will use ES6 proxies to
* protect users from making common mistakes while writing assertions. When
* enabled, Chai will throw an error when a problem is detected while
* accessing a property on a Chai Assertion object. The following situations
* are considered problems:
*
* - The object doesn't have the property that's being accessed, usually
* because the user misspelled an assertion, or forgot to invoke a
* non-chainable method previously in the assertion chain.
*
* - The `length` assertion is being chained directly off of an uninvoked
* method, which is a problem because it references `function`'s built-in
* `length` property instead of Chai's `length` assertion.
*
* Set it to false if you want to disable this feature.
*
* chai.config.useProxy = false; // disable use of Proxy
* chai.config.useProxy = false; // disable proxy protection
*
* This feature is automatically disabled regardless of this config value
* in environments that don't support proxies.
* in environments that don't support ES6 proxies.
*
* @param {Boolean}
* @api public
Expand All @@ -75,16 +85,32 @@ module.exports = {
/**
* ### config.proxyExcludedKeys
*
* User configurable property, defines which properties should be ignored
* instead of throwing an error if they do not exist on the assertion.
* This is only applied if the environment Chai is running in supports proxies and
* if the `useProxy` configuration setting is enabled.
* By default, `then` and `inspect` will not throw an error if they do not exist on the
* assertion object because the `.inspect` property is read by `util.inspect` (for example, when
* using `console.log` on the assertion object) and `.then` is necessary for promise type-checking.
* User configurable property that defines which properties should be excluded
* from Chai's proxy protection. This setting is only applied if the
* environment Chai is running in supports ES6 proxies, and if the `useProxy`
* configuration setting is enabled.
*
* By default, the following properties are excluded from proxy protection:
*
* - `then`: This property is read when working with an assertion object that
* has been wrapped in a promise.
*
* - `inspect`: This property is read by `util.inspect`, such as when using
* `console.log` on an assertion object.
*
* - `toJSON`: This property is read by `JSON.stringify`, which is used
* internally by `util.inspect`.
*
* Examples:
*
* // Exclude the "hiccup" property from proxy protection
* chai.config.proxyExcludedKeys.push('hiccup');
*
* // Don't exclude any properties from proxy protection, not even defaults
* chai.config.proxyExcludedKeys = [];
*
* // By default these keys will not throw an error if they do not exist on the assertion object
* chai.config.proxyExcludedKeys = ['then', 'inspect'];
* // Reset the list of excluded properties to the default
* chai.config.proxyExcludedKeys = ['then', 'inspect', 'toJSON'];
*
* @param {Array}
* @api public
Expand Down
2 changes: 1 addition & 1 deletion lib/chai/utils/addChainableMethod.js
Expand Up @@ -113,7 +113,7 @@ module.exports = function (ctx, name, method, chainingBehavior) {
}

transferFlags(this, assert);
return proxify(assert);
return proxify(assert, name, true);
}
, configurable: true
});
Expand Down
2 changes: 1 addition & 1 deletion lib/chai/utils/addMethod.js
Expand Up @@ -51,5 +51,5 @@ module.exports = function (ctx, name, method) {
return newAssertion;
};

ctx[name] = proxify(fn, name);
ctx[name] = proxify(fn, name, false);
};
2 changes: 1 addition & 1 deletion lib/chai/utils/overwriteMethod.js
Expand Up @@ -71,5 +71,5 @@ module.exports = function (ctx, name, method) {
return newAssertion;
}

ctx[name] = proxify(fn, name);
ctx[name] = proxify(fn, name, false);
};
86 changes: 63 additions & 23 deletions lib/chai/utils/proxify.js
Expand Up @@ -8,44 +8,84 @@ var getProperties = require('./getProperties');
*/

/**
* # proxify(object)
* # proxify(object, methodName, isChainable)
*
* Return a proxy of given object that throws an error when a problem is
* detected while accessing a property on the object. The following situations
* are considered problems:
*
* - The object doesn't have the property that's being accessed. There are two
* common reasons for this problem to occur. The first reason is that the
* user misspelled the name of an assertion. In this case, Chai makes an
* attempt to offer a reasonable suggestion from the list of existing
* assertions. The second reason is that the user is chaining an assertion
* off of an uninvoked non-chainable method. Non-chainable methods must be
* invoked prior to chaining. In this case, Chai suggests that the user
* consult the documentation for the given assertion.
*
* - The `length` assertion is being chained directly off of an uninvoked
* method, which is a problem because it references `function`'s built-in
* `length` property instead of Chai's `length` assertion. There are two
* common reasons for this problem to occur. The first reason is that the
* user chained the `length` assertion directly off of an uninvoked chainable
* method. In this case, Chai suggests that the user use `lengthOf` instead.
* The second reason is that the user chained the `length` assertion directly
* off of an uninvoked non-chainable method. Non-chainable methods must be
* invoked prior to chaining. In this case, Chai suggests that the user
* consult the documentation for the given assertion. Note that in ES6, the
* function's `length` property is configurable, so once support for legacy
* environments is dropped, Chai's `length` property can replace the built-in
* function's `length` property, and this problem will no longer need to be
* protected against. In the mean time, maintaining consistency across all
* environments is the priority.
*
* Return a proxy of given object that throws an error when a non-existent
* property is read. By default, the root cause is assumed to be a misspelled
* property, and thus an attempt is made to offer a reasonable suggestion from
* the list of existing properties. However, if a nonChainableMethodName is
* provided, then the root cause is instead a failure to invoke a non-chainable
* method prior to reading the non-existent property.
*
* If proxies are unsupported or disabled via the user's Chai config, then
* return object without modification.
* return object without modification. A list of individual properties to
* exclude from proxy protection is defined in `config.proxyExcludedKeys`.
*
* @param {Object} obj
* @param {String} nonChainableMethodName
* @param {String} methodName (optional)
* @param {String} isChainable (optional)
* @namespace Utils
* @name proxify
*/

module.exports = function proxify (obj, nonChainableMethodName) {
module.exports = function proxify (obj, methodName, isChainable) {
if (!config.useProxy || typeof Proxy === 'undefined' || typeof Reflect === 'undefined')
return obj;

return new Proxy(obj, {
get: function getProperty (target, property) {
// This check is here because we should not throw errors on Symbol properties
// such as `Symbol.toStringTag`.
// The values for which an error should be thrown can be configured using
// the `config.proxyExcludedKeys` setting.
if (typeof property === 'string' &&
config.proxyExcludedKeys.indexOf(property) === -1 &&
!Reflect.has(target, property)) {
// Special message for invalid property access of non-chainable methods.
if (nonChainableMethodName) {
throw Error('Invalid Chai property: ' + nonChainableMethodName + '.' +
property + '. See docs for proper usage of "' +
nonChainableMethodName + '".');
// Skip proxy protection for non-string properties because properties such
// as `Symbol.toStringTag` aren't indicative of a problem. Also skip proxy
// protection if this property name is configured to be skipped via
// `config.proxyExcludedKeys`.
if (typeof property !== 'string' ||
config.proxyExcludedKeys.indexOf(property) !== -1) {
return target[property];
}

var hasProperty = Reflect.has(target, property);

if (methodName) {
// Uninvoked non-chainable method either accessing a nonexistent
// property or `function`'s built-in `length` property
if (isChainable === false && (!hasProperty || property === 'length')) {
throw Error('Invalid Chai property: ' + methodName + '.' + property +
'. See docs for proper usage of "' + methodName + '".');
}

// Uninvoked chainable method accessing `function`'s built-in `length`
// property
if (isChainable === true && property === 'length') {
throw Error('Invalid Chai property: ' + methodName + '.length. Due' +
' to a compatibility issue, "length" cannot directly follow "' +
methodName + '". Use "' + methodName + '.lengthOf" instead.');
}
}

// All other nonexistent property access
if (!hasProperty) {
var orderedProperties = getProperties(target).filter(function(property) {
return !Object.prototype.hasOwnProperty(property) &&
['__flags', '__methods', '_obj', 'assert'].indexOf(property) === -1;
Expand Down

0 comments on commit 8fbe2c9

Please sign in to comment.