Loopback: Error when having many filter in `filter[where][or][0][id]=1` format

[davidcheung]

Problem

when a user sends a GET request with many filters with this format as follows

http://localhost:3000/api/Books?
filter[where][or][0][id]=1
&filter[where][or][1][id]=2
&filter[where][or][2][id]=1
&filter[where][or][3][id]=1
&filter[where][or][4][id]=1
&filter[where][or][5][id]=1
&filter[where][or][6][id]=1
&filter[where][or][7][id]=1
&filter[where][or][8][id]=1
&filter[where][or][9][id]=1
&filter[where][or][10][id]=1
&filter[where][or][11][id]=1
&filter[where][or][12][id]=1
&filter[where][or][13][id]=1
&filter[where][or][14][id]=1
&filter[where][or][15][id]=1
&filter[where][or][16][id]=1
&filter[where][or][17][id]=1
&filter[where][or][18][id]=1
&filter[where][or][19][id]=1
&filter[where][or][20][id]=1
&filter[where][or][21][id]=1

when the Array or[] have over 20 indices it maps it as an object
resulting in juggler throwing an error

this behavior is caused by express query parser https://github.com/ljharb/qs#parsing-arrays where default arrayLimit is 20
so req.query.where.filter.or becomes an object once it hits 20 indices

How to adjust the limit

include qs in your server/server.js and override the query parser function

'use strict';

var loopback = require('loopback');
var boot = require('loopback-boot');
var qs = require('qs');

var app = module.exports = loopback();
app.set('query parser', function(value, option) {
  return qs.parse(value, {arrayLimit: 500});
});

app.start = function() {

Solution

I dont think it make sense to just increase the number, since there is no "correct" number to set it as, while it is unclear to users that this would be a problem, perhaps all we can do is document this behavior?

@strongloop/loopback-dev @b-admike thoughts?

[jannyHou]

Agree with document it, probably under where filter tag:
http://loopback.io/doc/en/lb2/Where-filter.html

[superkhau]

There is nothing to resolve here as this is expected behaviour in express/qs. We should document the workaround for end-users though. I see this as a doc issue and not a bug, gonna relabel. @crandmck Can you help document this?

[superkhau]

@davidcheung I'll leave it up to you if you want to mark this as a bug again -- and the proposed solutions we discussed:

• Add an option to LoopBack to allow some sort of override out-of-box
• Increase the default limit in the LoopBack repo
• Hack up juggler to convert the object back to an Array

My opinion is as long as we provide the warning and workaround above, it should be good enough for now. Whether we want to take it further using one of the solutions in the list above is up for debate.

[bajtos]

I'm fine with keeping the current implementation and updating the documentation only.

IMO, the best solution is to encode the large filter object as JSON.

http://localhost:3000/api/Books
?filter={"where":{"or":[{"id":1},{"id":2},...,{"id":20"},{"id":21}]}}

[superkhau]

We should document @bajtos's suggestion too -- but we should also confirm that there are no issues with that (I assume there aren't -- hopefully qs agrees).

[davidcheung]

updated in http://loopback.io/doc/en/lb2/Where-filter.html#rest-api closing issue now

[doublemarked]

Just ran into this today when hitting a find route with an inq filter containing > 20 items. IMHO it's unreasonable for code to break by default if you suddenly have 20 items in an inq.

I'm glad for the arrayLimit solution, however - should juggler perhaps be fixed so that it can work with either an array or an object? Currently it just bails because it doesn't understand how to process the object that qs set up: https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/dao.js#L1614-L1619

... that's inside the method _coerce. Maybe it should get better at coercing? :)

[doublemarked]

Also, in the very least, can we improve the error inside _coerce to hint at arrayLimit? It took me an annoying amount of time to unravel the problem.

[davidcheung]

@doublemarked agreed, lets note the limitation from the _coercion method for now as a temp fix, so people can at least troubleshoot this without digging into the code all the way to qs

then later look into getting better at coercing :p

Maybe it should get better at coercing? :)

@bajtos @superkhau

[bajtos]

Sure, I think improving _coerce to handle object-like arrays created by qs is a reasonable idea. Anybody willing to send a pull request?

[doublemarked]

@bajtos I can give it a go.

BTW, according to https://github.com/ljharb/qs/issues/107 ... it looks likely that v7 of qs may default arrayLength to 0, so the direction things are going is that we'll need the juggler parsing array-like objects. Also in that Issue they reveal that arrayLength exists to prevent params like key[999999] auto-vivifying an array of that length, which I'll keep in mind to avoid as well.