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.

Author: Heath Morrison

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++) {

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