Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: require an extended body parser #532

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat: require an extended body parser #532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 13 additions & 40 deletions lib/types/urlencoded.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ var typeis = require('type-is')

module.exports = urlencoded

/**
* Cache of parser modules.
*/

var parsers = Object.create(null)

/**
* Create a middleware to parse urlencoded bodies.
*
Expand All @@ -48,7 +42,6 @@ function urlencoded (options) {
deprecate('undefined extended: provide extended option')
}

var extended = opts.extended !== false
var inflate = opts.inflate !== false
var limit = typeof opts.limit !== 'number'
? bytes.parse(opts.limit || '100kb')
Expand All @@ -61,8 +54,8 @@ function urlencoded (options) {
}

// create the appropriate query parser
var queryparse = extended
? extendedparser(opts)
var queryparse = typeof opts.extended === 'function'
? extendedparser(opts.extended, opts)
: simpleparser(opts)

// create the appropriate type checking function
Expand Down Expand Up @@ -129,11 +122,10 @@ function urlencoded (options) {
* @param {object} options
*/

function extendedparser (options) {
function extendedparser (parse, options) {
var parameterLimit = options.parameterLimit !== undefined
? options.parameterLimit
: 1000
var parse = parser('qs')

if (isNaN(parameterLimit) || parameterLimit < 1) {
throw new TypeError('option parameterLimit must be a positive number')
Expand All @@ -153,15 +145,8 @@ function extendedparser (options) {
})
}

var arrayLimit = Math.max(100, paramCount)

debug('parse extended urlencoding')
return parse(body, {
allowPrototypes: true,
arrayLimit: arrayLimit,
depth: Infinity,
parameterLimit: parameterLimit
})
return parse(body)
}
}

Expand Down Expand Up @@ -204,35 +189,23 @@ function parameterCount (body, limit) {
return count
}

let queryStringModule

/**
* Get parser for module name dynamically.
* Loads the querystring module lazily and caches it
*
* @param {string} name
* @return {function}
* @api private
*/

function parser (name) {
var mod = parsers[name]

if (mod !== undefined) {
return mod.parse
}

// this uses a switch for static require analysis
switch (name) {
case 'qs':
mod = require('qs')
break
case 'querystring':
mod = require('querystring')
break
function loadQueryStringModule () {
if (queryStringModule) {
return queryStringModule.parse
}

// store to prevent invoking require()
parsers[name] = mod
queryStringModule = require('querystring')

return mod.parse
return queryStringModule.parse
}

/**
Expand All @@ -245,7 +218,7 @@ function simpleparser (options) {
var parameterLimit = options.parameterLimit !== undefined
? options.parameterLimit
: 1000
var parse = parser('querystring')
var parse = loadQueryStringModule()

if (isNaN(parameterLimit) || parameterLimit < 1) {
throw new TypeError('option parameterLimit must be a positive number')
Expand Down
1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"http-errors": "2.0.0",
"iconv-lite": "0.4.24",
"on-finished": "2.4.1",
"qs": "6.12.3",
"raw-body": "2.5.2",
"type-is": "~1.6.18",
"unpipe": "1.0.0"
Expand Down
109 changes: 16 additions & 93 deletions test/urlencoded.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,20 @@ describe('bodyParser.urlencoded()', function () {
.expect(200, '{"user":"tobi"}', done)
})

it('should parse extended syntax', function (done) {
it('should parse multiple key instances', function (done) {
request(this.server)
.post('/')
.set('Content-Type', 'application/x-www-form-urlencoded')
.send('user=Tobi&user=Loki')
.expect(200, '{"user":["Tobi","Loki"]}', done)
})

it('should parse not parse nested objects', function (done) {
request(this.server)
.post('/')
.set('Content-Type', 'application/x-www-form-urlencoded')
.send('user[name][first]=Tobi')
.expect(200, '{"user":{"name":{"first":"Tobi"}}}', done)
.expect(200, '{"user[name][first]":"Tobi"}', done)
})

describe('with extended option', function () {
Expand All @@ -119,103 +127,18 @@ describe('bodyParser.urlencoded()', function () {
})
})

describe('when true', function () {
describe('when set to a custom parse function', function () {
before(function () {
this.server = createServer({ extended: true })
})

it('should parse multiple key instances', function (done) {
request(this.server)
.post('/')
.set('Content-Type', 'application/x-www-form-urlencoded')
.send('user=Tobi&user=Loki')
.expect(200, '{"user":["Tobi","Loki"]}', done)
})

it('should parse extended syntax', function (done) {
request(this.server)
.post('/')
.set('Content-Type', 'application/x-www-form-urlencoded')
.send('user[name][first]=Tobi')
.expect(200, '{"user":{"name":{"first":"Tobi"}}}', done)
})

it('should parse parameters with dots', function (done) {
request(this.server)
.post('/')
.set('Content-Type', 'application/x-www-form-urlencoded')
.send('user.name=Tobi')
.expect(200, '{"user.name":"Tobi"}', done)
})

it('should parse fully-encoded extended syntax', function (done) {
request(this.server)
.post('/')
.set('Content-Type', 'application/x-www-form-urlencoded')
.send('user%5Bname%5D%5Bfirst%5D=Tobi')
.expect(200, '{"user":{"name":{"first":"Tobi"}}}', done)
})

it('should parse array index notation', function (done) {
request(this.server)
.post('/')
.set('Content-Type', 'application/x-www-form-urlencoded')
.send('foo[0]=bar&foo[1]=baz')
.expect(200, '{"foo":["bar","baz"]}', done)
})

it('should parse array index notation with large array', function (done) {
var str = 'f[0]=0'

for (var i = 1; i < 500; i++) {
str += '&f[' + i + ']=' + i.toString(16)
}

request(this.server)
.post('/')
.set('Content-Type', 'application/x-www-form-urlencoded')
.send(str)
.expect(function (res) {
var obj = JSON.parse(res.text)
assert.strictEqual(Object.keys(obj).length, 1)
assert.strictEqual(Array.isArray(obj.f), true)
assert.strictEqual(obj.f.length, 500)
})
.expect(200, done)
})

it('should parse array of objects syntax', function (done) {
request(this.server)
.post('/')
.set('Content-Type', 'application/x-www-form-urlencoded')
.send('foo[0][bar]=baz&foo[0][fizz]=buzz&foo[]=done!')
.expect(200, '{"foo":[{"bar":"baz","fizz":"buzz"},"done!"]}', done)
const parser = () => ({ foo: 'bar' })
this.server = createServer({ extended: parser })
})

it('should parse deep object', function (done) {
var str = 'foo'

for (var i = 0; i < 500; i++) {
str += '[p]'
}

str += '=bar'

it('should parse via custom parser', function (done) {
request(this.server)
.post('/')
.set('Content-Type', 'application/x-www-form-urlencoded')
.send(str)
.expect(function (res) {
var obj = JSON.parse(res.text)
assert.strictEqual(Object.keys(obj).length, 1)
assert.strictEqual(typeof obj.foo, 'object')

var depth = 0
var ref = obj.foo
while ((ref = ref.p)) { depth++ }
assert.strictEqual(depth, 500)
})
.expect(200, done)
.send('a=b')
.expect(200, '{"foo":"bar"}', done)
})
})
})
Expand Down