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.

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

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

Open
pepkin88 opened this issue Jan 16, 2021 · 0 comments
Open

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

pepkin88 opened this issue Jan 16, 2021 · 0 comments

Comments

@pepkin88
Copy link

pepkin88 commented Jan 16, 2021

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('&'))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant