Skip to content

Processing unindexed array fields cause a whole process to crash #44

@pepkin88

Description

@pepkin88

A common use case is to use array fields with no specified indexes, e.g.:

<input type="checkbox" name="fruits[]" value="apple">
<input type="checkbox" name="fruits[]" value="orange">
<input type="checkbox" name="fruits[]" value="banana">

However, upon receiving more than one such field, e.g. fruits[]=apple and fruits[]=banana, the whole Node process crashes with this stack trace:

RangeError: Maximum call stack size exceeded
    at Function.keys (<anonymous>)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:193:22)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)
    at reconcile ([redacted]/node_modules/async-busboy/index.js:202:12)

There are 2 problems here. I propose the following solutions:

  1. There should be no process crash, in any circumstances. Upon any error, even internal, the exception should be thrown. It seams that the underlying busboy module poorly handles errors inside registered event handlers.
    Suggested solution: wrap any internal field and file processing function with a try..catch, which would reject the main promise.
  2. This use case is very common and should be handled properly. And making your own parser can be difficult and sometimes even unsafe.
    I think, in general, TJ’s qs module could (and maybe even should) be used here for robust field parsing and aggregating. Off the top of my head:
const qs = require('qs')
const fieldList = [
	{name: 'fruits[]', value: 'apple'},
	{name: 'fruits[]', value: 'banana'},
]
const fields = qs.parse(fieldList
	.map(({name, value}) => encodeURIComponent(name) + '=' + encodeURIComponent(value))
	.join('&'))

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions