From 6aaafd054c48677180945e19e459246ba3027156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Wed, 24 Aug 2016 15:54:42 +0200 Subject: [PATCH] Rework coercion of input arguments Simplify the design of input argument handling. - It's up to the Context and/or the Adapter to convert or coerce input arguments as needed. - The only responsibility of SharedMethod is to validate input arguments (check that they match the contract in "accepts"). - Define two types of conversion/coercion: a) When the input argument value comes from a source that preserves type information (typically JSON), then the argument should be converted with minimal coercion. b) When the input argument value comes from a "sloppy" source (typically string-encoded source like a query string), then coercion rules should be applied. Replace "Dynamic" API, which was relying on global singleton registry, with a new concept of a "TypeRegistry" and a type "Converter". The new registry is local to each RemoteObjects instance. Each type converter provides three methods: - `fromTypedValue` to handle values coming from JSON - `fromSloppyValue` to handle values coming e.g. from query string - `validate` which is called by SharedMethod to validate argument values To prevent unnecessary try/catch blocks with the associated performance costs, validation/conversion errors are not thrown but returned via function return value instead. The feature where one could encode all input arguments in a single (query string) parameter "args" is removed. This patch fixes many subtle bugs and changes the way how edge case values are treated. The general approach is to make both conversion and coercion more strict. When we are not sure how to treat an input value, we rather return HTTP 400 Bad Request than coerce the value incorrectly. Most notable breaking changes: - `null` value is accepted only for "object", "array" and "any" - Empty string is coerced to undefined to support ES6 default arguments - JSON requests providing scalar values for array-typed argument are rejected - Empty value is not converted to an empty array - Array values containing items of wrong type are rejected. For example, an array containing a string value is rejected when the input argument expects an array of numbers. - Array items from JSON requests are not coerced. For example, `[true]` is no longer coerced to `[1]` for number arrays, and the request is subsequently rejected. - Deep members of object arguments are no longer coerced. For example, a query like `?arg[count]=42` produces `{ count: '42' }` now. - "any" coercion preserves too large numbers as a string, to prevent losing precision. - Boolean types accepts only four string values: 'true', 'false', '0' and '1' Values are case-insensitive, i.e. 'TRUE' and 'FaLsE' are work too. - Date type detects "Invalid Date" and rejects such requests. Last but not least, conversion/validation error messages no longer include input argument values, in order to prevent reflection-based attacks. --- 3.0-RELEASE-NOTES.md | 63 +++++++ lib/context-base.js | 16 +- lib/dynamic.js | 127 ------------- lib/http-context.js | 178 +++++------------- lib/http-invocation.js | 40 ++-- lib/number-checks.js | 20 ++ lib/remote-objects.js | 97 +++++++++- lib/rest-adapter.js | 10 +- lib/shared-method.js | 148 +++------------ lib/type-registry.js | 118 ++++++++++++ lib/types/any.js | 60 ++++++ lib/types/array.js | 157 +++++++++++++++ lib/types/boolean.js | 46 +++++ lib/types/date.js | 51 +++++ lib/types/integer.js | 41 ++++ lib/types/number.js | 36 ++++ lib/types/object.js | 55 ++++++ lib/types/string.js | 40 ++++ package.json | 1 + test/http-context.test.js | 126 ------------- test/http-invocation.test.js | 50 +++-- test/rest-coercion/_urlencoded.context.js | 2 +- test/rest-coercion/jsonbody-any.suite.js | 2 +- test/rest-coercion/jsonbody-array.suite.js | 141 +++++++------- test/rest-coercion/jsonbody-object.suite.js | 2 +- test/rest-coercion/jsonform-any.suite.js | 21 +-- test/rest-coercion/jsonform-array.suite.js | 163 ++++++++-------- test/rest-coercion/jsonform-boolean.suite.js | 36 ++-- test/rest-coercion/jsonform-date.suite.js | 35 ++-- test/rest-coercion/jsonform-integer.suite.js | 26 ++- test/rest-coercion/jsonform-number.suite.js | 32 ++-- test/rest-coercion/jsonform-object.suite.js | 2 +- test/rest-coercion/jsonform-string.suite.js | 24 +-- test/rest-coercion/urlencoded-any.suite.js | 34 ++-- test/rest-coercion/urlencoded-array.suite.js | 155 +++++++-------- .../rest-coercion/urlencoded-boolean.suite.js | 41 ++-- test/rest-coercion/urlencoded-date.suite.js | 34 ++-- .../rest-coercion/urlencoded-integer.suite.js | 8 +- test/rest-coercion/urlencoded-number.suite.js | 8 +- test/rest-coercion/urlencoded-object.suite.js | 8 +- test/rest-coercion/urlencoded-string.suite.js | 8 +- test/rest.browser.test.js | 37 +++- test/rest.test.js | 25 +-- test/shared-method.test.js | 63 ++++--- test/type.test.js | 88 --------- 45 files changed, 1386 insertions(+), 1089 deletions(-) delete mode 100644 lib/dynamic.js create mode 100644 lib/number-checks.js create mode 100644 lib/type-registry.js create mode 100644 lib/types/any.js create mode 100644 lib/types/array.js create mode 100644 lib/types/boolean.js create mode 100644 lib/types/date.js create mode 100644 lib/types/integer.js create mode 100644 lib/types/number.js create mode 100644 lib/types/object.js create mode 100644 lib/types/string.js delete mode 100644 test/http-context.test.js delete mode 100644 test/type.test.js diff --git a/3.0-RELEASE-NOTES.md b/3.0-RELEASE-NOTES.md index c3c0c1e3..ff56a5e7 100644 --- a/3.0-RELEASE-NOTES.md +++ b/3.0-RELEASE-NOTES.md @@ -113,3 +113,66 @@ An example of an error response when an array of errors was raised: ``` Please see [Related code change](https://github.com/strongloop/strong-remoting/pull/302) here, and new [`strong-error-handler` here](https://github.com/strongloop/strong-error-handler/). + +## Type converters replace `Dynamic` API + +The `Dynamic` component was removed in favour of type converters and a +registry. + +The following APIs were removed: + +```js +RemoteObjects.convert(name, fn) +remoteObjectsInstance.convert(name, fn) +RemoteObjects.defineType(name, fn) +remoteObjectsInstance.defineType(name, fn) +``` + +Two new APIs are added as a replacement: + +```js +remoteObjectsInstance.defineType(name, converter) +remoteObjectsInstance.defineObjectType(name, factoryFn) +``` + +See the API docs and +[pull request #343](https://github.com/strongloop/strong-remoting/pull/343) +for more details. + +## Conversion and coercion of input arguments + +We have significantly reworked conversion and coercion of input arguments +when using the default REST adapter. The general approach is to make both +conversion and coercion more strict. When we are not sure how to treat +an input value, we rather return HTTP error `400 Bad Request` than coerce +the value incorrectly. + +Most notable breaking changes: + + - `null` value is accepted only for "object", "array" and "any" + - Empty string is coerced to undefined to support ES6 default arguments + - JSON requests providing scalar values for array-typed argument are + rejected + - Empty value is not converted to an empty array + - Array values containing items of wrong type are rejected. For + example, an array containing a string value is rejected when + the input argument expects an array of numbers. + - Array items from JSON requests are not coerced. For example, + `[true]` is no longer coerced to `[1]` for number arrays, + and the request is subsequently rejected. + - Deep members of object arguments are no longer coerced. For example, + a query like `?arg[count]=42` produces `{ count: '42' }` now. + - "any" coercion preserves too large numbers as a string, to prevent + losing precision. + - Boolean types accepts only four string values: + 'true', 'false', '0' and '1' + Values are case-insensitive, i.e. 'TRUE' and 'FaLsE' work too. + - Date type detects "Invalid Date" and rejects such requests. + +Hopefully this change should leave most LoopBack applications (and clients) +unaffected. If your start seeing unusual amount of 400 error responses after +upgrading to LoopBack 3.x, then please check the client code and ensure it +correctly encodes request parameters. + +See [pull request #343](https://github.com/strongloop/strong-remoting/pull/343) +for more details. diff --git a/lib/context-base.js b/lib/context-base.js index b41d2e7a..71fcd0b8 100644 --- a/lib/context-base.js +++ b/lib/context-base.js @@ -5,7 +5,9 @@ 'use strict'; +var assert = require('assert'); var EventEmitter = require('events').EventEmitter; +var TypeRegistry = require('./type-registry'); var inherits = require('util').inherits; module.exports = ContextBase; @@ -13,10 +15,18 @@ module.exports = ContextBase; /** * A base class for all Context instances */ -function ContextBase(method) { +function ContextBase(method, typeRegistry) { + // NOTE(bajtos) we are not asserting via "instanceof" to allow + // multiple copies of strong-remoting to cooperate together + assert(method && typeof method === 'object', + 'method must be a SharedClass instance'); + assert(typeRegistry && typeof typeRegistry === 'object', + 'typeRegistry must be a TypeRegistry instance'); + EventEmitter.call(this); this.method = method; + this.typeRegistry = typeRegistry; } inherits(ContextBase, EventEmitter); @@ -29,3 +39,7 @@ ContextBase.prototype.getScope = function() { method.ctor || method.sharedMethod && method.sharedMethod.ctor; }; + +ContextBase.prototype.setReturnArgByName = function(name, value) { + // no-op +}; diff --git a/lib/dynamic.js b/lib/dynamic.js deleted file mode 100644 index 271a4ffc..00000000 --- a/lib/dynamic.js +++ /dev/null @@ -1,127 +0,0 @@ -// Copyright IBM Corp. 2014,2015. All Rights Reserved. -// Node module: strong-remoting -// This file is licensed under the Artistic License 2.0. -// License text available at https://opensource.org/licenses/Artistic-2.0 - -/** - * Expose `Dynamic`. - */ -module.exports = Dynamic; - -/** - * Module dependencies. - */ - -var debug = require('debug')('strong-remoting:dynamic'); -var assert = require('assert'); - -/** - * Create a dynamic value from the given value. - * - * @param {*} val The value object - * @param {Context} ctx The Remote Context - */ - -function Dynamic(val, ctx) { - this.val = val; - this.ctx = ctx; -} - -/*! - * Object containing converter functions. - */ - -Dynamic.converters = []; - -/** - * Define a named type conversion. The conversion is used when a - * `SharedMethod` argument defines a type with the given `name`. - * - * ```js - * Dynamic.define('MyType', function(val, ctx) { - * // use the val and ctx objects to return the concrete value - * return new MyType(val); - * }); - * ``` - * - * @param {String} name The type name - * @param {Function} converter - */ - -Dynamic.define = function(name, converter) { - converter.typeName = name; - this.converters.unshift(converter); -}; - -/** - * Is the given type supported. - * - * @param {String} type - * @returns {Boolean} - */ - -Dynamic.canConvert = function(type) { - return !!this.getConverter(type); -}; - -/** - * Get converter by type name. - * - * @param {String} type - * @returns {Function} - */ - -Dynamic.getConverter = function(type) { - var converters = this.converters; - var converter; - for (var i = 0; i < converters.length; i++) { - converter = converters[i]; - if (converter.typeName === type) { - return converter; - } - } -}; - -/** - * Convert the dynamic value to the given type. - * - * @param {String} type - * @returns {*} The concrete value - */ - -Dynamic.prototype.to = function(type) { - var converter = this.constructor.getConverter(type); - assert(converter, 'No Type converter defined for ' + type); - return converter(this.val, this.ctx); -}; - -/** - * Built in type converters... - */ - -Dynamic.define('boolean', function convertBoolean(val) { - switch (typeof val) { - case 'string': - switch (val) { - case 'false': - case 'undefined': - case 'null': - case '0': - case '': - return false; - default: - return true; - } - break; - case 'number': - return val !== 0; - default: - return Boolean(val); - } -}); - -Dynamic.define('number', function convertNumber(val) { - if (val === 0) return val; - if (!val) return val; - return Number(val); -}); diff --git a/lib/http-context.js b/lib/http-context.js index 3004be8e..d61a25f5 100644 --- a/lib/http-context.js +++ b/lib/http-context.js @@ -17,7 +17,6 @@ var util = require('util'); var inherits = util.inherits; var assert = require('assert'); var ContextBase = require('./context-base'); -var Dynamic = require('./dynamic'); var js2xmlparser = require('js2xmlparser'); var SharedMethod = require('./shared-method'); @@ -48,8 +47,8 @@ var SSEClient = require('sse').Client; * @class */ -function HttpContext(req, res, method, options) { - ContextBase.call(this, method); +function HttpContext(req, res, method, options, typeRegistry) { + ContextBase.call(this, method, typeRegistry); this.req = req; this.res = res; @@ -119,12 +118,14 @@ HttpContext.prototype.buildArgs = function(method) { var httpFormat = o.http; var name = o.name || o.arg; var val; + var isJsonRequest = ctx.req.get('content-type') === 'application/json'; - // Support array types, such as ['string'] - var isArrayType = Array.isArray(o.type); - var otype = isArrayType ? o.type[0] : o.type; - otype = (typeof otype === 'string') && otype; - var isAny = !otype || otype.toLowerCase() === 'any'; + var typeConverter = ctx.typeRegistry.getConverter(o.type); + + // Turn off sloppy coercion for values coming from JSON payloads. + // This is because JSON, unlike other methods, properly retains types + // like Numbers, Booleans, and null/undefined. + var doSloppyCoerce = !isJsonRequest; // This is an http method keyword, which requires special parsing. if (httpFormat) { @@ -132,6 +133,8 @@ HttpContext.prototype.buildArgs = function(method) { case 'function': // the options have defined a formatter val = httpFormat(ctx); + // it's up to the custom provider to perform any coercion as needed + doSloppyCoerce = false; break; case 'object': switch (httpFormat.source) { @@ -146,13 +149,16 @@ HttpContext.prototype.buildArgs = function(method) { case 'query': // From the query string val = ctx.req.query[name]; + doSloppyCoerce = true; break; case 'path': // From the url path val = ctx.req.params[name]; + doSloppyCoerce = true; break; case 'header': val = ctx.req.get(name); + doSloppyCoerce = true; break; case 'req': // Direct access to http req @@ -171,55 +177,36 @@ HttpContext.prototype.buildArgs = function(method) { } } else { val = ctx.getArgByName(name, o); - // Safe to coerce the contents of this - if (typeof val === 'object' && (!isArrayType || isAny)) { - val = coerceAll(val); - } - } - - // If we expect an array type and we received a string, parse it with JSON. - // If that fails, parse it with the arrayItemDelimiters option. - if (val && typeof val === 'string' && isArrayType) { - var parsed = false; - if (val[0] === '[') { - try { - val = JSON.parse(val); - parsed = true; - } catch (e) {} - } - if (!parsed && ctx.options.arrayItemDelimiters) { - // Construct delimiter regex if input was an array. Overwrite option - // so this only needs to happen once. - var delims = this.options.arrayItemDelimiters; - if (Array.isArray(delims)) { - delims = new RegExp(delims.map(escapeRegex).join('|'), 'g'); - this.options.arrayItemDelimiters = delims; - } - - val = val.split(delims); - } - } - - // Coerce dynamic args when input is a string. - if (isAny && typeof val === 'string') { - val = coerceAll(val); + doSloppyCoerce = !(isJsonRequest && ctx.req.body && + val === ctx.req.body[name]); } - // If the input is not an array, but we were expecting one, create - // an array. Create an empty array if input is empty. - if (!Array.isArray(val) && isArrayType) { - if (val !== undefined && val !== '') val = [val]; - else val = []; + // Most of the time, the data comes through 'sloppy' methods like HTTP headers or a qs + // which don't preserve types. + // + // Use some sloppy typing semantics to try to guess what the user meant to send. + var result = doSloppyCoerce ? + typeConverter.fromSloppyValue(ctx, val) : + typeConverter.fromTypedValue(ctx, val); + + debug('arg %j: %s converted %j to %j', + name, doSloppyCoerce ? 'sloppy' : 'typed', val, result); + + var isValidResult = typeof result === 'object' && + ('error' in result || 'value' in result); + if (!isValidResult) { + throw new (assert.AssertionError)({ + message: 'Type conversion result should have "error" or "value" property. ' + + 'Got ' + JSON.stringify(result) + ' instead.', + }); } - // For boolean and number types, convert certain strings to that type. - // The user can also define new dynamic types. - if (Dynamic.canConvert(otype)) { - val = dynamic(val, otype, ctx); + if (result.error) { + throw result.error; } - // set the argument value - args[o.arg] = val; + // Set the argument value. + args[o.arg] = result.value; } return args; @@ -234,97 +221,16 @@ HttpContext.prototype.buildArgs = function(method) { HttpContext.prototype.getArgByName = function(name, options) { var req = this.req; - var args = req.params && req.params.args !== undefined ? req.params.args : - req.body && req.body.args !== undefined ? req.body.args : - req.query && req.query.args !== undefined ? req.query.args : - undefined; - if (args) { - args = JSON.parse(args); - } - - if (typeof args !== 'object' || !args) { - args = {}; - } - - var arg = (args && args[name] !== undefined) ? args[name] : - this.req.params[name] !== undefined ? this.req.params[name] : - (this.req.body && this.req.body[name]) !== undefined ? this.req.body[name] : - this.req.query[name] !== undefined ? this.req.query[name] : - this.req.get(name); // search these in order by name - // req.params - // req.body - // req.query - // req.header + var arg = req.params[name] !== undefined ? req.params[name] : // params + (req.body && req.body[name]) !== undefined ? req.body[name] : // body + req.query[name] !== undefined ? req.query[name] : // query + req.get(name); // header return arg; }; -/*! - * Integer test regexp. - */ - -var isint = /^[0-9]+$/; - -/*! - * Float test regexp. - */ - -var isfloat = /^([0-9]+)?\.[0-9]+$/; - -// see http://stackoverflow.com/a/6969486/69868 -function escapeRegex(d) { - return d.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, '\\$&'); -} - -// Use dynamic to coerce a value or array of values. -function dynamic(val, toType, ctx) { - if (Array.isArray(val)) { - return val.map(function(v) { - return dynamic(v, toType, ctx); - }); - } - return (new Dynamic(val, ctx)).to(toType); -} - -function coerce(str) { - if (typeof str !== 'string') return str; - if ('null' === str) return null; - if ('true' === str) return true; - if ('false' === str) return false; - if (isfloat.test(str)) return parseFloat(str, 10); - if (isint.test(str) && str.charAt(0) !== '0') return parseInt(str, 10); - return str; -} - -// coerce every string in the given object / array -function coerceAll(obj) { - var type = Array.isArray(obj) ? 'array' : typeof obj; - var i, n; - - switch (type) { - case 'string': - return coerce(obj); - case 'object': - if (obj) { - var props = Object.keys(obj); - for (i = 0, n = props.length; i < n; i++) { - var key = props[i]; - obj[key] = coerceAll(obj[key]); - } - } - break; - case 'array': - for (i = 0, n = obj.length; i < n; i++) { - coerceAll(obj[i]); - } - break; - } - - return obj; -} - function buildArgs(ctx, method, fn) { try { return ctx.buildArgs(method); diff --git a/lib/http-invocation.js b/lib/http-invocation.js index eed40a87..4fe68c96 100644 --- a/lib/http-invocation.js +++ b/lib/http-invocation.js @@ -19,7 +19,7 @@ var inherits = util.inherits; var path = require('path'); var assert = require('assert'); var request = require('request'); -var Dynamic = require('./dynamic'); +var ContextBase = require('./context-base'); var SUPPORTED_TYPES = ['json', 'application/javascript', 'text/javascript']; var qs = require('qs'); var urlUtil = require('url'); @@ -42,17 +42,18 @@ var JSON_TYPES = ['boolean', 'string', 'object', 'number']; * @property {Array} args The arguments to be used when invoking the `SharedMethod` */ -function HttpInvocation(method, ctorArgs, args, base, auth) { - this.base = base; +function HttpInvocation(method, ctorArgs, args, baseUrl, auth, typeRegistry) { + this.base = baseUrl; this.auth = auth; this.method = method; this.args = args || []; this.ctorArgs = ctorArgs || []; + this.typeRegistry = typeRegistry; this.isStatic = (method.hasOwnProperty('isStatic') && method.isStatic) || (method.hasOwnProperty('sharedMethod') && method.sharedMethod.isStatic); var namedArgs = this.namedArgs = {}; - var val, type; + var val; if (!this.isStatic) { method.restClass.ctor.accepts.forEach(function(accept) { @@ -69,6 +70,8 @@ function HttpInvocation(method, ctorArgs, args, base, auth) { namedArgs[accept.arg || accept.name] = val; } }); + + this.context = new ContextBase(method, typeRegistry); } /** @@ -261,7 +264,7 @@ HttpInvocation.prototype.invoke = function(callback) { if (res.status === 204) err = null; } if (err) return callback(err); - self.res = res; + self.res = self.context.res = res; self.transformResponse(res, body, callback); }); }; @@ -288,11 +291,6 @@ HttpInvocation.prototype.transformResponse = function(res, body, callback) { var isObject = typeof body === 'object'; var err; var hasError = res.statusCode >= 400; - var ctx = { - method: method, - req: res.req, - res: res, - }; if (hasError) { if (isObject && body.error) { @@ -316,7 +314,7 @@ HttpInvocation.prototype.transformResponse = function(res, body, callback) { /* eslint-disable one-var */ var ret = returns[i]; var name = ret.name || ret.arg; - var val, dynamic; + var val; var type = ret.type; if (ret.root) { @@ -325,19 +323,17 @@ HttpInvocation.prototype.transformResponse = function(res, body, callback) { val = res.body[name]; } - if (typeof type === 'string' && Dynamic.canConvert(type)) { - dynamic = new Dynamic(val, ctx); - val = dynamic.to(type); - } else if (Array.isArray(type) && Dynamic.canConvert(type[0])) { - type = type[0]; - for (var j = 0, k = val.length; j < k; j++) { - var _val = val[j]; - dynamic = new Dynamic(_val, ctx); - val[j] = dynamic.to(type); - } + var converter = this.typeRegistry.getConverter(type); + var result = converter.fromTypedValue(this.context, val); + debug('return arg %j: converted %j to %j', name, val, result); + + if (result.error) { + err = result.error; + err.message = g.f('Invalid return argument %j. ', name) + err.message; + return callback(err); } - callbackArgs.push(val); + callbackArgs.push(result.value); /* eslint-enable one-var */ } diff --git a/lib/number-checks.js b/lib/number-checks.js new file mode 100644 index 00000000..d9662fe7 --- /dev/null +++ b/lib/number-checks.js @@ -0,0 +1,20 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-remoting +// This file is licensed under the Artistic License 2.0. +// License text available at https://opensource.org/licenses/Artistic-2.0 + +// To support Node v.0.10.x +var number = module.exports = { + MAX_SAFE_INTEGER: Number.MAX_SAFE_INTEGER || 9007199254740991, + MIN_SAFE_INTEGER: Number.MIN_SAFE_INTEGER || -9007199254740991, + isInteger: Number.isInteger || function(value) { + return typeof value === 'number' && + isFinite(value) && + Math.floor(value) === value; + }, + isSafeInteger: Number.isSafeInteger || function(value) { + return number.isInteger(value) && + value >= number.MIN_SAFE_INTEGER && + value <= number.MAX_SAFE_INTEGER; + }, +}; diff --git a/lib/remote-objects.js b/lib/remote-objects.js index 37e11994..cfea7151 100644 --- a/lib/remote-objects.js +++ b/lib/remote-objects.js @@ -20,11 +20,11 @@ var util = require('util'); var urlUtil = require('url'); var inherits = util.inherits; var assert = require('assert'); -var Dynamic = require('./dynamic'); var SharedClass = require('./shared-class'); var SharedMethod = require('./shared-method'); var ExportsHelper = require('./exports-helper'); var PhaseList = require('loopback-phase').PhaseList; +var TypeRegistry = require('./type-registry'); // require the rest adapter for browserification // TODO(ritch) remove this somehow...? @@ -60,6 +60,7 @@ function RemoteObjects(options) { this.setMaxListeners(16); this.options = options || {}; this.exports = this.options.exports || {}; + this._typeRegistry = new TypeRegistry(this.options.types); this._classes = {}; this._setupPhases(); @@ -707,22 +708,98 @@ RemoteObjects.prototype.getScope = function(ctx, method) { * Define a named type conversion. The conversion is used when a * `SharedMethod` argument defines a type with the given `name`. * - * ```js - * remotes.defineType('MyType', function(val, ctx) { - * // use the val and ctx objects to return the concrete value - * return new MyType(val); + * ``` + * remotes.defineType('MyType', { + * // Convert the raw "value" coming typically from JSON. + * // Use the remoting context in "ctx" to access the type registry and + * // other request-related information. + * fromTypedValue: function(ctx, value) { + * if (value === undefined || value === null) + * return { value: value }; + * + * value = new MyType(value); + * var error = this.validate(ctx, value); + * return error ? { error: error } : { value: value }; + * }, + * + * // Apply any coercion needed to convert values coming from + * // string-only sources like HTTP headers and query string. + * // + * // A sloppy value is one of: + * // - a string + * // - an array containing sloppy values + * // - an object where property values are sloppy + * fromSloppyValue: function(ctx, value) { + * var objectConverter = ctx.typeRegistry.getConverter('object'); + * var result = objectConverter.fromSloppyString(ctx, value); + * return result.error ? result : this.fromTypedValue(ctx, result.value); + * }, + * + * // Perform basic validation of the value. Validations are required to be + * // synchronous. + * validate: function(ctx, value) { + * if (value === undefined || value === null) + * return null; + * if (typeof value !== 'object' || !(value instanceof MyType) { + * return new Error('Value is not an instance of MyType.'); + * } + * return null; + * }, + * }); + * + * See also `remotes.defineObjectType`. + * + * @param {String} name The type name + * @param {Object} converter A converter implementing the following methods: + * - `fromTypedValue(ctx, value) -> { value } or { error }` + * - `fromSloppyValue(ctx, value) -> { value } or { error }` + * - `validate(ctx, value) -> error or undefined/null` + */ + +RemoteObjects.prototype.defineType = function(name, converter) { + if (typeof converter === 'function') { + throw new Error(g.f( + '%s is no longer supported. Use one of the new APIs instead: %s or %s', + 'remoteObjects.defineType(name, fn)', + 'remoteObjects.defineObjectType(name, factoryFn)', + 'remoteObjects.defineType(name, converter)')); + } + this._typeRegistry.defineType(name, converter); +}; + +/** + * Define a named type conversion for an object-like type. + * The conversion is used when a `SharedMethod` argument + * defines a type with the given `name`. + * + * ``` + * remotes.defineObjectType('MyClass', function(data) { + * return new MyClass(data); * }); * ``` * - * **Note: the alias `remotes.convert()` is deprecated.** + * Under the hood, a converter is created that ensures the input data + * is an object (or sloppy value is coerced to an object) and calls + * the provided factory function to convert plain data object to + * a class instance. * * @param {String} name The type name - * @param {Function} converter + * @param {Function(Object)} factoryFn Factory function creating object + * instance from a plan-data object. */ +RemoteObjects.prototype.defineObjectType = function(name, factoryFn) { + this._typeRegistry.registerObjectType(name, factoryFn); +}; -RemoteObjects.defineType = RemoteObjects.convert = -RemoteObjects.prototype.defineType = RemoteObjects.prototype.convert = function(name, fn) { - Dynamic.define(name, fn); + throw new Error(g.f( + 'RemoteObjects.convert(name, fn) is no longer supported. ' + + 'Use remoteObjects.defineType(name, converter) instead.')); +}; + +RemoteObjects.defineType = function(name, fn) { + throw new Error(g.f( + 'RemoteObjects.defineType(name, fn) is no longer supported. ' + + 'Use remoteObjects.defineType(name, converter) instead.')); }; diff --git a/lib/rest-adapter.js b/lib/rest-adapter.js index 7e5a32ea..45b7b0c5 100644 --- a/lib/rest-adapter.js +++ b/lib/rest-adapter.js @@ -27,7 +27,6 @@ var bodyParser = require('body-parser'); var cors = require('cors'); var async = require('async'); var HttpInvocation = require('./http-invocation'); -var ContextBase = require('./context-base'); var HttpContext = require('./http-context'); var strongErrorHandler = require('strong-error-handler'); @@ -46,6 +45,7 @@ function RestAdapter(remotes, options) { this.remotes = remotes; this.Context = HttpContext; this.options = options || (remotes.options || {}).rest; + this.typeRegistry = remotes._typeRegistry; } /** @@ -122,9 +122,9 @@ RestAdapter.prototype.invoke = function(method, ctorArgs, args, callback) { var remotes = this.remotes; var restMethod = this.getRestMethodByName(method); var invocation = new HttpInvocation( - restMethod, ctorArgs, args, this.connection, remotes.auth + restMethod, ctorArgs, args, this.connection, remotes.auth, this.typeRegistry ); - var ctx = new ContextBase(restMethod); + var ctx = invocation.context; ctx.req = invocation.createRequest(); var scope = ctx.getScope(); remotes.execHooks('before', restMethod, scope, ctx, function(err) { @@ -402,7 +402,7 @@ RestAdapter.prototype._createStaticMethodHandler = function(sharedMethod) { var Context = this.Context; return function restStaticMethodHandler(req, res, next) { - var ctx = new Context(req, res, sharedMethod, self.options); + var ctx = new Context(req, res, sharedMethod, self.options, self.typeRegistry); self._invokeMethod(ctx, sharedMethod, next); }; }; @@ -412,7 +412,7 @@ RestAdapter.prototype._createPrototypeMethodHandler = function(sharedMethod) { var Context = this.Context; return function restPrototypeMethodHandler(req, res, next) { - var ctx = new Context(req, res, sharedMethod, self.options); + var ctx = new Context(req, res, sharedMethod, self.options, self.typeRegistry); // invoke the shared constructor to get an instance ctx.invoke(sharedMethod.ctor, sharedMethod.sharedCtor, function(err, inst) { diff --git a/lib/shared-method.js b/lib/shared-method.js index babc8c60..242f0bee 100644 --- a/lib/shared-method.js +++ b/lib/shared-method.js @@ -4,6 +4,7 @@ // License text available at https://opensource.org/licenses/Artistic-2.0 var g = require('strong-globalize')(); + /*! * Expose `SharedMethod`. */ @@ -13,6 +14,11 @@ var debug = require('debug')('strong-remoting:shared-method'); var util = require('util'); var traverse = require('traverse'); var assert = require('assert'); +var Context = require('./context-base'); +var numberChecks = require('./number-checks'); + +var isInteger = numberChecks.isSafeInteger; +var isSafeInteger = numberChecks.isSafeInteger; /** * Create a new `SharedMethod` (remote method) with the given `fn`. @@ -204,6 +210,8 @@ SharedMethod.prototype.getReturnArgDescByName = function(name) { */ SharedMethod.prototype.invoke = function(scope, args, remotingOptions, ctx, cb) { + assert(ctx, 'invocation context is required'); + var accepts = this.accepts; var returns = this.returns; var errors = this.errors; @@ -211,16 +219,6 @@ SharedMethod.prototype.invoke = function(scope, args, remotingOptions, ctx, cb) var sharedMethod = this; var formattedArgs = []; - if (typeof ctx === 'function') { - cb = ctx; - ctx = undefined; - } - - if (cb === undefined && typeof remotingOptions === 'function') { - cb = remotingOptions; - remotingOptions = {}; - } - // map the given arg data in order they are expected in if (accepts) { for (var i = 0; i < accepts.length; i++) { @@ -229,7 +227,7 @@ SharedMethod.prototype.invoke = function(scope, args, remotingOptions, ctx, cb) var uarg = SharedMethod.convertArg(desc, args[name]); try { - uarg = coerceAccepts(uarg, desc, name); + uarg = validateInputArgument(uarg, desc, ctx); } catch (e) { debug('- %s - ' + e.message, sharedMethod.name); return cb(e); @@ -274,7 +272,8 @@ SharedMethod.prototype.invoke = function(scope, args, remotingOptions, ctx, cb) } return retval; } catch (err) { - debug('error caught during the invocation of %s', this.name); + debug('error caught during the invocation of %s: %s', + this.name, err.stack || err); return cb(err); } }; @@ -304,86 +303,27 @@ function escapeRegex(d) { * * @param {*} uarg Argument value. * @param {Object} desc Argument description. + * @param {Context} ctx Remoting request context. * @return {*} Coerced argument. */ -function coerceAccepts(uarg, desc) { - var message; +function validateInputArgument(uarg, desc, ctx) { var name = desc.name || desc.arg; - var targetType = convertToBasicRemotingType(desc.type); - var targetTypeIsArray = Array.isArray(targetType) && targetType.length === 1; - - // If coercing an array to an erray, - // then coerce all members of the array too - if (targetTypeIsArray && Array.isArray(uarg)) { - return uarg.map(function(arg, ix) { - // when coercing array items, use only name and type, - // ignore all other root settings like "required" - return coerceAccepts(arg, { - name: name + '[' + ix + ']', - type: targetType[0], - }); - }); - } - - var actualType = SharedMethod.getType(uarg, targetType); - - // convert values to the correct type - // TODO(bajtos) Move conversions to HttpContext (and friends) - // SharedMethod should only check that argument values match argument types. - var conversionNeeded = targetType !== 'any' && - actualType !== 'undefined' && - actualType !== targetType; - - if (conversionNeeded) { - // JSON.parse can throw, so catch this error. - try { - uarg = convertValueToTargetType(name, uarg, targetType); - actualType = SharedMethod.getType(uarg, targetType); - } catch (e) { - message = g.f('invalid value for argument \'%s\' of type ' + - '\'%s\': %s. Received type was %s. Error: %s', - name, targetType, uarg, typeof uarg, e.message); - throw badArgumentError(message); - } - } - - var typeMismatch = targetType !== 'any' && - actualType !== 'undefined' && - targetType !== actualType && - // In JavaScript, an array is an object too (typeof [] === 'object'). - // However, SharedMethod.getType([]) returns 'array' instead of 'object'. - // We must explicitly allow assignment of an array value to an argument - // of type 'object'. - !(targetType === 'object' && actualType === 'array'); - - if (typeMismatch) { - message = g.f('Invalid value for argument \'%s\' of type ' + - '\'%s\': %s. Received type was converted to %s.', - name, targetType, uarg, typeof uarg); - throw badArgumentError(message); - } // Verify that a required argument has a value - // FIXME(bajtos) "null" should be treated as no value too - if (actualType === 'undefined') { - if (desc.required) { + if (desc.required) { + var argIsNotSet = uarg === null || uarg === undefined || uarg === ''; + if (argIsNotSet) { throw badArgumentError(g.f('%s is a required argument', name)); - } else { - return undefined; } } - if (actualType === 'number' && Number.isNaN(uarg)) { - throw badArgumentError(g.f('%s must be a {{number}}', name)); - } - if (actualType === 'integer') { - if (Number.isNaN(uarg)) { - throw badArgumentError(g.f('%s must be an {{integer}}', name)); - } - if (!Number.isSafeInteger(uarg)) { - throw badArgumentError(g.f('%s must be a safe {{integer}}', name)); - } + var converter = ctx.typeRegistry.getConverter(desc.type); + var err = converter.validate(ctx, uarg); + if (err) { + err.message = g.f('Invalid argument %j. ', name) + err.message; + throw err; } + return uarg; } @@ -425,31 +365,6 @@ function convertToBasicRemotingType(type) { } } -function convertValueToTargetType(argName, value, targetType) { - switch (targetType) { - case 'string': - return String(value).valueOf(); - case 'date': - return new Date(value); - case 'number': - case 'integer': - return Number(value).valueOf(); - case 'boolean': - return Boolean(value).valueOf(); - // Other types such as 'object', 'array', - // ModelClass, ['string'], or [ModelClass] - default: - switch (typeof value) { - case 'string': - return JSON.parse(value); - case 'object': - return value; - default: - throw badArgumentError(g.f('%s must be %s', argName, targetType)); - } - } -} - /** * Returns an appropriate type based on `val`. * @param {*} val The value to determine the type for @@ -466,7 +381,7 @@ SharedMethod.getType = function(val, targetType) { case 'string': return type; case 'number': - return Number.isInteger(val) && targetType === 'integer' ? + return isInteger(val) && targetType === 'integer' ? 'integer' : 'number'; case 'object': // null @@ -612,7 +527,7 @@ SharedMethod.toResult = function(returns, raw, ctx) { if (targetType === 'integer') { var message; - if (!Number.isInteger(val)) { + if (!isInteger(val)) { message = g.f( 'Invalid return value for argument \'%s\' of type ' + '\'%s\': %s. Received type was %s.', @@ -620,7 +535,7 @@ SharedMethod.toResult = function(returns, raw, ctx) { throw internalServerError(message); } - if (!Number.isSafeInteger(val)) { + if (!isSafeInteger(val)) { message = g.f( 'Unsafe integer value returned for argument \'%s\' of type ' + '\'%s\': %s.', @@ -717,16 +632,3 @@ SharedMethod.prototype.addAlias = function(alias) { this.aliases.push(alias); } }; - -// To support Node v.0.10.x -Number.isInteger = Number.isInteger || function(value) { - return typeof value === 'number' && - isFinite(value) && - Math.floor(value) === value; -}; - -Number.MAX_SAFE_INTEGER = Number.MAX_SAFE_INTEGER || Math.pow(2, 53) - 1; - -Number.isSafeInteger = Number.isSafeInteger || function(value) { - return Number.isInteger(value) && Math.abs(value) <= Number.MAX_SAFE_INTEGER; -}; diff --git a/lib/type-registry.js b/lib/type-registry.js new file mode 100644 index 00000000..1d855355 --- /dev/null +++ b/lib/type-registry.js @@ -0,0 +1,118 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-remoting +// This file is licensed under the Artistic License 2.0. +// License text available at https://opensource.org/licenses/Artistic-2.0 + +'use strict'; + +var ArrayConverter = require('./types/array'); +var assert = require('assert'); +var debug = require('debug')('strong-remoting:types'); +var g = require('strong-globalize')(); + +module.exports = TypeRegistry; + +function TypeRegistry(options) { + this._options = options || {}; + this._types = Object.create(null); + this._registerBuiltinTypes(); +} + +TypeRegistry.prototype.registerType = function(typeName, converter) { + assert(typeof typeName === 'string' && typeName, + 'typeName must be a non-empty string'); + assert(typeof converter === 'object' && converter, + 'converter must be an object'); + assert(typeof converter.fromTypedValue === 'function', + 'converter.fromTypedValue must be a function'); + assert(typeof converter.fromSloppyValue === 'function', + 'converter.fromSloppyValue must be a function'); + assert(typeof converter.validate === 'function', + 'converter.validate must be a function'); + + typeName = typeName.toLowerCase(); + + if (typeName in this._types) { + if (this._options.warnWhenOverridingType !== false) + g.warn('Warning: overriding remoting type %s', typeName); + else + debug('Warning: overriding remoting type %s', typeName); + } + + this._types[typeName] = converter; +}; + +TypeRegistry.prototype.registerObjectType = function(typeName, factoryFn) { + assert(typeof typeName === 'string' && typeName, + 'typeName must be a non-empty string'); + assert(typeof factoryFn === 'function', 'factoryFn must be a function'); + + var converter = { + fromTypedValue: function(ctx, data) { + var objectConverter = ctx.typeRegistry.getConverter('object'); + var result = objectConverter.fromTypedValue(ctx, data); + if (result.error || result.value === undefined || result.value === null) + return result; + + try { + return { value: factoryFn(result.value) }; + } catch (err) { + return { error: err }; + } + }, + + fromSloppyValue: function(ctx, value) { + var objectConverter = ctx.typeRegistry.getConverter('object'); + var result = objectConverter.fromSloppyValue(ctx, value); + return result.error ? result : this.fromTypedValue(ctx, result.value); + }, + + validate: function(ctx, value) { + return ctx.typeRegistry.getConverter('object').validate(ctx, value); + }, + }; + + this.registerType(typeName, converter); +}; + +TypeRegistry.prototype.getConverter = function(type) { + assert(typeof type === 'string' || Array.isArray(type), + 'type must be either an array or a string.'); + + if (type === 'array') + type = ['any']; + + if (Array.isArray(type)) { + if (type.length !== 1) { + g.warn('Array types with more than one item type are not supported. ' + + 'Using the first item type and ignoring the rest.'); + } + + return new ArrayConverter(type[0] || 'any'); + } + + type = type.toLowerCase(); + + // TODO fall-back to Dynamic converters + return this._types[type] || this._getUnknownTypeConverter(type); +}; + +TypeRegistry.prototype._registerBuiltinTypes = function() { + // NOTE we have to explicitly enumerate all scripts to support browserify + this.registerType('any', require('./types/any')); + this.registerType('boolean', require('./types/boolean')); + this.registerType('date', require('./types/date')); + this.registerType('integer', require('./types/integer')); + this.registerType('number', require('./types/number')); + this.registerType('object', require('./types/object')); + this.registerType('string', require('./types/string')); +}; + +TypeRegistry.prototype._getUnknownTypeConverter = function(type) { + if (this._options.warnOnUnknownType !== false) + g.warn('Treating unknown remoting type %j as "any"', type); + else + debug('Treating unknown remoting type %j as "any"', type); + + return this.getConverter('any'); +}; diff --git a/lib/types/any.js b/lib/types/any.js new file mode 100644 index 00000000..1706f3b1 --- /dev/null +++ b/lib/types/any.js @@ -0,0 +1,60 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-remoting +// This file is licensed under the Artistic License 2.0. +// License text available at https://opensource.org/licenses/Artistic-2.0 + +'use strict'; + +var debug = require('debug')('strong-remoting:http-coercion'); +var g = require('strong-globalize')(); +var numberChecks = require('../number-checks'); + +var MAX_SAFE_INTEGER = numberChecks.MAX_SAFE_INTEGER; +var MIN_SAFE_INTEGER = numberChecks.MIN_SAFE_INTEGER; + +var IS_INT_REGEX = /^\-?(?:[0-9]|[1-9][0-9]*)$/; +var IS_FLOAT_REGEX = /^\-?([0-9]+)?\.[0-9]+$/; + +module.exports = { + fromTypedValue: function(ctx, value) { + return { value: value }; + }, + + fromSloppyValue: function(ctx, value) { + if (value === 'null' || value === null) + return { value: null }; + + if (typeof value !== 'string') + return { value: value }; + + if (value === '') { + // Pass on empty string as undefined. + // undefined was chosen so that it plays well with ES6 default parameters. + return { value: undefined }; + } + + if (value.toLowerCase() === 'true') + return { value: true }; + + if (value.toLowerCase() === 'false') + return { value: false }; + + if (IS_FLOAT_REGEX.test(value) || IS_INT_REGEX.test(value)) { + var num = +value; + // Cap at MAX_SAFE_INTEGER so we don't lose precision. + if (MIN_SAFE_INTEGER <= num && num <= MAX_SAFE_INTEGER) + value = num; + } + + var objectConverter = ctx.typeRegistry.getConverter('object'); + var objectResult = objectConverter.fromSloppyValue(ctx, value); + if (!objectResult.error && objectResult.value) + return objectResult; + + return { value: value }; + }, + + validate: function(ctx, value) { + // no-op, all values are valid + }, +}; diff --git a/lib/types/array.js b/lib/types/array.js new file mode 100644 index 00000000..831a1ae0 --- /dev/null +++ b/lib/types/array.js @@ -0,0 +1,157 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-remoting +// This file is licensed under the Artistic License 2.0. +// License text available at https://opensource.org/licenses/Artistic-2.0 + +'use strict'; + +var assert = require('assert'); +var debug = require('debug')('strong-remoting:http-coercion'); +var escapeRegex = require('escape-string-regexp'); +var g = require('strong-globalize')(); + +module.exports = ArrayConverter; + +function ArrayConverter(itemType) { + this._itemType = itemType; +} + +ArrayConverter.prototype.fromTypedValue = function(ctx, value) { + if (value === undefined || value === null) + return { value: value }; + + if (!Array.isArray(value)) + return { error: notAnArrayError() }; + + var items = new Array(value.length); + var itemResult; + var itemConverter = ctx.typeRegistry.getConverter(this._itemType); + + for (var ix in value) { + itemResult = itemConverter.fromTypedValue(ctx, value[ix]); + itemResult = validateConverterResult(itemResult); + debug('typed item result: %j -> %j as %s', + value[ix], itemResult, this._itemType); + + if (itemResult.error) + return itemResult; + items[ix] = itemResult.value; + } + return { value: items }; +}; + +ArrayConverter.prototype.fromSloppyValue = function(ctx, value) { + if (value === undefined || value === '') { + // undefined was chosen so that it plays well with ES6 default parameters. + return { value: undefined }; + } + + if (value === null || value === 'null') { + return { value: null }; + } + + return this._fromTypedValueString(ctx, value) || + this._fromDelimitedString(ctx, value) || + this._fromSloppyData(ctx, value); +}; + +ArrayConverter.prototype._fromTypedValueString = function(ctx, value) { + var looksLikeJsonArray = typeof value === 'string' && + value[0] === '[' && value[value.length - 1] === ']'; + + if (!looksLikeJsonArray) + return null; + + // If it looks like a JSON array, try to parse it. + try { + var result = JSON.parse(value); + debug('parsed %j as JSON: %j', value, result); + return this.fromTypedValue(ctx, result); + } catch (ex) { + debug('Cannot parse array value %j. %s', value, ex); + var err = new Error(g.f('Cannot parse JSON-encoded array value.')); + err.statusCode = 400; + return { error: err }; + } +}; + +ArrayConverter.prototype._fromDelimitedString = function(ctx, value) { + if (typeof value !== 'string') + return null; + + // The user may set delimiters like ',', or ';' to designate array items + // for easier usage. + var delims = ctx.options && ctx.options.arrayItemDelimiters; + if (!delims) + return null; + + // Construct delimiter regex if input was an array. Overwrite option + // so this only needs to happen once. + if (Array.isArray(delims)) { + delims = new RegExp(delims.map(escapeRegex).join('|'), 'g'); + ctx.options.arrayItemDelimiters = delims; + } + + var items = value.split(delims); + + // perform sloppy-string coercion + return this.fromSloppyValue(ctx, items); +}; + +ArrayConverter.prototype._fromSloppyData = function(ctx, value) { + if (!Array.isArray(value)) { + // Alright, not array-like, just wrap it in an array on the way out. + value = [value]; + } + + debug('Intermediate sloppy array result: %j', value); + + var items = new Array(value.length); + var itemResult; + var itemConverter = ctx.typeRegistry.getConverter(this._itemType); + + for (var ix in value) { + itemResult = itemConverter.fromSloppyValue(ctx, value[ix]); + itemResult = validateConverterResult(itemResult); + debug('item %d: sloppy converted %j to %j', ix, value[ix], itemResult); + if (itemResult.error) + return itemResult; + items[ix] = itemResult.value; + } + return { value: items }; +}; + +ArrayConverter.prototype.validate = function(ctx, value) { + if (value === undefined || value === null) + return null; + + if (!Array.isArray(value)) + return notAnArrayError(); + + var itemConverter = ctx.typeRegistry.getConverter(this._itemType); + var itemError; + for (var ix in value) { + itemError = itemConverter.validate(ctx, value[ix]); + if (itemError) return itemError; + } +}; + +function notAnArrayError() { + var err = new Error(g.f('Value is not an array.')); + err.statusCode = 400; + return err; +} + +function validateConverterResult(result) { + var isValid = typeof result === 'object' && + ('error' in result || 'value' in result); + if (isValid) + return result; + + var err = new (assert.AssertionError)({ + message: + 'Type conversion result should have "error" or "value" property. ' + + 'Got ' + JSON.stringify(result) + ' instead.', + }); + return { error: err }; +} diff --git a/lib/types/boolean.js b/lib/types/boolean.js new file mode 100644 index 00000000..4b6859ef --- /dev/null +++ b/lib/types/boolean.js @@ -0,0 +1,46 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-remoting +// This file is licensed under the Artistic License 2.0. +// License text available at https://opensource.org/licenses/Artistic-2.0 + +'use strict'; + +var debug = require('debug')('strong-remoting:http-coercion'); +var g = require('strong-globalize')(); + +module.exports = { + fromTypedValue: function(ctx, value) { + var error = this.validate(ctx, value); + return error ? { error: error } : { value: value }; + }, + + fromSloppyValue: function(ctx, value) { + if (value === '' || value === undefined) + return { value: undefined }; + + if (typeof value === 'string') { + switch (value.toLowerCase()) { + case 'false': + case '0': + return { value: false }; + case 'true': + case '1': + return { value: true }; + } + } + return { error: invalidBooleanError() }; + }, + + validate: function(ctx, value) { + if (value === undefined || typeof value === 'boolean') + return null; + + return invalidBooleanError(); + }, +}; + +function invalidBooleanError() { + var err = new Error(g.f('Value is not a boolean.')); + err.statusCode = 400; + return err; +} diff --git a/lib/types/date.js b/lib/types/date.js new file mode 100644 index 00000000..4d9e7384 --- /dev/null +++ b/lib/types/date.js @@ -0,0 +1,51 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-remoting +// This file is licensed under the Artistic License 2.0. +// License text available at https://opensource.org/licenses/Artistic-2.0 + +'use strict'; + +var debug = require('debug')('strong-remoting:http-coercion'); +var g = require('strong-globalize')(); + +module.exports = { + fromTypedValue: function(ctx, value) { + if (value === undefined) + return { value: value }; + + if (value === null) + return { error: invalidDateError() }; + + if (typeof value !== 'number' && typeof value !== 'string') + return { error: invalidDateError() }; + + var result = new Date(value); + var error = this.validate(result); + return error ? { error: error } : { value: result }; + }, + + fromSloppyValue: function(ctx, value) { + if (value === '') + return { value: undefined }; + + // we don't have any special sloppy conversion yet + // TODO(bajtos) convert numeric strings to numbers first + return this.fromTypedValue(ctx, value); + }, + + validate: function(ctx, value) { + if (value === undefined) + return null; + + if (value instanceof Date && !Number.isNaN(value.getTime())) + return null; + + return invalidDateError(); + }, +}; + +function invalidDateError() { + var err = new Error(g.f('Value is not a valid date.')); + err.statusCode = 400; + return err; +} diff --git a/lib/types/integer.js b/lib/types/integer.js new file mode 100644 index 00000000..962a9ce2 --- /dev/null +++ b/lib/types/integer.js @@ -0,0 +1,41 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-remoting +// This file is licensed under the Artistic License 2.0. +// License text available at https://opensource.org/licenses/Artistic-2.0 + +'use strict'; + +var debug = require('debug')('strong-remoting:http-coercion'); +var g = require('strong-globalize')(); +var isSafeInteger = require('../number-checks').isSafeInteger; +var numberConverter = require('./number'); + +module.exports = { + fromTypedValue: function(ctx, value) { + var error = this.validate(ctx, value); + return error ? { error: error } : { value: value }; + }, + + fromSloppyValue: function(ctx, value) { + var result = numberConverter.fromSloppyValue(ctx, value); + if (result.error) + return result; + return this.fromTypedValue(ctx, result.value); + }, + + validate: function(ctx, value) { + if (value === undefined) + return null; + + var err = numberConverter.validate(ctx, value); + if (err) + return err; + + if (isSafeInteger(value)) + return null; + + err = new Error(g.f('Value is not a safe integer.')); + err.statusCode = 400; + return err; + }, +}; diff --git a/lib/types/number.js b/lib/types/number.js new file mode 100644 index 00000000..e56c1573 --- /dev/null +++ b/lib/types/number.js @@ -0,0 +1,36 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-remoting +// This file is licensed under the Artistic License 2.0. +// License text available at https://opensource.org/licenses/Artistic-2.0 + +'use strict'; + +var debug = require('debug')('strong-remoting:http-coercion'); +var g = require('strong-globalize')(); + +module.exports = { + fromTypedValue: function(ctx, value) { + var error = this.validate(ctx, value); + return error ? { error: error } : { value: value }; + }, + + fromSloppyValue: function(ctx, value) { + if (value === undefined || value === '') + return { value: undefined }; + + var result = +value; + return this.fromTypedValue(ctx, result); + }, + + validate: function(ctx, value) { + if (value === undefined) + return null; + + if (typeof value === 'number' && !isNaN(value)) + return null; + + var err = Error(g.f('Value is not a number.')); + err.statusCode = 400; + return err; + }, +}; diff --git a/lib/types/object.js b/lib/types/object.js new file mode 100644 index 00000000..69373a7e --- /dev/null +++ b/lib/types/object.js @@ -0,0 +1,55 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-remoting +// This file is licensed under the Artistic License 2.0. +// License text available at https://opensource.org/licenses/Artistic-2.0 + +'use strict'; + +var debug = require('debug')('strong-remoting:http-coercion'); +var g = require('strong-globalize')(); + +module.exports = { + fromTypedValue: function(ctx, value) { + var error = this.validate(ctx, value); + return error ? { error: error } : { value: value }; + }, + + fromSloppyValue: function(ctx, value) { + if (value === undefined || value === '') { + // undefined was chosen so that it plays well with ES6 default parameters. + return { value: undefined }; + } + + if (value === null || value === 'null') + return { value: null }; + + var looksLikeJson = typeof value === 'string' && ( + (value[0] === '[' && value[value.length - 1] === ']') || + (value[0] === '{' && value[value.length - 1] === '}')); + + if (looksLikeJson) { + try { + var result = JSON.parse(value); + debug('parsed %j as JSON: %j', value, result); + return this.fromTypedValue(ctx, result); + } catch (ex) { + debug('Cannot parse object value %j. %s', value, ex); + var err = new Error(g.f('Cannot parse JSON-encoded object value.')); + err.statusCode = 400; + return { error: err }; + } + } + + // NOTE: nested values in objects are intentionally not coerced + return this.fromTypedValue(ctx, value); + }, + + validate: function(ctx, value) { + if (value === undefined || typeof value === 'object') + return null; + + var err = new Error(g.f('Value is not an object.')); + err.statusCode = 400; + return err; + }, +}; diff --git a/lib/types/string.js b/lib/types/string.js new file mode 100644 index 00000000..f3ab3d4b --- /dev/null +++ b/lib/types/string.js @@ -0,0 +1,40 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-remoting +// This file is licensed under the Artistic License 2.0. +// License text available at https://opensource.org/licenses/Artistic-2.0 + +'use strict'; + +var debug = require('debug')('strong-remoting:http-coercion'); +var g = require('strong-globalize')(); + +module.exports = { + fromTypedValue: function(ctx, value) { + var error = this.validate(ctx, value); + return error ? { error: error } : { value: value }; + }, + + fromSloppyValue: function(ctx, value) { + if (value === '') { + // Pass on empty string as undefined. + // undefined was chosen so that it plays well with ES6 default parameters. + return { value: undefined }; + } + + // TODO(bajtos) should we reject objects/arrays values created from complex + // query strings, e.g. ?arg[1]=a&arg[2]=b + if (value !== undefined && value !== null) + value = '' + value; + + return this.fromTypedValue(ctx, value); + }, + + validate: function(ctx, value) { + if (value === undefined || typeof value === 'string') + return null; + + var err = new Error(g.f('Value is not a string.')); + err.statusCode = 400; + return err; + }, +}; diff --git a/package.json b/package.json index e46eb898..9af39a5c 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "debug": "^2.2.0", "depd": "^1.1.0", "eventemitter2": "^2.1.0", + "escape-string-regexp": "^1.0.5", "express": "4.x", "inflection": "^1.7.1", "jayson": "^1.2.0", diff --git a/test/http-context.test.js b/test/http-context.test.js deleted file mode 100644 index 3d257a24..00000000 --- a/test/http-context.test.js +++ /dev/null @@ -1,126 +0,0 @@ -// Copyright IBM Corp. 2015,2016. All Rights Reserved. -// Node module: strong-remoting -// This file is licensed under the Artistic License 2.0. -// License text available at https://opensource.org/licenses/Artistic-2.0 - -var request = require('supertest'); -var HttpContext = require('../lib/http-context'); -var SharedMethod = require('../lib/shared-method'); -var Dynamic = require('../lib/dynamic'); -var expect = require('chai').expect; - -describe('HttpContext', function() { - beforeEach(function() { - var test = this; - }); - - describe('ctx.args', function() { - describe('arguments with a defined type (not any)', function() { - it('should include a named string arg', givenMethodExpectArg({ - type: 'string', - input: 'foobar', - expectedValue: 'foobar', - })); - it('should coerce integer strings into actual numbers', givenMethodExpectArg({ - type: 'number', - input: '123456', - expectedValue: 123456, - })); - it('should coerce float strings into actual numbers', givenMethodExpectArg({ - type: 'number', - input: '0.123456', - expectedValue: 0.123456, - })); - it('should coerce number strings preceded by 0 into numbers', givenMethodExpectArg({ - type: 'number', - input: '000123', - expectedValue: 123, - })); - it('should not coerce null strings into null', givenMethodExpectArg({ - type: 'string', - input: 'null', - expectedValue: 'null', - })); - it('should coerce array types properly with non-array input', givenMethodExpectArg({ - type: ['string'], - input: 123, - expectedValue: ['123'], - })); - it('should not coerce a single string into a number', givenMethodExpectArg({ - type: ['string'], - input: '123', - expectedValue: ['123'], - })); - }); - - describe('arguments without a defined type (or any)', function() { - it('should coerce boolean strings into actual booleans', givenMethodExpectArg({ - type: 'any', - input: 'true', - expectedValue: true, - })); - it('should coerce integer strings into actual numbers', givenMethodExpectArg({ - type: 'any', - input: '123456', - expectedValue: 123456, - })); - it('should coerce float strings into actual numbers', givenMethodExpectArg({ - type: 'any', - input: '0.123456', - expectedValue: 0.123456, - })); - it('should coerce null strings into null', givenMethodExpectArg({ - type: 'any', - input: 'null', - expectedValue: null, - })); - it('should coerce number strings preceded by 0 into strings', givenMethodExpectArg({ - type: 'any', - input: '000123', - expectedValue: '000123', - })); - }); - - describe('arguments with custom type', function() { - Dynamic.define('CustomType', function(val) { - return JSON.parse(val); - }); - - it('should coerce dynamic type with string prop into object', givenMethodExpectArg({ - type: 'CustomType', - input: JSON.stringify({ stringProp: 'string' }), - expectedValue: { stringProp: 'string' }, - })); - - it('should coerce dynamic type with int prop into object', givenMethodExpectArg({ - type: 'CustomType', - input: JSON.stringify({ intProp: 1 }), - expectedValue: { intProp: 1 }, - })); - }); - }); -}); - -function givenMethodExpectArg(options) { - return function(done) { - var method = new SharedMethod(noop, 'testMethod', noop, { - accepts: [{ arg: 'testArg', type: options.type }], - }); - - var app = require('express')(); - - app.get('/', function(req, res) { - var ctx = new HttpContext(req, res, method); - try { - expect(ctx.args.testArg).to.eql(options.expectedValue); - } catch (e) { - return done(e); - } - done(); - }); - - request(app).get('/?testArg=' + options.input).end(); - }; -} - -function noop() {} diff --git a/test/http-invocation.test.js b/test/http-invocation.test.js index 2162c006..505a38c9 100644 --- a/test/http-invocation.test.js +++ b/test/http-invocation.test.js @@ -6,9 +6,9 @@ var assert = require('assert'); var HttpInvocation = require('../lib/http-invocation'); var SharedMethod = require('../lib/shared-method'); -var Dynamic = require('../lib/dynamic'); var extend = require('util')._extend; var expect = require('chai').expect; +var TypeRegistry = require('../lib/type-registry'); describe('HttpInvocation', function() { describe('namedArgs', function() { @@ -16,7 +16,7 @@ describe('HttpInvocation', function() { var method = givenSharedStaticMethod({ accepts: accepts, }); - var inv = new HttpInvocation(method, null, inputArgs); + var inv = givenInvocation(method, { args: inputArgs }); expect(inv.namedArgs).to.deep.equal(expectedNamedArgs); } @@ -78,18 +78,10 @@ describe('HttpInvocation', function() { }); }); }); - function setupReturnTypes(returns, converterName, converter, res, cb) { - var method = givenSharedStaticMethod({ returns: returns }); - var inv = new HttpInvocation(method); - var body = res.body || {}; - - Dynamic.define(converterName, converter); - inv.transformResponse(res, body, cb); - } describe('transformResponse', function() { it('should return a single instance of TestClass', function(done) { - setupReturnTypes({ + transformReturnType({ arg: 'data', type: 'bar', root: true, @@ -98,7 +90,9 @@ describe('HttpInvocation', function() { }, { body: { foo: 'bar' }, }, function(err, inst) { - expect(err).to.be.null; + if (err) return done(err); + if (inst.error) return done(inst.error); + expect(inst).to.be.instanceOf(TestClass); expect(inst.foo).to.equal('bar'); done(); @@ -110,7 +104,7 @@ describe('HttpInvocation', function() { }); it('should return an array of TestClass instances', function(done) { - setupReturnTypes({ + transformReturnType({ arg: 'data', type: ['bar'], root: true, @@ -122,7 +116,9 @@ describe('HttpInvocation', function() { { foo: 'grok' }, ], }, function(err, insts) { - expect(err).to.be.null; + if (err) return done(err); + if (insts.error) return done(insts.error); + expect(insts).to.be.an('array'); expect(insts[0]).to.be.instanceOf(TestClass); expect(insts[1]).to.be.instanceOf(TestClass); @@ -138,7 +134,7 @@ describe('HttpInvocation', function() { it('should forward all error properties', function(done) { var method = givenSharedStaticMethod({}); - var inv = new HttpInvocation(method); + var inv = givenInvocation(method); var res = { statusCode: 555, body: { @@ -169,7 +165,7 @@ describe('HttpInvocation', function() { it('should forward statusCode and non-object error response', function(done) { var method = givenSharedStaticMethod({}); - var inv = new HttpInvocation(method); + var inv = givenInvocation(method); var res = { statusCode: 555, body: 'error body', @@ -184,6 +180,18 @@ describe('HttpInvocation', function() { done(); }); }); + + function transformReturnType(returns, typeName, typeFactoryFn, res, cb) { + var method = givenSharedStaticMethod({ returns: returns }); + + var typeRegistry = new TypeRegistry(); + typeRegistry.registerObjectType(typeName, typeFactoryFn); + + var inv = givenInvocation(method, { typeRegistry: typeRegistry }); + var body = res.body || {}; + + inv.transformResponse(res, body, cb); + } }); }); @@ -199,3 +207,13 @@ function givenSharedStaticMethod(fn, config) { extend(testClass.testMethod, config); return SharedMethod.fromFunction(fn, 'testStaticMethodName', null, true); } + +function givenInvocation(method, params) { + params = params || {}; + return new HttpInvocation(method, + params.ctorArgs, + params.args, + params.baseUrl, + params.auth, + params.typeRegistry || new TypeRegistry()); +} diff --git a/test/rest-coercion/_urlencoded.context.js b/test/rest-coercion/_urlencoded.context.js index 2b036db4..b0c94591 100644 --- a/test/rest-coercion/_urlencoded.context.js +++ b/test/rest-coercion/_urlencoded.context.js @@ -57,7 +57,7 @@ module.exports = function createUrlEncodedContext(ctx, target) { var niceInput = queryString === EMPTY_QUERY ? TARGET_QUERY_STRING ? 'empty query' : 'empty form' : - queryString; + '?' + queryString; var niceExpectation = ctx.prettyExpectation(expectedValue); var testName = format('coerces %s to %s', niceInput, niceExpectation); diff --git a/test/rest-coercion/jsonbody-any.suite.js b/test/rest-coercion/jsonbody-any.suite.js index d42d13e2..789111ea 100644 --- a/test/rest-coercion/jsonbody-any.suite.js +++ b/test/rest-coercion/jsonbody-any.suite.js @@ -15,7 +15,7 @@ module.exports = function(ctx) { describe('json body - any - required', function() { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'anyname', type: 'any', required: true }, [ - [null, null], // should be: ERROR_BAD_REQUEST + [null, ERROR_BAD_REQUEST], // Both empty array and empty object are valid values for "any" [[]], [{}], diff --git a/test/rest-coercion/jsonbody-array.suite.js b/test/rest-coercion/jsonbody-array.suite.js index 1c507924..ddd20ecd 100644 --- a/test/rest-coercion/jsonbody-array.suite.js +++ b/test/rest-coercion/jsonbody-array.suite.js @@ -23,13 +23,15 @@ module.exports = function(ctx) { [[]], // an empty array is a valid value for required array [[true, false]], // invalid values - should trigger ERROR_BAD_REQUEST - [null, [false]], // should be: ERROR_BAD_REQUEST + [null, ERROR_BAD_REQUEST], ]); }); describe('json body - array of booleans - optional', function() { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'anyname', type: ['boolean'] }, [ + // no value is provided + [null], // empty array [[]], @@ -39,32 +41,33 @@ module.exports = function(ctx) { [[true, false], [true, false]], // Value is not an array - should return ERROR_BAD_REQUEST - [null, [false]], - [false, [false]], - [true, [true]], - [0, [false]], - [1, [true]], - [2, [true]], - [-1, [true]], - ['"text"', [true]], - [{}, [true]], + [false, ERROR_BAD_REQUEST], + [true, ERROR_BAD_REQUEST], + [0, ERROR_BAD_REQUEST], + [1, ERROR_BAD_REQUEST], + [2, ERROR_BAD_REQUEST], + [-1, ERROR_BAD_REQUEST], + ['"text"', ERROR_BAD_REQUEST], + [{}, ERROR_BAD_REQUEST], // Array items have wrong type - should return ERROR_BAD_REQUEST - [[null], [false]], - [['true', 'false'], [true, false]], - [['0'], [false]], - [['1'], [true]], - [['2'], [true]], - [['-1'], [true]], - [['text'], [true]], - [[{}], [true]], - [[[]], [true]], + [[null], ERROR_BAD_REQUEST], + [['true', 'false'], ERROR_BAD_REQUEST], + [['0'], ERROR_BAD_REQUEST], + [['1'], ERROR_BAD_REQUEST], + [['2'], ERROR_BAD_REQUEST], + [['-1'], ERROR_BAD_REQUEST], + [['text'], ERROR_BAD_REQUEST], + [[{}], ERROR_BAD_REQUEST], + [[[]], ERROR_BAD_REQUEST], ]); }); describe('json body - array of numbers - optional', function() { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'anyname', type: ['number'] }, [ + // no value is provided + [null], // empty array [[]], @@ -82,31 +85,30 @@ module.exports = function(ctx) { [[-1.234e+30], [-1.234e+30]], // Value is not an array - should return ERROR_BAD_REQUEST - [null, [0]], - [false, [0]], - [true, [1]], - [0, [0]], - ['"0"', [0]], - [1, [1]], - ['"1"', [1]], - [-1, [-1]], - ['"-1"', [-1]], - [1.2, [1.2]], - ['"1.2"', [1.2]], - [-1.2, [-1.2]], - ['"-1.2"', [-1.2]], + [false, ERROR_BAD_REQUEST], + [true, ERROR_BAD_REQUEST], + [0, ERROR_BAD_REQUEST], + ['"0"', ERROR_BAD_REQUEST], + [1, ERROR_BAD_REQUEST], + ['"1"', ERROR_BAD_REQUEST], + [-1, ERROR_BAD_REQUEST], + ['"-1"', ERROR_BAD_REQUEST], + [1.2, ERROR_BAD_REQUEST], + ['"1.2"', ERROR_BAD_REQUEST], + [-1.2, ERROR_BAD_REQUEST], + ['"-1.2"', ERROR_BAD_REQUEST], ['"text"', ERROR_BAD_REQUEST], [{}, ERROR_BAD_REQUEST], [{ x: true }, ERROR_BAD_REQUEST], // Array items have wrong type - should return ERROR_BAD_REQUEST - [[null], [0]], - [[true], [1]], - [['0'], [0]], - [['1'], [1]], - [['-1'], [-1]], - [['1.2'], [1.2]], - [['-1.2'], [-1.2]], + [[null], ERROR_BAD_REQUEST], + [[true], ERROR_BAD_REQUEST], + [['0'], ERROR_BAD_REQUEST], + [['1'], ERROR_BAD_REQUEST], + [['-1'], ERROR_BAD_REQUEST], + [['1.2'], ERROR_BAD_REQUEST], + [['-1.2'], ERROR_BAD_REQUEST], [['text'], ERROR_BAD_REQUEST], [[1, 'text'], ERROR_BAD_REQUEST], ]); @@ -115,6 +117,8 @@ module.exports = function(ctx) { describe('json body - array of strings - optional', function() { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'anyname', type: ['string'] }, [ + // no value is provided + [null], // Empty array [[]], @@ -124,26 +128,27 @@ module.exports = function(ctx) { [['one', 'two']], // Value is not an array - should return ERROR_BAD_REQUEST - [null, ['null']], - [false, ['false']], - [true, ['true']], - [0, ['0']], - [1, ['1']], - ['"text"', ['text']], - [{}, ['[object Object]']], + [false, ERROR_BAD_REQUEST], + [true, ERROR_BAD_REQUEST], + [0, ERROR_BAD_REQUEST], + [1, ERROR_BAD_REQUEST], + ['"text"', ERROR_BAD_REQUEST], + [{}, ERROR_BAD_REQUEST], // Array items have wrong type - should return ERROR_BAD_REQUEST - [[null], ['null']], - [[1], ['1']], - [[true], ['true']], - [[{}], ['[object Object]']], - [[[]], ['']], + [[null], ERROR_BAD_REQUEST], + [[1], ERROR_BAD_REQUEST], + [[true], ERROR_BAD_REQUEST], + [[{}], ERROR_BAD_REQUEST], + [[[]], ERROR_BAD_REQUEST], ]); }); describe('json body - array of dates - optional', function() { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'anyname', type: ['date'] }, [ + // no value is provided + [null], // Empty array [[]], @@ -160,23 +165,24 @@ module.exports = function(ctx) { ]], // Value is not an array - should return ERROR_BAD_REQUEST - [null, [new Date(0)]], - [false, [new Date(0)]], - [true, [new Date(1)]], + [false, ERROR_BAD_REQUEST], + [true, ERROR_BAD_REQUEST], ['text', ERROR_BAD_REQUEST], ['2016-05-19T13:28:51.299Z', ERROR_BAD_REQUEST], // Array items have wrong type - should return ERROR_BAD_REQUEST - [[null], [new Date(0)]], - [[false], [new Date(0)]], - [[true], [new Date(1)]], - [['text'], [INVALID_DATE]], + [[null], ERROR_BAD_REQUEST], + [[false], ERROR_BAD_REQUEST], + [[true], ERROR_BAD_REQUEST], + [['text'], ERROR_BAD_REQUEST], ]); }); describe('json body - array of any - optional', function() { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'anyname', type: ['any'] }, [ + // no value is provided + [null], // Empty array [[]], @@ -220,16 +226,15 @@ module.exports = function(ctx) { [['text', 10, false]], // Value is not an array - should return ERROR_BAD_REQUEST - [null, [null]], - [false, [false]], - [true, [true]], - [0, [0]], - [1, [1]], - [-1, [-1]], - [1.2, [1.2]], - [-1.2, [-1.2]], - ['"text"', ['text']], - [{}, [{}]], + [false, ERROR_BAD_REQUEST], + [true, ERROR_BAD_REQUEST], + [0, ERROR_BAD_REQUEST], + [1, ERROR_BAD_REQUEST], + [-1, ERROR_BAD_REQUEST], + [1.2, ERROR_BAD_REQUEST], + [-1.2, ERROR_BAD_REQUEST], + ['"text"', ERROR_BAD_REQUEST], + [{}, ERROR_BAD_REQUEST], ]); }); }; diff --git a/test/rest-coercion/jsonbody-object.suite.js b/test/rest-coercion/jsonbody-object.suite.js index 5a7af498..99eae27b 100644 --- a/test/rest-coercion/jsonbody-object.suite.js +++ b/test/rest-coercion/jsonbody-object.suite.js @@ -31,7 +31,7 @@ module.exports = function(ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'anyname', type: 'object' }, [ // Empty values - [null, ERROR_BAD_REQUEST], // should be: null + [null, null], // Valid values, arrays are objects too [[]], diff --git a/test/rest-coercion/jsonform-any.suite.js b/test/rest-coercion/jsonform-any.suite.js index 8c043b79..0f495512 100644 --- a/test/rest-coercion/jsonform-any.suite.js +++ b/test/rest-coercion/jsonform-any.suite.js @@ -20,11 +20,11 @@ module.exports = function(ctx) { [{ arg: 1234 }, 1234], [{ arg: 'text' }, 'text'], [{ arg: 'undefined' }, 'undefined'], + [{ arg: 'null' }, 'null'], // Invalid (empty) values should trigger ERROR_BAD_REQUEST [EMPTY_BODY, ERROR_BAD_REQUEST], - [{ arg: '' }, ''], - [{ arg: null }, null], - [{ arg: 'null' }, null], + [{ arg: '' }, ERROR_BAD_REQUEST], + [{ arg: null }, ERROR_BAD_REQUEST], ]); }); @@ -56,20 +56,19 @@ module.exports = function(ctx) { // Verify that deep coercion is not triggered // and types specified in JSON are preserved - [{ arg: 'null' }, null], // should be string 'null' - [{ arg: 'false' }, false], // should be string 'false' - [{ arg: 'true' }, true], // should be string 'true' + [{ arg: 'null' }, 'null'], + [{ arg: 'false' }, 'false'], + [{ arg: 'true' }, 'true'], [{ arg: '0' }, '0'], - [{ arg: '1' }, 1], // should be string '1' + [{ arg: '1' }, '1'], [{ arg: '-1' }, '-1'], - [{ arg: '1.2' }, 1.2], // should be string '1.2' - [{ arg: '-1.2' }, '-1.2'], // should be -1.2 (number) + [{ arg: '1.2' }, '1.2'], + [{ arg: '-1.2' }, '-1.2'], [{ arg: '[]' }, '[]'], [{ arg: '{}' }, '{}'], // Numberic strings larger than MAX_SAFE_INTEGER - // the following should be string '2343546576878989879789' - [{ arg: '2343546576878989879789' }, 2.34354657687899e+21], + [{ arg: '2343546576878989879789' }, '2343546576878989879789'], [{ arg: '-2343546576878989879789' }, '-2343546576878989879789'], // Strings mimicking scientific notation diff --git a/test/rest-coercion/jsonform-array.suite.js b/test/rest-coercion/jsonform-array.suite.js index d1b92cda..44faf9a2 100644 --- a/test/rest-coercion/jsonform-array.suite.js +++ b/test/rest-coercion/jsonform-array.suite.js @@ -25,17 +25,17 @@ module.exports = function(ctx) { [{ arg: [true, false] }, [true, false]], // Empty values should trigger ERROR_BAD_REQUEST - [EMPTY_BODY, []], - [{ arg: null }, [false]], + [EMPTY_BODY, ERROR_BAD_REQUEST], + [{ arg: null }, ERROR_BAD_REQUEST], // Invalid values should trigger ERROR_BAD_REQUEST - [{ arg: [null] }, [false]], - [{ arg: false }, [false]], - [{ arg: true }, [true]], - [{ arg: 0 }, [false]], - [{ arg: 1 }, [true]], - [{ arg: '' }, []], - [{ arg: 'text' }, [true]], + [{ arg: [null] }, ERROR_BAD_REQUEST], + [{ arg: false }, ERROR_BAD_REQUEST], + [{ arg: true }, ERROR_BAD_REQUEST], + [{ arg: 0 }, ERROR_BAD_REQUEST], + [{ arg: 1 }, ERROR_BAD_REQUEST], + [{ arg: '' }, ERROR_BAD_REQUEST], + [{ arg: 'text' }, ERROR_BAD_REQUEST], ]); }); @@ -43,9 +43,9 @@ module.exports = function(ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: ['boolean'] }, [ // Empty values - [EMPTY_BODY, []], // should be: undefined + [EMPTY_BODY, undefined], // should be: undefined [{ arg: [] }, []], - [{ arg: null }, [false]], // should be: null + [{ arg: null }, null], // should be: null // Valid values [{ arg: [false] }, [false]], @@ -53,25 +53,25 @@ module.exports = function(ctx) { [{ arg: [true, false] }, [true, false]], // Value is not an array - should return ERROR_BAD_REQUEST - [{ arg: false }, [false]], - [{ arg: true }, [true]], - [{ arg: 0 }, [false]], - [{ arg: 1 }, [true]], - [{ arg: 2 }, [true]], - [{ arg: -1 }, [true]], - [{ arg: 'text' }, [true]], - [{ arg: {}}, [true]], + [{ arg: false }, ERROR_BAD_REQUEST], + [{ arg: true }, ERROR_BAD_REQUEST], + [{ arg: 0 }, ERROR_BAD_REQUEST], + [{ arg: 1 }, ERROR_BAD_REQUEST], + [{ arg: 2 }, ERROR_BAD_REQUEST], + [{ arg: -1 }, ERROR_BAD_REQUEST], + [{ arg: 'text' }, ERROR_BAD_REQUEST], + [{ arg: {}}, ERROR_BAD_REQUEST], // Array items have wrong type - should return ERROR_BAD_REQUEST - [{ arg: [null] }, [false]], - [{ arg: ['true', 'false'] }, [true, false]], - [{ arg: ['0'] }, [false]], - [{ arg: ['1'] }, [true]], - [{ arg: ['2'] }, [true]], - [{ arg: ['-1'] }, [true]], - [{ arg: ['text'] }, [true]], - [{ arg: [{}] }, [true]], - [{ arg: [[]] }, [true]], + [{ arg: [null] }, ERROR_BAD_REQUEST], + [{ arg: ['true', 'false'] }, ERROR_BAD_REQUEST], + [{ arg: ['0'] }, ERROR_BAD_REQUEST], + [{ arg: ['1'] }, ERROR_BAD_REQUEST], + [{ arg: ['2'] }, ERROR_BAD_REQUEST], + [{ arg: ['-1'] }, ERROR_BAD_REQUEST], + [{ arg: ['text'] }, ERROR_BAD_REQUEST], + [{ arg: [{}] }, ERROR_BAD_REQUEST], + [{ arg: [[]] }, ERROR_BAD_REQUEST], ]); }); @@ -79,9 +79,9 @@ module.exports = function(ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: ['number'] }, [ // Empty values - [EMPTY_BODY, []], // should be: undefined + [EMPTY_BODY, undefined], [{ arg: [] }, []], - [{ arg: null }, [0]], // should be: null + [{ arg: null }, null], // Valid values [{ arg: [0] }, [0]], @@ -97,29 +97,29 @@ module.exports = function(ctx) { [{ arg: [-1.234e+30] }, [-1.234e+30]], // Value is not an array - should return ERROR_BAD_REQUEST - [{ arg: false }, [0]], - [{ arg: true }, [1]], - [{ arg: 0 }, [0]], - [{ arg: '0' }, [0]], - [{ arg: 1 }, [1]], - [{ arg: '1' }, [1]], - [{ arg: -1 }, [-1]], - [{ arg: '-1' }, [-1]], - [{ arg: 1.2 }, [1.2]], - [{ arg: '1.2' }, [1.2]], - [{ arg: -1.2 }, [-1.2]], - [{ arg: '-1.2' }, [-1.2]], + [{ arg: false }, ERROR_BAD_REQUEST], + [{ arg: true }, ERROR_BAD_REQUEST], + [{ arg: 0 }, ERROR_BAD_REQUEST], + [{ arg: '0' }, ERROR_BAD_REQUEST], + [{ arg: 1 }, ERROR_BAD_REQUEST], + [{ arg: '1' }, ERROR_BAD_REQUEST], + [{ arg: -1 }, ERROR_BAD_REQUEST], + [{ arg: '-1' }, ERROR_BAD_REQUEST], + [{ arg: 1.2 }, ERROR_BAD_REQUEST], + [{ arg: '1.2' }, ERROR_BAD_REQUEST], + [{ arg: -1.2 }, ERROR_BAD_REQUEST], + [{ arg: '-1.2' }, ERROR_BAD_REQUEST], [{ arg: 'text' }, ERROR_BAD_REQUEST], [{ arg: {}}, ERROR_BAD_REQUEST], [{ arg: { a: true }}, ERROR_BAD_REQUEST], // Array items have wrong type - should return ERROR_BAD_REQUEST - [{ arg: [null] }, [0]], - [{ arg: ['0'] }, [0]], - [{ arg: ['1'] }, [1]], - [{ arg: ['-1'] }, [-1]], - [{ arg: ['1.2'] }, [1.2]], - [{ arg: ['-1.2'] }, [-1.2]], + [{ arg: [null] }, ERROR_BAD_REQUEST], + [{ arg: ['0'] }, ERROR_BAD_REQUEST], + [{ arg: ['1'] }, ERROR_BAD_REQUEST], + [{ arg: ['-1'] }, ERROR_BAD_REQUEST], + [{ arg: ['1.2'] }, ERROR_BAD_REQUEST], + [{ arg: ['-1.2'] }, ERROR_BAD_REQUEST], [{ arg: ['text'] }, ERROR_BAD_REQUEST], [{ arg: [1, 'text'] }, ERROR_BAD_REQUEST], ]); @@ -129,28 +129,28 @@ module.exports = function(ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: ['string'] }, [ // Empty values - [EMPTY_BODY, []], // should be: undefined + [EMPTY_BODY, undefined], [{ arg: [] }, []], - [{ arg: null }, ['null']], // should be: null + [{ arg: null }, null], // Valid values - [{ arg: 'text' }, ['text']], + [{ arg: ['text'] }, ['text']], [{ arg: ['one', 'two'] }, ['one', 'two']], // Value is not an array - should return ERROR_BAD_REQUEST - [{ arg: false }, ['false']], - [{ arg: true }, ['true']], - [{ arg: 0 }, ['0']], - [{ arg: 1 }, ['1']], - [{ arg: {}}, ['[object Object]']], + [{ arg: false }, ERROR_BAD_REQUEST], + [{ arg: true }, ERROR_BAD_REQUEST], + [{ arg: 0 }, ERROR_BAD_REQUEST], + [{ arg: 1 }, ERROR_BAD_REQUEST], + [{ arg: {}}, ERROR_BAD_REQUEST], // Array items have wrong type - should return ERROR_BAD_REQUEST - [{ arg: [null] }, ['null']], - [{ arg: [0] }, ['0']], - [{ arg: [1] }, ['1']], - [{ arg: [true] }, ['true']], - [{ arg: [{}] }, ['[object Object]']], - [{ arg: [[]] }, ['']], + [{ arg: [null] }, ERROR_BAD_REQUEST], + [{ arg: [0] }, ERROR_BAD_REQUEST], + [{ arg: [1] }, ERROR_BAD_REQUEST], + [{ arg: [true] }, ERROR_BAD_REQUEST], + [{ arg: [{}] }, ERROR_BAD_REQUEST], + [{ arg: [[]] }, ERROR_BAD_REQUEST], ]); }); @@ -158,9 +158,9 @@ module.exports = function(ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: ['date'] }, [ // Empty values - [EMPTY_BODY, []], // should be: undefined + [EMPTY_BODY, undefined], [{ arg: [] }, []], - [{ arg: null }, [new Date(0)]], // should be: null + [{ arg: null }, null], // Valid values - numbers are treated as timestamps [{ arg: [0] }, [new Date(0)]], @@ -179,15 +179,14 @@ module.exports = function(ctx) { ]], // Value is not an array - should return ERROR_BAD_REQUEST - [{ arg: false }, [new Date(0)]], - [{ arg: true }, [new Date(1)]], - [{ arg: 'text' }, [INVALID_DATE]], - [{ arg: '2016-05-19T13:28:51.299Z' }, - [new Date('2016-05-19T13:28:51.299Z')]], + [{ arg: false }, ERROR_BAD_REQUEST], + [{ arg: true }, ERROR_BAD_REQUEST], + [{ arg: 'text' }, ERROR_BAD_REQUEST], + [{ arg: '2016-05-19T13:28:51.299Z' }, ERROR_BAD_REQUEST], // Array items have wrong type - should return ERROR_BAD_REQUEST - [{ arg: [null] }, [new Date(0)]], - [{ arg: ['text'] }, [INVALID_DATE]], + [{ arg: [null] }, ERROR_BAD_REQUEST], + [{ arg: ['text'] }, ERROR_BAD_REQUEST], ]); }); @@ -195,9 +194,9 @@ module.exports = function(ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: ['any'] }, [ // Empty values - [EMPTY_BODY, []], // should be: undefined + [EMPTY_BODY, undefined], [{ arg: [] }, []], - [{ arg: null }, [null]], // should be: null + [{ arg: null }, null], // Valid values - booleans [{ arg: [true, false] }, [true, false]], @@ -239,15 +238,15 @@ module.exports = function(ctx) { [{ arg: ['text', 10, false] }, ['text', 10, false]], // Value is not an array - should return ERROR_BAD_REQUEST - [{ arg: false }, [false]], - [{ arg: true }, [true]], - [{ arg: 0 }, [0]], - [{ arg: 1 }, [1]], - [{ arg: -1 }, [-1]], - [{ arg: 1.2 }, [1.2]], - [{ arg: -1.2 }, [-1.2]], - [{ arg: 'text' }, ['text']], - [{ arg: {}}, [{}]], + [{ arg: false }, ERROR_BAD_REQUEST], + [{ arg: true }, ERROR_BAD_REQUEST], + [{ arg: 0 }, ERROR_BAD_REQUEST], + [{ arg: 1 }, ERROR_BAD_REQUEST], + [{ arg: -1 }, ERROR_BAD_REQUEST], + [{ arg: 1.2 }, ERROR_BAD_REQUEST], + [{ arg: -1.2 }, ERROR_BAD_REQUEST], + [{ arg: 'text' }, ERROR_BAD_REQUEST], + [{ arg: {}}, ERROR_BAD_REQUEST], ]); }); }; diff --git a/test/rest-coercion/jsonform-boolean.suite.js b/test/rest-coercion/jsonform-boolean.suite.js index afb6955d..79c43153 100644 --- a/test/rest-coercion/jsonform-boolean.suite.js +++ b/test/rest-coercion/jsonform-boolean.suite.js @@ -21,9 +21,9 @@ module.exports = function(ctx) { [{ arg: true }, true], // Empty values should trigger ERROR_BAD_REQUEST - [EMPTY_BODY, false], - [{ arg: null }, false], - [{ arg: '' }, false], + [EMPTY_BODY, ERROR_BAD_REQUEST], + [{ arg: null }, ERROR_BAD_REQUEST], + [{ arg: '' }, ERROR_BAD_REQUEST], ]); }); @@ -31,27 +31,27 @@ module.exports = function(ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: 'boolean' }, [ // Empty values - [EMPTY_BODY, false], // should be: undefined - [{ arg: null }, false], // should be: undefined or null + [EMPTY_BODY, undefined], // Valid values [{ arg: false }, false], [{ arg: true }, true], // Invalid values should trigger ERROR_BAD_REQUEST - [{ arg: '' }, false], - [{ arg: 'null' }, false], - [{ arg: 'false' }, false], - [{ arg: 'true' }, true], - [{ arg: 0 }, false], - [{ arg: '0' }, false], - [{ arg: 1 }, true], - [{ arg: '1' }, true], - [{ arg: 'text' }, true], - [{ arg: [] }, true], - [{ arg: [1, 2] }, true], - [{ arg: {}}, true], - [{ arg: { a: true }}, true], + [{ arg: null }, ERROR_BAD_REQUEST], + [{ arg: '' }, ERROR_BAD_REQUEST], + [{ arg: 'null' }, ERROR_BAD_REQUEST], + [{ arg: 'false' }, ERROR_BAD_REQUEST], + [{ arg: 'true' }, ERROR_BAD_REQUEST], + [{ arg: 0 }, ERROR_BAD_REQUEST], + [{ arg: '0' }, ERROR_BAD_REQUEST], + [{ arg: 1 }, ERROR_BAD_REQUEST], + [{ arg: '1' }, ERROR_BAD_REQUEST], + [{ arg: 'text' }, ERROR_BAD_REQUEST], + [{ arg: [] }, ERROR_BAD_REQUEST], + [{ arg: [1, 2] }, ERROR_BAD_REQUEST], + [{ arg: {}}, ERROR_BAD_REQUEST], + [{ arg: { a: true }}, ERROR_BAD_REQUEST], ]); }); }; diff --git a/test/rest-coercion/jsonform-date.suite.js b/test/rest-coercion/jsonform-date.suite.js index df318a0d..38f41e61 100644 --- a/test/rest-coercion/jsonform-date.suite.js +++ b/test/rest-coercion/jsonform-date.suite.js @@ -26,8 +26,8 @@ module.exports = function(ctx) { // Empty values should trigger ERROR_BAD_REQUEST [EMPTY_BODY, ERROR_BAD_REQUEST], - [{ arg: null }, new Date(0)], // should be: ERROR_BAD_REQUEST - [{ arg: '' }, INVALID_DATE], // should be: ERROR_BAD_REQUEST + [{ arg: null }, ERROR_BAD_REQUEST], + [{ arg: '' }, ERROR_BAD_REQUEST], ]); }); @@ -35,8 +35,7 @@ module.exports = function(ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: 'date' }, [ // Empty cases - [EMPTY_BODY, undefined], // should be: undefined - [{ arg: null }, new Date(0)], // should be: null + [EMPTY_BODY, undefined], // Valid values - ISO format [{ arg: '2016-05-19T13:28:51.299Z' }, new Date('2016-05-19T13:28:51.299Z')], @@ -59,21 +58,23 @@ module.exports = function(ctx) { [{ arg: '-1.2' }, new Date('-1.2')], // Invalid values should trigger ERROR_BAD_REQUEST - [{ arg: '' }, INVALID_DATE], // should be: ERROR_BAD_REQUEST - [{ arg: 'null' }, INVALID_DATE], // should be: ERROR_BAD_REQUEST - [{ arg: false }, new Date(0)], // should be: ERROR_BAD_REQUEST - [{ arg: 'false' }, INVALID_DATE], // should be: ERROR_BAD_REQUEST - [{ arg: true }, new Date(1)], // should be: ERROR_BAD_REQUEST - [{ arg: 'true' }, INVALID_DATE], // should be: ERROR_BAD_REQUEST - [{ arg: 'text' }, INVALID_DATE], // should be: ERROR_BAD_REQUEST - [{ arg: [] }, INVALID_DATE], // should be: ERROR_BAD_REQUEST - [{ arg: {}}, INVALID_DATE], // should be: ERROR_BAD_REQUEST + [{ arg: null }, ERROR_BAD_REQUEST], + [{ arg: '' }, ERROR_BAD_REQUEST], + [{ arg: 'null' }, ERROR_BAD_REQUEST], + [{ arg: false }, ERROR_BAD_REQUEST], + [{ arg: 'false' }, ERROR_BAD_REQUEST], + [{ arg: true }, ERROR_BAD_REQUEST], + [{ arg: 'true' }, ERROR_BAD_REQUEST], + [{ arg: 'text' }, ERROR_BAD_REQUEST], + [{ arg: [] }, ERROR_BAD_REQUEST], + [{ arg: {}}, ERROR_BAD_REQUEST], + // Numbers larger than MAX_SAFE_INTEGER - should cause ERROR_BAD_REQUEST - [{ arg: 2343546576878989879789 }, INVALID_DATE], - [{ arg: -2343546576878989879789 }, INVALID_DATE], + [{ arg: 2343546576878989879789 }, ERROR_BAD_REQUEST], + [{ arg: -2343546576878989879789 }, ERROR_BAD_REQUEST], // Scientific notation - should cause ERROR_BAD_REQUEST - [{ arg: 1.234e+30 }, INVALID_DATE], - [{ arg: -1.234e+30 }, INVALID_DATE], + [{ arg: 1.234e+30 }, ERROR_BAD_REQUEST], + [{ arg: -1.234e+30 }, ERROR_BAD_REQUEST], ]); }); }; diff --git a/test/rest-coercion/jsonform-integer.suite.js b/test/rest-coercion/jsonform-integer.suite.js index 3c5dc8b6..e916bd5c 100644 --- a/test/rest-coercion/jsonform-integer.suite.js +++ b/test/rest-coercion/jsonform-integer.suite.js @@ -23,8 +23,8 @@ module.exports = function(ctx) { // Empty values should trigger ERROR_BAD_REQUEST [EMPTY_BODY, ERROR_BAD_REQUEST], - [{ arg: null }, 0], // should be: ERROR_BAD_REQUEST - [{ arg: '' }, 0], // should be: ERROR_BAD_REQUEST + [{ arg: null }, ERROR_BAD_REQUEST], + [{ arg: '' }, ERROR_BAD_REQUEST], ]); }); @@ -33,7 +33,6 @@ module.exports = function(ctx) { verifyTestCases({ arg: 'arg', type: 'integer' }, [ // Empty values [EMPTY_BODY, undefined], - [{ arg: null }, 0], // should be: null // Valid values [{ arg: 0 }, 0], @@ -49,28 +48,27 @@ module.exports = function(ctx) { [{ arg: -1.234e+3 }, -1.234e+3], // Integer-like string values should trigger ERROR_BAD_REQUEST - [{ arg: '0' }, 0], - [{ arg: '1' }, 1], - [{ arg: '-1' }, -1], + [{ arg: '0' }, ERROR_BAD_REQUEST], + [{ arg: '1' }, ERROR_BAD_REQUEST], + [{ arg: '-1' }, ERROR_BAD_REQUEST], + [{ arg: '1.2' }, ERROR_BAD_REQUEST], + [{ arg: '-1.2' }, ERROR_BAD_REQUEST], [{ arg: '2343546576878989879789' }, ERROR_BAD_REQUEST], [{ arg: '-2343546576878989879789' }, ERROR_BAD_REQUEST], [{ arg: '1.234e+30' }, ERROR_BAD_REQUEST], [{ arg: '-1.234e+30' }, ERROR_BAD_REQUEST], - [{ arg: '1.234e+3' }, 1.234e+3], - [{ arg: '-1.234e+3' }, -1.234e+3], // All other non-integer values should trigger ERROR_BAD_REQUEST + [{ arg: null }, ERROR_BAD_REQUEST], [{ arg: 1.2 }, ERROR_BAD_REQUEST], [{ arg: -1.2 }, ERROR_BAD_REQUEST], - [{ arg: '1.2' }, ERROR_BAD_REQUEST], - [{ arg: '-1.2' }, ERROR_BAD_REQUEST], - [{ arg: '' }, 0], - [{ arg: false }, 0], + [{ arg: '' }, ERROR_BAD_REQUEST], + [{ arg: false }, ERROR_BAD_REQUEST], [{ arg: 'false' }, ERROR_BAD_REQUEST], - [{ arg: true }, 1], + [{ arg: true }, ERROR_BAD_REQUEST], [{ arg: 'true' }, ERROR_BAD_REQUEST], [{ arg: 'text' }, ERROR_BAD_REQUEST], - [{ arg: [] }, 0], + [{ arg: [] }, ERROR_BAD_REQUEST], [{ arg: [1, 2] }, ERROR_BAD_REQUEST], [{ arg: {}}, ERROR_BAD_REQUEST], [{ arg: { a: true }}, ERROR_BAD_REQUEST], diff --git a/test/rest-coercion/jsonform-number.suite.js b/test/rest-coercion/jsonform-number.suite.js index e04ff30c..4315b386 100644 --- a/test/rest-coercion/jsonform-number.suite.js +++ b/test/rest-coercion/jsonform-number.suite.js @@ -23,8 +23,8 @@ module.exports = function(ctx) { // Empty values should trigger ERROR_BAD_REQUEST [EMPTY_BODY, ERROR_BAD_REQUEST], - [{ arg: null }, 0], // should be: ERROR_BAD_REQUEST - [{ arg: '' }, 0], // should be: ERROR_BAD_REQUEST + [{ arg: null }, ERROR_BAD_REQUEST], + [{ arg: '' }, ERROR_BAD_REQUEST], ]); }); @@ -33,7 +33,6 @@ module.exports = function(ctx) { verifyTestCases({ arg: 'arg', type: 'number' }, [ // Empty values [EMPTY_BODY, undefined], - [{ arg: null }, 0], // should be: null // Valid values [{ arg: 0 }, 0], @@ -51,24 +50,25 @@ module.exports = function(ctx) { [{ arg: -1.234e+30 }, -1.234e+30], // Number-like string values should trigger ERROR_BAD_REQUEST - [{ arg: '0' }, 0], - [{ arg: '1' }, 1], - [{ arg: '-1' }, -1], - [{ arg: '1.2' }, 1.2], - [{ arg: '-1.2' }, -1.2], - [{ arg: '2343546576878989879789' }, 2.34354657687899e+21], - [{ arg: '-2343546576878989879789' }, -2.34354657687899e+21], - [{ arg: '1.234e+30' }, 1.234e+30], - [{ arg: '-1.234e+30' }, -1.234e+30], + [{ arg: '0' }, ERROR_BAD_REQUEST], + [{ arg: '1' }, ERROR_BAD_REQUEST], + [{ arg: '-1' }, ERROR_BAD_REQUEST], + [{ arg: '1.2' }, ERROR_BAD_REQUEST], + [{ arg: '-1.2' }, ERROR_BAD_REQUEST], + [{ arg: '2343546576878989879789' }, ERROR_BAD_REQUEST], + [{ arg: '-2343546576878989879789' }, ERROR_BAD_REQUEST], + [{ arg: '1.234e+30' }, ERROR_BAD_REQUEST], + [{ arg: '-1.234e+30' }, ERROR_BAD_REQUEST], // All other non-number values should trigger ERROR_BAD_REQUEST - [{ arg: '' }, 0], - [{ arg: false }, 0], + [{ arg: null }, ERROR_BAD_REQUEST], + [{ arg: '' }, ERROR_BAD_REQUEST], + [{ arg: false }, ERROR_BAD_REQUEST], [{ arg: 'false' }, ERROR_BAD_REQUEST], - [{ arg: true }, 1], + [{ arg: true }, ERROR_BAD_REQUEST], [{ arg: 'true' }, ERROR_BAD_REQUEST], [{ arg: 'text' }, ERROR_BAD_REQUEST], - [{ arg: [] }, 0], + [{ arg: [] }, ERROR_BAD_REQUEST], [{ arg: [1, 2] }, ERROR_BAD_REQUEST], [{ arg: {}}, ERROR_BAD_REQUEST], [{ arg: { a: true }}, ERROR_BAD_REQUEST], diff --git a/test/rest-coercion/jsonform-object.suite.js b/test/rest-coercion/jsonform-object.suite.js index 93697de8..fd798ed4 100644 --- a/test/rest-coercion/jsonform-object.suite.js +++ b/test/rest-coercion/jsonform-object.suite.js @@ -35,7 +35,7 @@ module.exports = function(ctx) { verifyTestCases({ arg: 'arg', type: 'object' }, [ // Empty values [EMPTY_BODY, undefined], - [{ arg: null }, ERROR_BAD_REQUEST], // should be null + [{ arg: null }, null], // Valid values [{ arg: { x: null }}, { x: null }], diff --git a/test/rest-coercion/jsonform-string.suite.js b/test/rest-coercion/jsonform-string.suite.js index 1011b657..5c907c6d 100644 --- a/test/rest-coercion/jsonform-string.suite.js +++ b/test/rest-coercion/jsonform-string.suite.js @@ -22,8 +22,8 @@ module.exports = function(ctx) { // Empty values should trigger ERROR_BAD_REQUEST [EMPTY_BODY, ERROR_BAD_REQUEST], - [{ arg: '' }, ''], - [{ arg: null }, 'null'], + [{ arg: '' }, ERROR_BAD_REQUEST], + [{ arg: null }, ERROR_BAD_REQUEST], ]); }); @@ -33,7 +33,6 @@ module.exports = function(ctx) { // Empty values [EMPTY_BODY, undefined], [{ arg: '' }, ''], - [{ arg: null }, 'null'], // should be: null // Valid values [{ arg: 'text' }, 'text'], @@ -51,15 +50,16 @@ module.exports = function(ctx) { [{ arg: 'true' }, 'true'], // Invalid values should trigger ERROR_BAD_REQUEST - [{ arg: false }, 'false'], - [{ arg: true }, 'true'], - [{ arg: 0 }, '0'], - [{ arg: 1 }, '1'], - [{ arg: -1 }, '-1'], - [{ arg: 1.2 }, '1.2'], - [{ arg: -1.2 }, '-1.2'], - [{ arg: [] }, ''], - [{ arg: {}}, '[object Object]'], + [{ arg: null }, ERROR_BAD_REQUEST], + [{ arg: false }, ERROR_BAD_REQUEST], + [{ arg: true }, ERROR_BAD_REQUEST], + [{ arg: 0 }, ERROR_BAD_REQUEST], + [{ arg: 1 }, ERROR_BAD_REQUEST], + [{ arg: -1 }, ERROR_BAD_REQUEST], + [{ arg: 1.2 }, ERROR_BAD_REQUEST], + [{ arg: -1.2 }, ERROR_BAD_REQUEST], + [{ arg: [] }, ERROR_BAD_REQUEST], + [{ arg: {}}, ERROR_BAD_REQUEST], ]); }); }; diff --git a/test/rest-coercion/urlencoded-any.suite.js b/test/rest-coercion/urlencoded-any.suite.js index 1d466f0a..1657fd32 100644 --- a/test/rest-coercion/urlencoded-any.suite.js +++ b/test/rest-coercion/urlencoded-any.suite.js @@ -26,9 +26,9 @@ function suite(prefix, ctx) { // Empty values should trigger ERROR_BAD_REQUEST [EMPTY_QUERY, ERROR_BAD_REQUEST], - ['arg', ''], // should be: ERROR_BAD_REQUEST - ['arg=', ''], // should be: ERROR_BAD_REQUEST - ['arg=null', null], // should be: ERROR_BAD_REQUEST + ['arg', ERROR_BAD_REQUEST], + ['arg=', ERROR_BAD_REQUEST], + ['arg=null', ERROR_BAD_REQUEST], ]); }); @@ -37,30 +37,30 @@ function suite(prefix, ctx) { verifyTestCases({ arg: 'arg', type: 'any' }, [ // Empty values [EMPTY_QUERY, undefined], - ['arg', ''], // should be: undefined - ['arg=', ''], // should be: undefined + ['arg', undefined], + ['arg=', undefined], ['arg=null', null], // should be: 'null' // Valid values (coerced) ['arg=undefined', 'undefined'], // 'undefined' is treated as a string ['arg=false', false], ['arg=true', true], - ['arg=0', '0'], // should be 0 (number) + ['arg=0', 0], ['arg=1', 1], - ['arg=-1', '-1'], // should be -1 (number) + ['arg=-1', -1], ['arg=1.2', 1.2], - ['arg=-1.2', '-1.2'], // should be -1.2 (number) + ['arg=-1.2', -1.2], ['arg=text', 'text'], - ['arg=[]', '[]'], // should be an empty array - ['arg={}', '{}'], // should be an empty object - ['arg={x:1}', '{x:1}'], // should be parsed as an object {x:1} - ['arg={x:"1"}', '{x:"1"}'], // should be parsed as an object {x:'1'} - ['arg=[1]', '[1]'], // should be parsed as an array [1] - ['arg=["1"]', '["1"]'], // should be parsed as an array ['1'] + ['arg=[]', []], + ['arg={}', {}], + ['arg={"x":1}', { x: 1 }], + ['arg={"x":"1"}', { x: '1' }], + ['arg={x:1}', '{x:1}'], // invalid JSON - the key is not quoted + ['arg=[1]', [1]], + ['arg=["1"]', ['1']], - // Numbers larger than MAX_SAFE_INTEGER - ['arg=2343546576878989879789', 2.34354657687899e+21], - // This should have been recognized as number + // Numbers larger than MAX_SAFE_INTEGER are treated as strings + ['arg=2343546576878989879789', '2343546576878989879789'], ['arg=-2343546576878989879789', '-2343546576878989879789'], // Scientific notation should be recognized as a number ['arg=1.234e%2B30', '1.234e+30'], diff --git a/test/rest-coercion/urlencoded-array.suite.js b/test/rest-coercion/urlencoded-array.suite.js index 5cc6a0d9..f760cfd5 100644 --- a/test/rest-coercion/urlencoded-array.suite.js +++ b/test/rest-coercion/urlencoded-array.suite.js @@ -31,22 +31,22 @@ function suite(prefix, ctx) { // Valid values - nested keys ['arg=false', [false]], ['arg=true', [true]], + ['arg=0', [false]], + ['arg=1', [true]], ['arg=true&arg=false', [true, false]], // Empty values should trigger ERROR_BAD_REQUEST - [EMPTY_QUERY, []], // should be: ERROR_BAD_REQUEST - ['arg', []], // should be: ERROR_BAD_REQUEST - ['arg=', []], // should be: ERROR_BAD_REQUEST + [EMPTY_QUERY, ERROR_BAD_REQUEST], + ['arg', ERROR_BAD_REQUEST], + ['arg=', ERROR_BAD_REQUEST], // Invalid values - array items have wrong type or value is not an array // All test cases should trigger ERROR_BAD_REQUEST - ['arg=null', [false]], - ['arg=undefined', [false]], - ['arg=0', [false]], - ['arg=1', [true]], - ['arg={}', [true]], - ['arg=["true"]', [true]], - ['arg=[1]', [true]], + ['arg=null', ERROR_BAD_REQUEST], + ['arg=undefined', ERROR_BAD_REQUEST], + ['arg={}', ERROR_BAD_REQUEST], + ['arg=["true"]', ERROR_BAD_REQUEST], + ['arg=[1]', ERROR_BAD_REQUEST], ]); }); @@ -54,14 +54,16 @@ function suite(prefix, ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: ['boolean'] }, [ // Empty values - [EMPTY_QUERY, []], // should be: undefined - ['arg', []], // should be: undefined - ['arg=', []], // should be: undefined - ['arg=null', [false]], // should be: null + [EMPTY_QUERY, undefined], + ['arg', undefined], + ['arg=', undefined], + ['arg=null', null], // Valid values - repeated keys ['arg=false', [false]], ['arg=true', [true]], + ['arg=0', [false]], + ['arg=1', [true]], ['arg=true&arg=false', [true, false]], // Valid values - JSON encoding @@ -69,22 +71,20 @@ function suite(prefix, ctx) { ['arg=[true,false]', [true, false]], // Invalid values should trigger ERROR_BAD_REQUEST - ['arg=undefined', [false]], - ['arg=true&arg=text', [true, true]], - ['arg=0', [false]], - ['arg=1', [true]], - ['arg=2', [true]], - ['arg=-1', [true]], - ['arg=text', [true]], - ['arg=[1,2]', [true, true]], - ['arg=["text"]', [true]], - ['arg=[null]', [false]], - ['arg={}', [true]], - ['arg={"a":true}', [true]], + ['arg=undefined', ERROR_BAD_REQUEST], + ['arg=true&arg=text', ERROR_BAD_REQUEST], + ['arg=2', ERROR_BAD_REQUEST], + ['arg=-1', ERROR_BAD_REQUEST], + ['arg=text', ERROR_BAD_REQUEST], + ['arg=[1,2]', ERROR_BAD_REQUEST], + ['arg=["text"]', ERROR_BAD_REQUEST], + ['arg=[null]', ERROR_BAD_REQUEST], + ['arg={}', ERROR_BAD_REQUEST], + ['arg={"a":true}', ERROR_BAD_REQUEST], // Malformed JSON should trigger ERROR_BAD_REQUEST - ['arg={malformed}', [true]], - ['arg=[malformed]', [true]], + ['arg={malformed}', ERROR_BAD_REQUEST], + ['arg=[malformed]', ERROR_BAD_REQUEST], ]); }); @@ -92,9 +92,10 @@ function suite(prefix, ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: ['number'] }, [ // Empty values - [EMPTY_QUERY, []], // should be: undefined - ['arg', []], // should be: undefined - ['arg=', []], // should be: undefined + [EMPTY_QUERY, undefined], + ['arg', undefined], + ['arg=', undefined], + ['arg=null', null], // Valid values - repeated keys ['arg=0', [0]], @@ -116,14 +117,13 @@ function suite(prefix, ctx) { // Invalid values should trigger ERROR_BAD_REQUEST ['arg=undefined', ERROR_BAD_REQUEST], - ['arg=null', ERROR_BAD_REQUEST], ['arg=true', ERROR_BAD_REQUEST], ['arg=false', ERROR_BAD_REQUEST], ['arg=text', ERROR_BAD_REQUEST], ['arg=1&arg=text', ERROR_BAD_REQUEST], - ['arg=["1"]', [1]], // notice the item is a string, we should not coerce + ['arg=["1"]', ERROR_BAD_REQUEST], // notice the item is a string ['arg=[1,"text"]', ERROR_BAD_REQUEST], - ['arg=[null]', [0]], + ['arg=[null]', ERROR_BAD_REQUEST], ['arg={}', ERROR_BAD_REQUEST], ['arg={"a":true}', ERROR_BAD_REQUEST], @@ -137,13 +137,13 @@ function suite(prefix, ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: ['string'] }, [ // Empty values - [EMPTY_QUERY, []], // should be: undefined - ['arg', []], // should be: undefined - ['arg=', []], // should be: undefined + [EMPTY_QUERY, undefined], + ['arg', undefined], + ['arg=', undefined], + ['arg=null', null], // Valid values - repeated keys ['arg=undefined', ['undefined']], - ['arg=null', ['null']], ['arg=0', ['0']], ['arg=1', ['1']], ['arg=false', ['false']], @@ -156,18 +156,21 @@ function suite(prefix, ctx) { // Valid values - JSON encoding ['arg=[]', []], - ['arg=[1,2]', ['1', '2']], + ['arg=["1","2"]', ['1', '2']], - // Invalid values should trigger ERROR_BAD_REQUEST + // Valid values - array arguments don't recognize object value + // and treat them as single-item string ['arg={}', ['{}']], ['arg={"a":true}', ['{"a":true}']], - ['arg=[1]', ['1']], - ['arg=[true]', ['true']], - ['arg=[null]', ['null']], + ['arg={malformed}', ['{malformed}']], + // Invalid values should trigger ERROR_BAD_REQUEST + ['arg=[1]', ERROR_BAD_REQUEST], + ['arg=[1,2]', ERROR_BAD_REQUEST], + ['arg=[true]', ERROR_BAD_REQUEST], + ['arg=[null]', ERROR_BAD_REQUEST], // Malformed JSON should trigger ERROR_BAD_REQUEST - ['arg={malformed}', ['{malformed}']], - ['arg=[malformed]', ['[malformed]']], + ['arg=[malformed]', ERROR_BAD_REQUEST], ]); }); @@ -175,14 +178,14 @@ function suite(prefix, ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: ['date'] }, [ // Empty values - [EMPTY_QUERY, []], // should be: undefined - ['arg', []], // should be: undefined - ['arg=', []], // should be: undefined + [EMPTY_QUERY, undefined], + ['arg', undefined], + ['arg=', undefined], + ['arg=null', null], // Valid values - repeated keys ['arg=0', [new Date('0')]], // 1999-12-31T23:00:00.000Z in CEST ['arg=1', [new Date('1')]], // 2000-12-31T23:00:00.000Z - ['arg=text', [INVALID_DATE]], // should be: ERROR_BAD_REQUEST ['arg=2016-05-19T13:28:51.299Z', [new Date('2016-05-19T13:28:51.299Z')]], ['arg=2016-05-19T13:28:51.299Z&arg=2016-05-20T08:27:28.539Z', [ @@ -203,20 +206,20 @@ function suite(prefix, ctx) { ]], // Invalid values should trigger ERROR_BAD_REQUEST - ['arg=undefined', [INVALID_DATE]], // should be: ERROR_BAD_REQUEST - ['arg=null', [INVALID_DATE]], // should be: ERROR_BAD_REQUEST - ['arg=false', [INVALID_DATE]], // should be: ERROR_BAD_REQUEST - ['arg=true', [INVALID_DATE]], // should be: ERROR_BAD_REQUEST - ['arg={}', [INVALID_DATE]], // should be: ERROR_BAD_REQUEST - ['arg={"a":true}', [INVALID_DATE]], - ['arg=[null]', [new Date(0)]], - ['arg=[false]', [new Date(0)]], - ['arg=[true]', [new Date(1)]], - ['arg=["text"]', [INVALID_DATE]], + ['arg=undefined', ERROR_BAD_REQUEST], + ['arg=false', ERROR_BAD_REQUEST], + ['arg=true', ERROR_BAD_REQUEST], + ['arg=text', ERROR_BAD_REQUEST], + ['arg={}', ERROR_BAD_REQUEST], + ['arg={"a":true}', ERROR_BAD_REQUEST], + ['arg=[null]', ERROR_BAD_REQUEST], + ['arg=[false]', ERROR_BAD_REQUEST], + ['arg=[true]', ERROR_BAD_REQUEST], + ['arg=["text"]', ERROR_BAD_REQUEST], // Malformed JSON should trigger ERROR_BAD_REQUEST - ['arg={malformed}', [INVALID_DATE]], // should be: ERROR_BAD_REQUEST - ['arg=[malformed]', [INVALID_DATE]], // should be: ERROR_BAD_REQUEST + ['arg={malformed}', ERROR_BAD_REQUEST], + ['arg=[malformed]', ERROR_BAD_REQUEST], ]); }); @@ -224,25 +227,24 @@ function suite(prefix, ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: ['any'] }, [ // Empty values - [EMPTY_QUERY, []], // should be: undefined - ['arg', []], // should be: undefined - ['arg=', []], // should be: undefined - ['arg=null', [null]], // should be: null (?) + [EMPTY_QUERY, undefined], + ['arg', undefined], + ['arg=', undefined], + ['arg=null', null], // Valid values - repeated keys ['arg=undefined', ['undefined']], ['arg=false', [false]], ['arg=true', [true]], - ['arg=0', ['0']], // should be 0 (number) + ['arg=0', [0]], ['arg=1', [1]], - ['arg=-1', ['-1']], // should be -1 (number) + ['arg=-1', [-1]], ['arg=1.2', [1.2]], - ['arg=-1.2', ['-1.2']], // should be -1.2 (number) + ['arg=-1.2', [-1.2]], ['arg=text', ['text']], - ['arg=text&arg=10&arg=false', ['text', '10', 'false']], // should be coerced + ['arg=text&arg=10&arg=false', ['text', 10, false]], // Numbers larger than MAX_SAFE_INTEGER - ['arg=2343546576878989879789', [2.34354657687899e+21]], - // this should have been recognized as number + ['arg=2343546576878989879789', ['2343546576878989879789']], ['arg=-2343546576878989879789', ['-2343546576878989879789']], // Scientific notation - should it be recognized as a number? ['arg=1.234e%2B30&arg=-1.234e%2B30', ['1.234e+30', '-1.234e+30']], @@ -251,13 +253,14 @@ function suite(prefix, ctx) { ['arg=[]', []], ['arg=["text",10,false]', ['text', 10, false]], - // Invalid values should trigger ERROR_BAD_REQUEST - ['arg={}', ['{}']], - ['arg={"foo":"bar"}', ['{"foo":"bar"}']], + // Valid values - items are objects + ['arg={}', [{}]], + ['arg={"foo":"bar"}', [{ 'foo': 'bar' }]], + // Item is not a valid JSON object - it will be treated as a string + ['arg={malformed}', ['{malformed}']], // Malformed JSON should trigger ERROR_BAD_REQUEST - ['arg={malformed}', ['{malformed}']], - ['arg=[malformed]', ['[malformed]']], + ['arg=[malformed]', ERROR_BAD_REQUEST], ]); }); } diff --git a/test/rest-coercion/urlencoded-boolean.suite.js b/test/rest-coercion/urlencoded-boolean.suite.js index 5270895a..9667498c 100644 --- a/test/rest-coercion/urlencoded-boolean.suite.js +++ b/test/rest-coercion/urlencoded-boolean.suite.js @@ -23,16 +23,16 @@ function suite(prefix, ctx) { // Valid values ['arg=false', false], ['arg=true', true], + ['arg=0', false], // Empty values should trigger ERROR_BAD_REQUEST - [EMPTY_QUERY, false], - ['arg', false], - ['arg=', false], + [EMPTY_QUERY, ERROR_BAD_REQUEST], + ['arg', ERROR_BAD_REQUEST], + ['arg=', ERROR_BAD_REQUEST], // Empty-like values should trigger ERROR_BAD_REQUEST too - ['arg=undefined', false], - ['arg=null', false], - ['arg=0', false], + ['arg=undefined', ERROR_BAD_REQUEST], + ['arg=null', ERROR_BAD_REQUEST], ]); }); @@ -40,29 +40,30 @@ function suite(prefix, ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: 'boolean' }, [ // Empty values - [EMPTY_QUERY, false], // should be: undefined - ['arg', false], // should be: undefined - ['arg=', false], // should be: undefined + [EMPTY_QUERY, undefined], + ['arg', undefined], + ['arg=', undefined], // Valid values ['arg=false', false], ['arg=true', true], + ['arg=0', false], + ['arg=1', true], // values are case insensitive - ['arg=FalsE', true], // should be false + ['arg=FalsE', false], ['arg=TruE', true], - ['arg=FALSE', true], // should be false + ['arg=FALSE', false], ['arg=TRUE', true], // Invalid values should trigger ERROR_BAD_REQUEST - ['arg=undefined', false], - ['arg=null', false], - ['arg=0', false], - ['arg=1', true], - ['arg=text', true], - ['arg=[]', true], - ['arg=[1,2]', true], - ['arg={}', true], - ['arg={"a":true}', true], + ['arg=undefined', ERROR_BAD_REQUEST], + ['arg=null', ERROR_BAD_REQUEST], + ['arg=2', ERROR_BAD_REQUEST], + ['arg=text', ERROR_BAD_REQUEST], + ['arg=[]', ERROR_BAD_REQUEST], + ['arg=[1,2]', ERROR_BAD_REQUEST], + ['arg={}', ERROR_BAD_REQUEST], + ['arg={"a":true}', ERROR_BAD_REQUEST], ]); }); } diff --git a/test/rest-coercion/urlencoded-date.suite.js b/test/rest-coercion/urlencoded-date.suite.js index 6839c773..097c2e85 100644 --- a/test/rest-coercion/urlencoded-date.suite.js +++ b/test/rest-coercion/urlencoded-date.suite.js @@ -27,12 +27,12 @@ function suite(prefix, ctx) { // Empty values should trigger ERROR_BAD_REQUEST [EMPTY_QUERY, ERROR_BAD_REQUEST], - ['arg', INVALID_DATE], - ['arg=', INVALID_DATE], + ['arg', ERROR_BAD_REQUEST], + ['arg=', ERROR_BAD_REQUEST], // Empty-like values should trigger ERROR_BAD_REQUEST too - ['arg=undefined', INVALID_DATE], - ['arg=null', INVALID_DATE], + ['arg=undefined', ERROR_BAD_REQUEST], + ['arg=null', ERROR_BAD_REQUEST], ]); }); @@ -41,8 +41,8 @@ function suite(prefix, ctx) { verifyTestCases({ arg: 'arg', type: 'date' }, [ // Empty values [EMPTY_QUERY, undefined], - ['arg', INVALID_DATE], // should be: undefined - ['arg=', INVALID_DATE], // should be: undefined + ['arg', undefined], + ['arg=', undefined], // Valid values - ISO format ['arg=2016-05-19T13:28:51.299Z', new Date('2016-05-19T13:28:51.299Z')], @@ -66,19 +66,19 @@ function suite(prefix, ctx) { ['arg=-1.2', new Date('-1.2')], // 2001-01-01T23:00:00.000Z // Invalid values should trigger ERROR_BAD_REQUEST - ['arg=undefined', INVALID_DATE], - ['arg=null', INVALID_DATE], - ['arg=false', INVALID_DATE], - ['arg=true', INVALID_DATE], - ['arg=text', INVALID_DATE], - ['arg=[]', INVALID_DATE], - ['arg={}', INVALID_DATE], + ['arg=null', ERROR_BAD_REQUEST], + ['arg=undefined', ERROR_BAD_REQUEST], + ['arg=false', ERROR_BAD_REQUEST], + ['arg=true', ERROR_BAD_REQUEST], + ['arg=text', ERROR_BAD_REQUEST], + ['arg=[]', ERROR_BAD_REQUEST], + ['arg={}', ERROR_BAD_REQUEST], // Numbers larger than MAX_SAFE_INTEGER - should cause ERROR_BAD_REQUEST - ['arg=2343546576878989879789', INVALID_DATE], - ['arg=-2343546576878989879789', INVALID_DATE], + ['arg=2343546576878989879789', ERROR_BAD_REQUEST], + ['arg=-2343546576878989879789', ERROR_BAD_REQUEST], // Scientific notation - should cause ERROR_BAD_REQUEST - ['arg=1.234e%2B30', INVALID_DATE], - ['arg=-1.234e%2B30', INVALID_DATE], + ['arg=1.234e%2B30', ERROR_BAD_REQUEST], + ['arg=-1.234e%2B30', ERROR_BAD_REQUEST], ]); }); } diff --git a/test/rest-coercion/urlencoded-integer.suite.js b/test/rest-coercion/urlencoded-integer.suite.js index 2295a0a3..5bf58d21 100644 --- a/test/rest-coercion/urlencoded-integer.suite.js +++ b/test/rest-coercion/urlencoded-integer.suite.js @@ -27,8 +27,8 @@ function suite(prefix, ctx) { // Empty values should trigger ERROR_BAD_REQUEST [EMPTY_QUERY, ERROR_BAD_REQUEST], - ['arg', 0], - ['arg=', 0], + ['arg', ERROR_BAD_REQUEST], + ['arg=', ERROR_BAD_REQUEST], // Empty-like values should trigger ERROR_BAD_REQUEST too ['arg=undefined', ERROR_BAD_REQUEST], @@ -41,8 +41,8 @@ function suite(prefix, ctx) { verifyTestCases({ arg: 'arg', type: 'integer' }, [ // Empty values [EMPTY_QUERY, undefined], - ['arg', 0], // should be: undefined - ['arg=', 0], // should be: undefined + ['arg', undefined], + ['arg=', undefined], // Valid values ['arg=0', 0], diff --git a/test/rest-coercion/urlencoded-number.suite.js b/test/rest-coercion/urlencoded-number.suite.js index beddeaae..31e59096 100644 --- a/test/rest-coercion/urlencoded-number.suite.js +++ b/test/rest-coercion/urlencoded-number.suite.js @@ -27,8 +27,8 @@ function suite(prefix, ctx) { // Empty values should trigger ERROR_BAD_REQUEST [EMPTY_QUERY, ERROR_BAD_REQUEST], - ['arg', 0], - ['arg=', 0], + ['arg', ERROR_BAD_REQUEST], + ['arg=', ERROR_BAD_REQUEST], // Empty-like values should trigger ERROR_BAD_REQUEST too ['arg=undefined', ERROR_BAD_REQUEST], @@ -41,8 +41,8 @@ function suite(prefix, ctx) { verifyTestCases({ arg: 'arg', type: 'number' }, [ // Empty values [EMPTY_QUERY, undefined], - ['arg', 0], // should be: undefined - ['arg=', 0], // should be: undefined + ['arg', undefined], + ['arg=', undefined], // Valid values ['arg=0', 0], diff --git a/test/rest-coercion/urlencoded-object.suite.js b/test/rest-coercion/urlencoded-object.suite.js index aab64dd4..02abcc9c 100644 --- a/test/rest-coercion/urlencoded-object.suite.js +++ b/test/rest-coercion/urlencoded-object.suite.js @@ -46,10 +46,10 @@ function suite(prefix, ctx) { // See verifyTestCases' jsdoc for details about the format of test cases. verifyTestCases({ arg: 'arg', type: 'object' }, [ // Empty values - [EMPTY_QUERY, undefined], // should be: undefined - ['arg', ERROR_BAD_REQUEST], // should be undefined - ['arg=', ERROR_BAD_REQUEST], // should be undefined - ['arg=null', ERROR_BAD_REQUEST], // should be null + [EMPTY_QUERY, undefined], + ['arg', undefined], + ['arg=', undefined], + ['arg=null', null], ['arg={}', {}], ['arg=[]', []], diff --git a/test/rest-coercion/urlencoded-string.suite.js b/test/rest-coercion/urlencoded-string.suite.js index c6bf1f54..73e6f5bd 100644 --- a/test/rest-coercion/urlencoded-string.suite.js +++ b/test/rest-coercion/urlencoded-string.suite.js @@ -28,8 +28,8 @@ function suite(prefix, ctx) { // Empty values should trigger ERROR_BAD_REQUEST [EMPTY_QUERY, ERROR_BAD_REQUEST], - ['arg', ''], - ['arg=', ''], + ['arg', ERROR_BAD_REQUEST], + ['arg=', ERROR_BAD_REQUEST], ]); }); @@ -38,8 +38,8 @@ function suite(prefix, ctx) { verifyTestCases({ arg: 'arg', type: 'string' }, [ // Empty values [EMPTY_QUERY, undefined], - ['arg', ''], // should be: undefined - ['arg=', ''], + ['arg', undefined], + ['arg=', undefined], // Valid values - all non-empty value are valid strings ['arg=undefined', 'undefined'], diff --git a/test/rest.browser.test.js b/test/rest.browser.test.js index 06e3b818..dca071c4 100644 --- a/test/rest.browser.test.js +++ b/test/rest.browser.test.js @@ -252,29 +252,50 @@ describe('strong-remoting-rest', function() { it('should allow and return falsy required arguments of correct type', function(done) { var method = givenSharedStaticMethod( - function bar(num, str, bool, cb) { - cb(null, num, str, bool); + function bar(num, bool, cb) { + cb(null, num, bool); }, { accepts: [ { arg: 'num', type: 'number', required: true }, - { arg: 'str', type: 'string', required: true }, { arg: 'bool', type: 'boolean', required: true }, ], returns: [ { arg: 'num', type: 'number' }, - { arg: 'str', type: 'string' }, { arg: 'bool', type: 'boolean' }, ], http: { path: '/' }, } ); - objects.invoke(method.name, [0, '', false], function(err, a, b, c) { - expect(err).to.not.be.an.instanceof(Error); + objects.invoke(method.name, [0, false], function(err, a, b) { + if (err) return done(err); assert.equal(a, 0); - assert.equal(b, ''); - assert.equal(c, false); + assert.equal(b, false); + done(); + }); + } + ); + + it('should reject empty string when string required', + function(done) { + var method = givenSharedStaticMethod( + function bar(str, cb) { + cb(null, str); + }, + { + accepts: [ + { arg: 'str', type: 'string', required: true }, + ], + returns: [ + { arg: 'str', type: 'string' }, + ], + http: { path: '/' }, + } + ); + + objects.invoke(method.name, [''], function(err, a, b, c) { + expect(err).to.be.an.instanceof(Error); done(); }); } diff --git a/test/rest.test.js b/test/rest.test.js index e1f7eec9..631a487b 100644 --- a/test/rest.test.js +++ b/test/rest.test.js @@ -46,8 +46,11 @@ describe('strong-remoting-rest', function() { // setup beforeEach(function() { - objects = RemoteObjects.create({ json: { limit: '1kb' }, - errorHandler: { debug: true, log: false }}); + objects = RemoteObjects.create({ + json: { limit: '1kb' }, + errorHandler: { debug: true, log: false }, + types: { warnOnUnknownType: false }, + }); remotes = objects.exports; // connect to the app @@ -560,7 +563,7 @@ describe('strong-remoting-rest', function() { accepts: [ { arg: 'b', type: 'number' }, { arg: 'a', type: 'number', http: function(ctx) { - return ctx.req.query.a; + return +ctx.req.query.a; } }, ], returns: { arg: 'n', type: 'number' }, @@ -866,7 +869,7 @@ describe('strong-remoting-rest', function() { .expect(400, done); }); - it('should coerce boolean strings - true', function(done) { + it('should not coerce nested boolean strings - true', function(done) { remotes.foo = { bar: function(a, fn) { fn(null, a); @@ -882,10 +885,10 @@ describe('strong-remoting-rest', function() { fn.returns = { root: true }; json('get', '/foo/bar?a[foo]=true') - .expect({ foo: true }, done); + .expect({ foo: 'true' }, done); }); - it('should coerce boolean strings - false', function(done) { + it('should not coerce nested boolean strings - false', function(done) { remotes.foo = { bar: function(a, fn) { fn(null, a); @@ -901,7 +904,7 @@ describe('strong-remoting-rest', function() { fn.returns = { root: true }; json('get', '/foo/bar?a[foo]=false') - .expect({ foo: false }, done); + .expect({ foo: 'false' }, done); }); it('should coerce number strings', function(done) { @@ -1048,7 +1051,7 @@ describe('strong-remoting-rest', function() { ]; fn.returns = { root: true }; - json('get', '/foo/bar?a=["1","2","3","4","5"]') + json('get', '/foo/bar?a=[1,2,3,4,5]') .expect(200, function(err, res) { assert.equal(res.body, 15); done(); @@ -1192,7 +1195,7 @@ describe('strong-remoting-rest', function() { }); json('post', method.url + '?a=') - .expect({ data: [] }, done); + .expect({ /* data is undefined */ }, done); }); it('should still support JSON arrays with arrayItemDelimiters', function(done) { @@ -1204,7 +1207,7 @@ describe('strong-remoting-rest', function() { returns: { arg: 'data', type: 'object' }, }); - json('post', method.url + '?a=["1","2","3"]') + json('post', method.url + '?a=[1,2,3]') .expect({ data: [1, 2, 3] }, done); }); @@ -2726,7 +2729,7 @@ describe('strong-remoting-rest', function() { { arg: 'b', type: 'number' }, ], returns: [ - { arg: 'id', type: 'string' }, + { arg: 'id', type: 'any' }, { arg: 'a', type: 'number' }, { arg: 'b', type: 'number' }, ], diff --git a/test/shared-method.test.js b/test/shared-method.test.js index 8c187304..a3cde582 100644 --- a/test/shared-method.test.js +++ b/test/shared-method.test.js @@ -6,7 +6,9 @@ var assert = require('assert'); var extend = require('util')._extend; var expect = require('chai').expect; +var Context = require('../lib/context-base'); var SharedMethod = require('../lib/shared-method'); +var TypeRegistry = require('../lib/type-registry'); var factory = require('./helpers/shared-objects-factory.js'); var Promise = global.Promise || require('bluebird'); @@ -136,10 +138,10 @@ describe('SharedMethod', function() { accepts: { arg: 'num', type: 'number' }, }); - method.invoke('ctx', { num: NaN }, function(err) { + method.invoke('ctx', { num: NaN }, {}, ctx(method), function(err) { setImmediate(function() { expect(err).to.exist; - expect(err.message).to.contain('num must be a number'); + expect(err.message).to.contain('not a number'); expect(err.statusCode).to.equal(400); done(); }); @@ -163,10 +165,10 @@ describe('SharedMethod', function() { accepts: { arg: 'num', type: 'integer' }, }); - method.invoke('ctx', { num: 2.5 }, function(err) { + method.invoke('ctx', { num: 2.5 }, {}, ctx(method), function(err) { setImmediate(function() { expect(err).to.exist; - expect(err.message).to.match(/integer/i); + expect(err.message).to.match(/not a safe integer/); expect(err.statusCode).to.equal(400); done(); }); @@ -178,10 +180,10 @@ describe('SharedMethod', function() { accepts: { arg: 'num', type: 'integer' }, }); - method.invoke('ctx', { num: NaN }, function(err) { + method.invoke('ctx', { num: NaN }, {}, ctx(method), function(err) { setImmediate(function() { expect(err).to.exist; - expect(err.message).to.match(/integer/i); + expect(err.message).to.match(/not a number/i); expect(err.statusCode).to.equal(400); done(); }); @@ -198,7 +200,8 @@ describe('SharedMethod', function() { accepts: { arg: 'num', type: 'integer' }, }); - method.invoke('ctx', { num: 2343546576878989879789 }, function(err) { + method.invoke('ctx', { num: 2343546576878989879789 }, {}, ctx(method), + function(err) { setImmediate(function() { expect(err).to.exist; expect(err.message).to.match(/integer/i); @@ -217,7 +220,7 @@ describe('SharedMethod', function() { accepts: { arg: 'num', type: 'integer' }, }); - method.invoke('ctx', { num: '12.0' }, function(result) { + method.invoke('ctx', { num: 12.0 }, {}, ctx(method), function(result) { setImmediate(function() { expect(result.num).to.equal(12); done(); @@ -235,7 +238,7 @@ describe('SharedMethod', function() { returns: { arg: 'value', type: 'integer' }, }); - method.invoke('ctx', {}, function(err, result) { + method.invoke('ctx', {}, {}, ctx(method), function(err, result) { setImmediate(function() { expect(err).to.exist; expect(err.message).to.match(/integer/i); @@ -254,10 +257,10 @@ describe('SharedMethod', function() { returns: { arg: 'value', type: 'integer' }, }); - method.invoke('ctx', {}, function(err, result) { + method.invoke('ctx', {}, {}, ctx(method), function(err, result) { setImmediate(function() { expect(err).to.exist; - expect(err.message).to.match(/safe integer/i); + expect(err.message).to.match(/integer/i); expect(err.statusCode).to.equal(500); done(); }); @@ -270,10 +273,10 @@ describe('SharedMethod', function() { accepts: [{ arg: 'obj', type: 'object' }], }); - method.invoke('ctx', { obj: 'test' }, function(err) { + method.invoke('ctx', { obj: 'test' }, {}, ctx(method), function(err) { setImmediate(function() { expect(err).to.exist; - expect(err.message).to.contain('invalid value for argument'); + expect(err.message).to.contain('not an object'); expect(err.statusCode).to.equal(400); done(); }); @@ -294,7 +297,7 @@ describe('SharedMethod', function() { ], }); - method.invoke('ctx', {}, function(err, result) { + method.invoke('ctx', {}, {}, ctx(method), function(err, result) { setImmediate(function() { expect(result).to.eql({ first: 'one', second: 'two' }); done(); @@ -315,7 +318,7 @@ describe('SharedMethod', function() { ], }); - method.invoke('ctx', {}, function(err, result) { + method.invoke('ctx', {}, {}, ctx(method), function(err, result) { setImmediate(function() { expect(result).to.eql({ value: 'data' }); done(); @@ -336,7 +339,7 @@ describe('SharedMethod', function() { ], }); - method.invoke('ctx', {}, function(err, result) { + method.invoke('ctx', {}, {}, ctx(method), function(err, result) { setImmediate(function() { expect(result).to.eql({ value: ['a', 'b'] }); done(); @@ -352,24 +355,28 @@ describe('SharedMethod', function() { }); }); - method.invoke('ctx', {}, function(err, result) { + method.invoke('ctx', {}, {}, ctx(method), function(err, result) { setImmediate(function() { expect(err).to.equal(testError); done(); }); }); }); + }); - function givenSharedMethod(fn, options) { - if (options === undefined && typeof fn === 'object') { - options = fn; - fn = function() { - arguments[arguments.length - 1](); - }; - } - - var mockSharedClass = { fn: fn }; - return new SharedMethod(fn, 'fn', mockSharedClass, options); + function givenSharedMethod(fn, options) { + if (options === undefined && typeof fn === 'object') { + options = fn; + fn = function() { + arguments[arguments.length - 1](); + }; } - }); + + var mockSharedClass = { fn: fn }; + return new SharedMethod(fn, 'fn', mockSharedClass, options); + } + + function ctx(method) { + return new Context(method, new TypeRegistry({ warnOnUnknownType: false })); + } }); diff --git a/test/type.test.js b/test/type.test.js deleted file mode 100644 index a52c4667..00000000 --- a/test/type.test.js +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright IBM Corp. 2014,2015. All Rights Reserved. -// Node module: strong-remoting -// This file is licensed under the Artistic License 2.0. -// License text available at https://opensource.org/licenses/Artistic-2.0 - -var assert = require('assert'); -var Dynamic = require('../lib/dynamic'); -var RemoteObjects = require('../'); - -describe('types', function() { - var remotes; - beforeEach(function() { - remotes = RemoteObjects.create(); - }); - describe('remotes.defineType(name, fn)', function() { - it('should define a new type converter', function() { - var name = 'MyType'; - remotes.defineType(name, function(val, ctx) { - return val; - }); - assert(Dynamic.getConverter(name)); - }); - }); - - describe('Dynamic(val, [ctx])', function() { - describe('Dynamic.to(typeName)', function() { - it('should convert the dynamic to the given type', function() { - Dynamic.define('beep', function(str) { - return 'boop'; - }); - var dyn = new Dynamic('beep'); - assert.equal(dyn.to('beep'), 'boop'); - }); - }); - describe('Dynamic.canConvert(typeName)', function() { - it('should only return true when a converter exists', function() { - Dynamic.define('MyType', function() {}); - assert(Dynamic.canConvert('MyType')); - assert(!Dynamic.canConvert('FauxType')); - }); - }); - describe('Built in converters', function() { - it('should convert Boolean values', function() { - shouldConvert(true, true); - shouldConvert(false, false); - shouldConvert(256, true); - shouldConvert(-1, true); - shouldConvert(1, true); - shouldConvert(0, false); - shouldConvert('true', true); - shouldConvert('false', false); - shouldConvert('0', false); - shouldConvert('1', true); - shouldConvert('-1', true); - shouldConvert('256', true); - shouldConvert('null', false); - shouldConvert('undefined', false); - shouldConvert('', false); - - function shouldConvert(val, expected) { - var dyn = new Dynamic(val); - assert.equal(dyn.to('boolean'), expected); - } - }); - it('should convert Number values', function() { - shouldConvert('-1', -1); - shouldConvert('0', 0); - shouldConvert('1', 1); - shouldConvert('0.1', 0.1); - shouldConvert(1, 1); - shouldConvert(true, 1); - shouldConvert(false, 0); - shouldConvert({}, 'NaN'); - shouldConvert([], 0); - - function shouldConvert(val, expected) { - var dyn = new Dynamic(val); - - if (expected === 'NaN') { - return assert(Number.isNaN(dyn.to('number'))); - } - - assert.equal(dyn.to('number'), expected); - } - }); - }); - }); -});