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

Invalid busboy behavior on incomplete multipart #73

Closed
thomas-riccardi opened this issue Mar 3, 2015 · 6 comments
Closed

Invalid busboy behavior on incomplete multipart #73

thomas-riccardi opened this issue Mar 3, 2015 · 6 comments

Comments

@thomas-riccardi
Copy link

With busboy 0.2.9 when piping an incomplete multipart to busboy the last (incomplete) part is emitted on source end (field emitted, or file stream end emitted), then busboy stops emitting events (no file end, no finish, no error).
It should instead emit an error: the multipart is invalid (because incomplete) and should not emit the current part, because it's not finished.

head commit e70d9e4 fixes part of this issue: now busboy emits an error (Error: Unexpected end of multipart data), but the current incomplete part is still emitted.
For field events we should just not emit them and emit error on busboy.
For file events we should emit error on file, on busboy or on both, I'm not sure which.
Busboy also emits finish, I'm not sure if we want that in addition to the error event.

I'm not sure how to properly handle errors in a pipe involving multiple streams (in my case: http request incoming message, busboy, aws-sdk S3 upload; the S3 upload needs an explicit abort on source error, and the http request needs a reply): busboy readme doesn't show any error handling example.

@mscdex
Copy link
Owner

mscdex commented Apr 5, 2015

I'm always open to pull requests to improve busboy.

@rbriank
Copy link

rbriank commented Jul 9, 2017

I've seen this problem too. My workaround is to add a flag to the file end event and check it in the busboy finish event. If it's not set, I know file end was never called so I can reject.

@FranckFreiburger
Copy link

ping !

@tomasdev
Copy link

@thomas-riccardi did you fix it? what was your workaround?

@thomas-riccardi
Copy link
Author

@tomasdev Sorry I don't remember, it was too long ago, I stopped using node.js since.

@mscdex
Copy link
Owner

mscdex commented Dec 19, 2021

Closing this for now with the new implementation in master and with changes to node streams in general in modern versions of node. 'close' is now the event to listen for whether there was an error or not and 'finish' should only be emitted if there was no error.

@mscdex mscdex closed this as completed Dec 19, 2021
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

5 participants