Skip to content

Commit a59c339

Browse files
virtual machinevirtual machine
authored andcommitted
Fix query parameter truncation with configurable limit
- Change default query parser from 'simple' to 'extended' - Add 'query parser limit' setting with default of 10000 - Pass limit to query parser at parse time (not compile time) - Fixes issue expressjs#5878 - query params truncated at 1000+ params - Supersedes PR expressjs#7116 which used Infinity (security concern)
1 parent 6c4249f commit a59c339

4 files changed

Lines changed: 29 additions & 8 deletions

File tree

lib/application.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ app.defaultConfiguration = function defaultConfiguration() {
9494
this.enable('x-powered-by');
9595
this.set('etag', 'weak');
9696
this.set('env', env);
97-
this.set('query parser', 'simple')
97+
this.set('query parser', 'extended')
98+
this.set('query parser limit', 10000);
9899
this.set('subdomain offset', 2);
99100
this.set('trust proxy', false);
100101

lib/request.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,9 @@ defineGetter(req, 'query', function query(){
236236
}
237237

238238
var querystring = parse(this).query;
239+
var limit = this.app.get('query parser limit');
239240

240-
return queryparse(querystring);
241+
return queryparse(querystring, limit);
241242
});
242243

243244
/**

lib/utils.js

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,18 +163,27 @@ exports.compileQueryParser = function compileQueryParser(val) {
163163
var fn;
164164

165165
if (typeof val === 'function') {
166-
return val;
166+
return function(str, limit) {
167+
// For custom functions, pass both args but don't enforce limit
168+
return val(str);
169+
};
167170
}
168171

169172
switch (val) {
170173
case true:
171174
case 'simple':
172-
fn = querystring.parse;
175+
fn = function(str, limit) {
176+
// querystring.parse doesn't support limit, ignore it
177+
return querystring.parse(str);
178+
};
173179
break;
174180
case false:
175181
break;
176182
case 'extended':
177-
fn = parseExtendedQueryString;
183+
fn = function(str, limit) {
184+
// parseExtendedQueryString supports limit
185+
return parseExtendedQueryString(str, limit);
186+
};
178187
break;
179188
default:
180189
throw new TypeError('unknown value for query parser function: ' + val);
@@ -260,12 +269,14 @@ function createETagGenerator (options) {
260269
* Parse an extended query string with qs.
261270
*
262271
* @param {String} str
272+
* @param {Number} [limit] - Maximum number of parameters to parse (default: 10000)
263273
* @return {Object}
264274
* @private
265275
*/
266276

267-
function parseExtendedQueryString(str) {
277+
function parseExtendedQueryString(str, limit) {
268278
return qs.parse(str, {
269-
allowPrototypes: true
279+
allowPrototypes: true,
280+
parameterLimit: limit !== undefined ? limit : 10000
270281
});
271282
}

test/req.query.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,17 @@ describe('req', function(){
1414
.expect(200, '{}', done);
1515
});
1616

17-
it('should default to parse simple keys', function (done) {
17+
it('should default to parse extended keys', function (done) {
1818
var app = createApp();
1919

20+
request(app)
21+
.get('/?user[name]=tj')
22+
.expect(200, '{"user":{"name":"tj"}}', done);
23+
});
24+
25+
it('should parse simple keys when configured', function (done) {
26+
var app = createApp('simple');
27+
2028
request(app)
2129
.get('/?user[name]=tj')
2230
.expect(200, '{"user[name]":"tj"}', done);

0 commit comments

Comments
 (0)