From c62ba21307a93ec850ba8a70cabf74dbbd4fefec Mon Sep 17 00:00:00 2001 From: Heath Morrison Date: Tue, 18 Oct 2016 22:39:30 +0200 Subject: [PATCH] Coerce array-like objects into arrays The query-string parser used by express https://github.com/ljharb/qs#parsing-arrays limits the size of arrays that are created from query strings to 20 items. Arrays larger than that are converted to objects using numeric indices. This commit fixes the coercion algorithm used by queries to treat number-indexed objects as arrays. We still maintain a strict understanding of an "array-like object" to limit the opportunity for subtle bugs. In particular, the presence of non-index keys is an indication that the object was not intended to be interpreted as an array. --- lib/dao.js | 66 ++++++++++++++++++++++++++++++++-------- test/loopback-dl.test.js | 50 ++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 13 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 7ac5ce97..25142265 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -1537,6 +1537,27 @@ function NumberType(val) { return !isNaN(num) ? num : val; } +function coerceArray(val) { + if (Array.isArray(val)) { + return val; + } + + if (!utils.isPlainObject(val)) { + throw new Error(g.f('Value is not an {{array}} or {{object}} with sequential numeric indices')); + } + + var arrayVal = new Array(Object.keys(val).length); + for (var i = 0; i < arrayVal.length; ++i) { + if (!val.hasOwnProperty(i)) { + throw new Error(g.f('Value is not an {{array}} or {{object}} with sequential numeric indices')); + } + + arrayVal[i] = val[i]; + } + + return arrayVal; +} + /* * Coerce values based the property types * @param {Object} where The where clause @@ -1561,16 +1582,18 @@ DataAccessObject._coerce = function(where) { // Handle logical operators if (p === 'and' || p === 'or' || p === 'nor') { var clauses = where[p]; - if (Array.isArray(clauses)) { - for (var k = 0; k < clauses.length; k++) { - self._coerce(clauses[k]); - } - } else { - err = new Error(g.f('The %s operator has invalid clauses %j', p, clauses)); + try { + clauses = coerceArray(clauses); + } catch (e) { + err = new Error(g.f('The %s operator has invalid clauses %j: %s', p, clauses, e.message)); err.statusCode = 400; throw err; } + for (var k = 0; k < clauses.length; k++) { + self._coerce(clauses[k]); + } + continue; } var DataType = props[p] && props[p].type; @@ -1625,15 +1648,21 @@ DataAccessObject._coerce = function(where) { switch (operator) { case 'inq': case 'nin': - if (!Array.isArray(val)) { - err = new Error(g.f('The %s property has invalid clause %j', p, where[p])); + case 'between': + try { + val = coerceArray(val); + } catch (e) { + err = new Error(g.f('The %s property has invalid clause %j: %s', p, where[p], e)); err.statusCode = 400; throw err; } - break; - case 'between': - if (!Array.isArray(val) || val.length !== 2) { - err = new Error(g.f('The %s property has invalid clause %j', p, where[p])); + + if (operator === 'between' && val.length !== 2) { + err = new Error(g.f( + 'The %s property has invalid clause %j: Expected precisely 2 values, received %d', + p, + where[p], + val.length)); err.statusCode = 400; throw err; } @@ -1643,7 +1672,10 @@ DataAccessObject._coerce = function(where) { case 'ilike': case 'nilike': if (!(typeof val === 'string' || val instanceof RegExp)) { - err = new Error(g.f('The %s property has invalid clause %j', p, where[p])); + err = new Error(g.f( + 'The %s property has invalid clause %j: Expected a string or RegExp', + p, + where[p])); err.statusCode = 400; throw err; } @@ -1660,6 +1692,14 @@ DataAccessObject._coerce = function(where) { } } } + + try { + // Coerce val into an array if it resembles an array-like object + val = coerceArray(val); + } catch (e) { + // NOOP when not coercable into an array. + } + // Coerce the array items if (Array.isArray(val)) { for (var i = 0; i < val.length; i++) { diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index fc0e5a7c..ccabc951 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -1476,6 +1476,56 @@ describe('DataAccessObject', function() { assert.deepEqual(where, {and: [{age: 10}], vip: true}); }); + var COERCIONS = [ + { + in: {scores: {0: '10', 1: '20'}}, + out: {scores: [10, 20]}, + }, + { + in: {and: {0: {age: '10'}, 1: {vip: 'true'}}}, + out: {and: [{age: 10}, {vip: true}]}, + }, + { + in: {or: {0: {age: '10'}, 1: {vip: 'true'}}}, + out: {or: [{age: 10}, {vip: true}]}, + }, + { + in: {id: {inq: {0: 'aaa', 1: 'bbb'}}}, + out: {id: {inq: ['aaa', 'bbb']}}, + }, + { + in: {id: {nin: {0: 'aaa', 1: 'bbb'}}}, + out: {id: {nin: ['aaa', 'bbb']}}, + }, + { + in: {scores: {between: {0: '0', 1: '42'}}}, + out: {scores: {between: [0, 42]}}, + }, + ]; + + COERCIONS.forEach(function(coercion) { + var inStr = JSON.stringify(coercion.in); + it('coerces where clause with array-like objects ' + inStr, function() { + assert.deepEqual(model._coerce(coercion.in), coercion.out); + }); + }); + + var INVALID_CLAUSES = [ + {scores: {inq: {0: '10', 1: '20', 4: '30'}}}, + {scores: {inq: {0: '10', 1: '20', bogus: 'true'}}}, + {scores: {between: {0: '10', 1: '20', 2: '30'}}}, + ]; + + INVALID_CLAUSES.forEach(function(where) { + var whereStr = JSON.stringify(where); + it('throws an error on malformed array-like object ' + whereStr, + function() { + assert.throws( + function() { model._coerce(where); }, + /property has invalid clause/); + }); + }); + it('throws an error if the where property is not an object', function() { try { // The where clause has to be an object