diff --git a/CHANGES.md b/CHANGES.md index 97a061a35..25cddd5c7 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,12 @@ # restify Changelog +## 4.2.0 + +- #925 Support passing (most) [qs](https://github.com/ljharb/qs#readme) options + to the `restify.queryParser` plugin. Update the "qs" dependency to latest (v6) + while maintaining backward compatibility (see notes in the API doc and + "test/query.test.js" on `allowDots` and `plainObjects`). + ## 4.1.1 - update negotiator (NSP advisory #106) and lru-cache (bug fix). diff --git a/docs/index.restdown b/docs/index.restdown index a9cef6164..2349ed00c 100644 --- a/docs/index.restdown +++ b/docs/index.restdown @@ -929,14 +929,41 @@ interpreted in seconds, to allow for clock skew. ### QueryParser - server.use(restify.queryParser()); + server.use(restify.queryParser([OPTIONS])); + +Parses the HTTP query string (e.g., `/foo?id=bar&name=mark`) to an object on +`req.query`, e.g. `{id: 'bar', name: 'mark'}`. + +By default, query params are merged into `req.params` (which may include +parameters from `restify.bodyParser`). This can be disabled by the `mapParams` +option: + + server.use(restify.queryParser({mapParams: false})); + +QueryParser options are as follows. All but `mapParams` are directly passed to +the underlying ["qs" module](https://github.com/ljharb/qs#readme). + +|| **Option** || **Type** || **Default** || **Description** || +|| mapParams || bool || true || Whether to merge req.query into req.params. || +|| allowDots || bool || true \* || Transform `?foo.bar=baz` to a nested object: `{foo: {bar: 'baz'}}`. || +|| arrayLimit || num || 20 || Only transform `?a[$index]=b` to an array if `$index` is less than `arrayLimit`. || +|| depth || num || 5 || The depth limit for parsing nested objects, e.g. `?a[b][c][d][e][f][g][h][i]=j`. || +|| parameterLimit || num || 1000 || Maximum number of query params parsed. Additional params are silently dropped. || +|| parseArrays || bool || true || Whether to parse `?a[]=b&a[1]=c` to an array, e.g. `{a: ['b', 'c']}`. || +|| plainObjects || bool || true \* || Whether `req.query` is a "plain" object -- does not inherit from `Object`. This can be used to allow query params whose names collide with Object methods, e.g. `?hasOwnProperty=blah`. || +|| strictNullHandling || bool || false || If true, `?a&b=` results in `{a: null, b: ''}`. Otherwise, `{a: '', b: ''}`. || + +\*: The `allowDots` and `plainObjects` options are `true` by default in restify +4.x because of an accident. Restify 4.0 updated from qs@2 to qs@3, which changed +default behavior to this. Subsequent qs major versions reverted that behaviour. +The restify 4.x stream will maintain this default, but all other major versions +of restify (those before v4) and the `queryParser` plugin in restify-plugins@1.x +and later (restify 5.x no longer bundles the plugins) default to +`allowDots=false` and `plainObjects=false`. The recommended usage (for new +code or to maintain compat across restify major versions is): -Parses the HTTP query string (i.e., `/foo?id=bar&name=mark`). If you use this, -the parsed content will always be available in `req.query`, additionally params -are merged into `req.params`. You can disable by passing in `mapParams: false` -in the options object: + server.use(restify.queryParser({allowDots: false, plainObjects: false})); - server.use(restify.queryParser({ mapParams: false })); ### JSONP diff --git a/lib/plugins/query.js b/lib/plugins/query.js index 270fd7b1d..dc4c0f17d 100644 --- a/lib/plugins/query.js +++ b/lib/plugins/query.js @@ -5,6 +5,23 @@ var qs = require('qs'); var assert = require('assert-plus'); +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 '&'. + */ +}; /** * Returns a plugin that will parse the query string, and merge the results @@ -22,6 +39,25 @@ function queryParser(options) { } assert.object(options, 'options'); + /* + * Releases of restify 4.x up to 4.1.1 used qs@3 which effectively defaulted + * to `plainObjects=true` and `allowDots=true`. To maintain backward + * compatibility for the restify 4.x stream while using the latest qs + * version, we need to maintain those defaults. Note that restify-plugins + * changes back to the pre-restify-4.x behaviour. See test/query.test.js + * for more details. + */ + var qsOptions = { + plainObjects: true, + allowDots: true + }; + Object.keys(EXPOSED_QS_OPTIONS).forEach(function (k) { + EXPOSED_QS_OPTIONS[k](options[k], k); // assert type of this option + + if (options.hasOwnProperty(k)) { + qsOptions[k] = options[k]; + } + }); function parseQueryString(req, res, next) { if (!req.getQuery()) { @@ -29,7 +65,7 @@ function queryParser(options) { return (next()); } - req.query = qs.parse(req.getQuery()); + req.query = qs.parse(req.getQuery(), qsOptions); if (options.mapParams !== false) { Object.keys(req.query).forEach(function (k) { diff --git a/package.json b/package.json index 0d06c6635..191799d84 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,7 @@ "name": "restify", "homepage": "http://restifyjs.com", "description": "REST framework", - "version": "4.1.1", + "version": "4.2.0", "repository": { "type": "git", "url": "git://github.com/restify/node-restify.git" @@ -70,7 +70,7 @@ "negotiator": "^0.6.1", "node-uuid": "^1.4.1", "once": "^1.3.0", - "qs": "^3.1.0", + "qs": "^6.2.1", "semver": "^4.3.3", "spdy": "^3.3.3", "tunnel-agent": "^0.4.0", diff --git a/test/plugins.test.js b/test/plugins.test.js index 8cfabfd9f..01ce23ac8 100644 --- a/test/plugins.test.js +++ b/test/plugins.test.js @@ -147,57 +147,6 @@ test('authorization basic invalid', function (t) { }); -test('query ok', function (t) { - SERVER.get('/query/:id', function (req, res, next) { - t.equal(req.params.id, 'foo'); - t.equal(req.params.name, 'markc'); - t.equal(req.params.name, 'markc'); - res.send(); - next(); - }); - - CLIENT.get('/query/foo?id=bar&name=markc', function (err, _, res) { - t.ifError(err); - t.equal(res.statusCode, 200); - t.end(); - }); -}); - - -test('GH-124 query ok no query string', function (t) { - SERVER.get('/query/:id', function (req, res, next) { - t.equal(req.getQuery(), ''); - res.send(); - next(); - }); - - CLIENT.get('/query/foo', function (err, _, res) { - t.ifError(err); - t.equal(res.statusCode, 200); - t.end(); - }); -}); - - -test('query object', function (t) { - SERVER.get('/query/:id', function (req, res, next) { - t.equal(req.params.id, 'foo'); - t.ok(req.params.name); - t.equal(req.params.name.first, 'mark'); - t.equal(req.query.name.last, 'cavage'); - res.send(); - next(); - }); - - var p = '/query/foo?name[first]=mark&name[last]=cavage'; - CLIENT.get(p, function (err, _, res) { - t.ifError(err); - t.equal(res.statusCode, 200); - t.end(); - }); -}); - - test('body url-encoded ok', function (t) { SERVER.post('/bodyurl/:id', restify.bodyParser(), diff --git a/test/query.test.js b/test/query.test.js new file mode 100644 index 000000000..0a5ec4fa9 --- /dev/null +++ b/test/query.test.js @@ -0,0 +1,336 @@ +// Copyright 2012 Mark Cavage, Inc. All rights reserved. + +/* + * Test query param handling, i.e. the 'queryParser' plugin. + */ + +'use strict'; + +var restify = require('../lib'); + + +if (require.cache[__dirname + '/lib/helper.js']) { + delete require.cache[__dirname + '/lib/helper.js']; +} +var helper = require('./lib/helper.js'); + + +///--- Globals + +var after = helper.after; +var before = helper.before; +var test = helper.test; + +var PORT = process.env.UNIT_TEST_PORT || 0; +var CLIENT; +var SERVER; + + +///--- Tests + +before(function (callback) { + try { + SERVER = restify.createServer({ + dtrace: helper.dtrace, + log: helper.getLog('server') + }); + + SERVER.listen(PORT, '127.0.0.1', function () { + PORT = SERVER.address().port; + CLIENT = restify.createJsonClient({ + url: 'http://127.0.0.1:' + PORT, + dtrace: helper.dtrace, + retry: false, + agent: false + }); + + process.nextTick(callback); + }); + } catch (e) { + console.error(e.stack); + process.exit(1); + } +}); + + +after(function (callback) { + try { + SERVER.close(callback); + } catch (e) { + console.error(e.stack); + process.exit(1); + } +}); + + +test('query ok', function (t) { + SERVER.get('/query/:id', + restify.queryParser(), + function (req, res, next) { + t.equal(req.params.id, 'foo'); + t.equal(req.params.name, 'markc'); + res.send(); + next(); + }); + + CLIENT.get('/query/foo?id=bar&name=markc', function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + + +test('GH-124 query ok no query string', function (t) { + SERVER.get('/query/:id', + restify.queryParser(), + function (req, res, next) { + t.equal(req.getQuery(), ''); + res.send(); + next(); + }); + + CLIENT.get('/query/foo', function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + + +test('query: bracket object (default options)', function (t) { + SERVER.get('/query/:id', + restify.queryParser(), + function (req, res, next) { + t.equal(req.params.id, 'foo'); + t.ok(req.params.name); + t.equal(req.params.name.first, 'mark'); + t.equal(req.query.name.last, 'cavage'); + res.send(); + next(); + }); + + var p = '/query/foo?name[first]=mark&name[last]=cavage'; + CLIENT.get(p, function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +test('query: bracket array (default options)', function (t) { + SERVER.get('/query/:id', + restify.queryParser(), + function (req, res, next) { + t.deepEqual(req.params, {id: 'foo', a: [1, 2]}); + res.send(); + next(); + }); + + var p = '/query/foo?a[]=1&a[]=2'; + CLIENT.get(p, function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +test('query: bracket array (#444, #895)', function (t) { + SERVER.get('/query/:id', + restify.queryParser(), + function (req, res, next) { + t.deepEqual(req.params, { + id: 'foo', + pizza: { left: [ 'pepperoni', 'bacon' ], right: [ 'ham' ] } + }); + res.send(); + next(); + }); + + var p = '/query/foo' + + '?pizza[left][]=pepperoni&pizza[left][]=bacon&pizza[right][]=ham'; + CLIENT.get(p, function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + + +/* + * Restify's `queryParser()` plugin uses the "qs" module. Some qs and restify + * module history: + * + * RESTIFY VER QS VER QS OPTIONS NOTES + * (earlier) ??? + * restify@2.x ^1.0.0 + * restify@3.x ^2.4.1 adds `parameterLimit` + * restify@4.x ^3.1.0 allowDots=true is default; qs.parse returns + * "plain" objects + * restify-plugins@1.x ^6.2.1 allowDots=false is default; + * plainObjects=true is default + * + * Notes: + * - After restify@4.x, the plugins have been unbundled such that it is + * the restify-plugins version that is relevant for `queryParser()` behaviour. + * - qs@3 was an abberation in that `allowDots` (parsing `?field.a=1` into + * `{field: 'a'}`) was true by default and "plain" objects (null prototype) + * were returned. This was changed in qs@5 to have the default behaviour + * of qs@2. + * + * Unfortunately this means that restify@4.x's `queryParser()` is an aberration: + * restify before v4 and after (via restify-plugins) have the same default + * behaviour. + * + * The following test cases attempt to ensure unintended query string parsing + * changes don't surprise restify's `queryParser()` again. + */ + +// Note: This default is the opposite of restify versions other than 4.x +// and restify-plugins. +test('query: plainObjects=true by default', function (t) { + SERVER.get('/query/:id', + restify.queryParser(), + function (req, res, next) { + t.equal(req.query.field, 'value'); + // Doesn't inherit from Object (i.e. a 'plain' object). + t.ok(req.query.hasOwnProperty === undefined); + res.send(); + next(); + }); + + var p = '/query/foo?field=value'; + CLIENT.get(p, function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +// Note: This default is the opposite of restify versions other than 4.x +// and restify-plugins. +test('query: allowDots=true by default', function (t) { + SERVER.get('/query/:id', + restify.queryParser(), + function (req, res, next) { + t.deepEqual(req.params, + {id: 'foo', name: {first: 'Trent', last: 'Mick'}}); + res.send(); + next(); + }); + + var p = '/query/foo?name.first=Trent&name.last=Mick'; + CLIENT.get(p, function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +test('query: allowDots=false plainObjects=false', function (t) { + SERVER.get('/query/:id', + restify.queryParser({allowDots: false, plainObjects: false}), + function (req, res, next) { + t.deepEqual(req.query, + {'name.first': 'Trent', 'name.last': 'Mick'}); + t.equal(typeof (req.query.hasOwnProperty), 'function'); + t.equal(req.query.hasOwnProperty('name.first'), true); + res.send(); + next(); + }); + + var p = '/query/foo?name.first=Trent&name.last=Mick'; + CLIENT.get(p, function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + + +test('query: mapParams=false', function (t) { + SERVER.get('/query/:id', + restify.queryParser({mapParams: false}), + function (req, res, next) { + t.deepEqual(req.query, {field: 'value'}); + t.equal(req.params.hasOwnProperty('field'), false); + res.send(); + next(); + }); + + var p = '/query/foo?field=value'; + CLIENT.get(p, function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +test('query: arrayLimit=1', function (t) { + SERVER.get('/query/:id', + restify.queryParser({arrayLimit: 2}), + function (req, res, next) { + t.deepEqual(req.query, {field: {0: 'a', 3: 'b'}}); + res.send(); + next(); + }); + + var p = '/query/foo?field[]=a&field[3]=b'; + CLIENT.get(p, function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +test('query: depth=2', function (t) { + SERVER.get('/query/:id', + restify.queryParser({depth: 2}), + function (req, res, next) { + t.deepEqual(req.query, + { a: { b: { c: { '[d][e][f][g][h][i]': 'j' } } } }); + res.send(); + next(); + }); + + var p = '/query/foo?a[b][c][d][e][f][g][h][i]=j'; + CLIENT.get(p, function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +test('query: parameterLimit=3', function (t) { + SERVER.get('/query/:id', + restify.queryParser({parameterLimit: 3}), + function (req, res, next) { + t.deepEqual(req.query, {a: 'b', c: 'd', e: 'f'}); + res.send(); + next(); + }); + + var p = '/query/foo?a=b&c=d&e=f&g=h'; + CLIENT.get(p, function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +test('query: strictNullHandling=true', function (t) { + SERVER.get('/query/:id', + restify.queryParser({strictNullHandling: true}), + function (req, res, next) { + t.deepEqual(req.query, {a: null, b: '', c: 'd'}); + res.send(); + next(); + }); + + var p = '/query/foo?a&b=&c=d'; + CLIENT.get(p, function (err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +});