Express: Make it possible to configure the querystring parser *options*

[wearhere]

Over on qs' repository I and some others have identified an ambiguity in query string parsing that can be resolved by setting arrayLimit: 0. How may I do this? Ideally I would not have to set a custom string parsing _function_ on my application, which I see you support with the 'query parser' setting here, but rather just pass some options to your default invocation of qs.parse.

I see that you expose your query middleware, which uses qs.parse by default, but unfortunately doing

var express = require('express');
var app = express();

app.set('query parser', false);
app.use(express.query({
  arrayLimit: 0
}));

does not suffice because with query parsing disabled, req.query is set to {} by the time that express.query runs, which causes it to skip parsing.

(Note: it would suffice for my purposes if you would add arrayLimit: 0 to your default qs.parse options here (and I suppose here even though that's not used by the framework) and perhaps you will consider doing so given that this ambiguity is problematic and arrayLimit: 0 may become the default in the next version of qs.)

[dougwilson]

Hi @wearhere, we are just not going to really expose you the options to do this, as the qs module just keeps changing the options with it's continuous major version bumps. Exposing this is a very hard thing to control and would end up leaving Express 4.x stuck on some really old version of qs. This was a bug issue in 3.x and so the decision was to just allow you to provide a parser function, in which you can use whatever version and options of qs you want, which is better in the long run.

Here is how you configure the options of qs in Express:

var express = require('express');
var qs = require('qs');
var app = express();

app.set('query parser', function (str) {
  return qs.parse(str, opts);
});

The express.query is a middleware that was supposed to have been removed in the 3.x -> 4.x transition, but was forgotten. Unfortunately we cannot actually remove it, because people may be using it, perhaps even mixing it up with Connect or some such, nor can we change it's behavior. In Express 5.x we have already completely removed it, so of course suggesting changes to it for the next major wouldn't make sense, since it no longer exists.

[wearhere]

Thanks for the response @dougwilson! That clarifies why using passing options through to qs and/or using express.query would not be a good idea.

Would you still consider adding arrayLimit: 0 to your default qs.parse options here? Then you would not need to wait for arrayLimit: 0 to become the default in the next version of qs. Also this would help express be more explicit in its behavior—see qs' maintainer's thoughts here.

[dougwilson]

Would you still consider adding arrayLimit: 0 to your default qs.parse options here?

At the very least, it is a breaking change and thus cannot occur in Express 4.x, so just wanted to frame the discussion for this in a 5.x time line so we're on the same page.

As for the actual change, the body-parser module also uses qs, and there was actually a lot of discussion around arrayLimit: 0 there and I even wanted to do it. There was a lot of contention and it just ended up being too incompatible in a general sense, and currently the body-parser module works in basically a arrayLimit: "auto" mode, which ended up being the best of both worlds. If that can just get added into qs instead, that would actually just be the most ideal solution, i.m.o.

[dougwilson]

We've also made changes currently in Express 5.x to not even use qs by default, and are generally working to downplay that module as an in-built thing in Express, so I'm not sure how this suggestion really plays in with the goals of Express 5.x

[wearhere]

At the very least, it is a breaking change and thus cannot occur in Express 4.x, so just wanted to frame the discussion for this in a 5.x time line so we're on the same page.

For sure.

currently the body-parser module works in basically a arrayLimit: "auto" mode, which ended up being the best of both worlds. If that can just get added into qs instead, that would actually just be the most ideal solution, i.m.o.

Oh interesting. Would you mind commenting with that idea on https://github.com/ljharb/qs/issues/107? That might be helpful to @ljharb.

We've also made changes currently in Express 5.x to not even use qs by default, and are generally working to downplay that module as an in-built thing in Express, so I'm not sure how this suggestion really plays in with the goals of Express 5.x

Interesting. What are you replacing it with & what will be the new array-parsing behavior? (Hopefully something that avoids the ambiguity in qs' parsing?)

[ljharb]

I'd prefer to remove the ambiguity in qs' parsing, which is why I'm planning on changing arrayLimit to 0.

@dougwilson are the reasons express 5 won't use qs by default something I can address? If not, no worries, but if so, I'd prefer to do so.

I'd definitely consider adding an "auto" arrayLimit regardless, if someone wants to make a PR.

[dougwilson]

@ljharb sure, I will see about getting the "auto mode" ported upstream directly to qs :) A description is that it just counts the number of parameters in the string and sets the arrayLimit to that, making it seem infinite, but yet still protecting against the original attack.

As for Express 5.x, of course it's still in alpha and anything _can_ be changed, but the main reason is that since qs (of course, this was way before you became the maintainer :) ) does not actually correspond to any real specification, there are always arguments about how to parse, from using [ vs . to separate keys, to how arrays, Booleans, and more are represented.

The ideal situation would have been if the W3C went ahead with the JSON form spec and that became qs and we would use it. Here is the qs issue towards that goal: https://github.com/ljharb/qs/issues/63 Since the issue was closed, we choose to move away from qs, and either away from fancy query strings all together, or a different implementation that just coincides to _some actual spec_.

The current plans for Express 5.x are the following:

• Default to the Node.js querystring module to parse them by default.
• Provide an option to use qs

The possible changes to that plan that could happen before release are many, including not using the Node.js querystring at all, but rather a custom parser that provides predictability on which values will be arrays or strings and when (initial work was being done under https://github.com/pillarjs/qs-strict).

[ljharb]

Thanks - I was unaware there was actually a spec on this. I've reopened that issue, and would love further comment so I can try to understand what's involved.

[schmod]

Has there been any movement on this issue? The current behavior is unexpected and unpredictable for most users, and the workaround seems like a kludge.

Can we queue this up for the next semver-major release, or at least provide a straightforward opt-in to more-predictable qs parsing?

[dougwilson]

The next major version ~won't have the qs module at all~ is likely to default to queryparser, but it's not for sure.

[dougwilson]

The solution for the original issue is https://github.com/expressjs/express/issues/3039#issuecomment-235960551 so not sure why the issue wasn't closed before.

[schmod]

Thanks!

[ljharb]

For what it's worth, I remain happy to include these changes in qs; all it takes is a PR.

[dougwilson]

Ah yes, @ljharb that's why I was keeping the issue open: to remind myself to eventually make that PR. I guess that didn't work very well :) I'll see about working on it.

[dougwilson]

Also, fixed my statement in https://github.com/expressjs/express/issues/3039#issuecomment-337674970 after looking back through the discussions and issues. I know there is an issue open where a user wants qs gone, but IDK how realistic that is.