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

Return error for multipart file exceeding maxBytes #26

Merged
merged 4 commits into from Apr 6, 2016

Conversation

@johnbrett
Copy link
Contributor

johnbrett commented Apr 2, 2016

First part of fix for: hapijs/hapi#2825

johnbrett added 2 commits Apr 2, 2016
@johnbrett johnbrett changed the title Return error for multipart file outputs too big Return error for multipart file exceeding maxBytes Apr 2, 2016
if (this.settings.maxBytes !== undefined) {
let bytesUsed = 0;

source.on('data', (data) => {

This comment has been minimized.

Copy link
@Marsup

Marsup Apr 2, 2016

Member

Isn't this triggering old streams push mode ?

@hueniverse

This comment has been minimized.

Copy link
Member

hueniverse commented Apr 3, 2016

Wouldn't it be better to perform this at the pez level? This doesn't actually stops processing, just bails faster.

@cjihrig

This comment has been minimized.

Copy link
Contributor

cjihrig commented Apr 3, 2016

This doesn't actually stops processing, just bails faster.

I haven't tested, but couldn't you just unpipe() the dispenser? I think you might want to do something similar on timeout too.

@hueniverse

This comment has been minimized.

Copy link
Member

hueniverse commented Apr 3, 2016

We need to flush our the stream but also to stop the processor from trying to finish the job. It would be much cleaner if we just passed the option down the stack instead of hacking it here.

@cjihrig

This comment has been minimized.

Copy link
Contributor

cjihrig commented Apr 6, 2016

@johnbrett I moved this to hapijs/pez#9. I'll publish a new version, and you should be able to pass maxBytes to the pez Dispenser constructor.

@johnbrett

This comment has been minimized.

Copy link
Contributor Author

johnbrett commented Apr 6, 2016

I'd meant to ask how we'd tackle this today :) Nice one @cjihrig, ready to make the update when the updated pez version is published

@cjihrig

This comment has been minimized.

Copy link
Contributor

cjihrig commented Apr 6, 2016

OK, pez@2.1.0 is out.

@johnbrett johnbrett self-assigned this Apr 6, 2016
@johnbrett johnbrett merged commit 8ec84a5 into master Apr 6, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@johnbrett johnbrett added the bug label Apr 6, 2016
@johnbrett johnbrett added this to the 4.0.1 milestone Apr 6, 2016
@johnbrett johnbrett deleted the hapi-issues-2825 branch Apr 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.