queryParser: whether to limit and explicitly list which qs.parse options are passed through

[trentm]

@micahr, @yunong, @mmouterde I have a question about the change from #39. Recently I updated the 'queryParser' plugin in restify 4.x to also pass through options to qs.parse in restify/node-restify#1209 However, in the change to restify I (a) limited the set of options that were passed through and (b) explicitly listed and validated the types of the passed through options.

The set of qs.parse options passed through are:

var EXPOSED_QS_OPTIONS = {
    allowDots: assert.optionalBool,
    arrayLimit: assert.optionalNumber,
    depth: assert.optionalNumber,
    parameterLimit: assert.optionalNumber,
    parseArrays: assert.optionalBool,
    plainObjects: assert.optionalBool,
    strictNullHandling: assert.optionalBool

    /*
     * Exclusions (`qs.parse` options that restify does NOT expose):
     * - `allowPrototypes`: It is strongly suggested against in qs docs.
     * - `decoder`
     * - `delimiter`: For query string parsing we shouldn't support anything
     *   but the default '&'.
     */
};

That shows reasons (if any, other than tending to not add features without need) for excluding a few of the options.

Thoughts on whether we want to do the same for restify-plugins' queryParser? I.e. should I submit a PR to restify-plugins to explicit list, assert type on, and limit the set of qs.parse options?

[yunong]

@trentm I don't see why we would explicitly disable those options. We could add documentation warning of the consequences but leave it up to the end user whether they want to use them or not.

[trentm]

@yunong Okay sure. In my old age, I'm tending to default to being more strict and explicit at API barriers, but basically hitching queryParser to the "qs" module's API should be fine. After some drama around qs versions 3, 4, 5 and 6 I'd expect it to be mostly static now. Thanks for answering.

I'll try to get a PR for doc updates here soon.

[yunong]

@trentm Thanks for all of your work. 👍

[trentm]

k, I've commited a docs update to the restify-plugins readme.