From 2377792c22beb674e8444a0271ac5790d882b43f 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 | 47 ++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 13 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 3ffa8699..2ab4a204 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -1526,6 +1526,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 @@ -1550,16 +1571,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; @@ -1614,15 +1637,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; } @@ -1632,7 +1661,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; } @@ -1649,6 +1681,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 56659173..9c84952c 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -1412,6 +1412,53 @@ describe('DataAccessObject', function() { assert.deepEqual(where, {and: [{age: 10}], vip: true}); }); + const 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(coercion => { + var inStr = JSON.stringify(coercion.in); + it('coerces where clause with array-like objects ' + inStr, () => { + assert.deepEqual(model._coerce(coercion.in), coercion.out); + }); + }); + + const 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((where) => { + var whereStr = JSON.stringify(where); + it('throws an error on malformed array-like object ' + whereStr, () => { + assert.throws(() => 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