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

Only count files (not all form elements) against the Multipart File Limit #814

Conversation

johnnaegle
Copy link
Contributor

We were getting "Too many open files - Maximum file multiparts in content reached". However, our form only had one file input. It had several hundred other form elements. Each was counted as a file.

Only increment the opened_files counter for file inputs. If you have a large form, you can quickly hit the MultipartPartLimitError, even though you aren't actually opening files

@johnnaegle
Copy link
Contributor Author

Has anybody looked at this? It seems like quite the bug to count all fields as files when parsing multipart requests.

@tenderlove
Copy link
Member

Hesitant to pull this in because I know the limit is to do with a security issue. I'm not sure where the file handles are actually created, so I don't know if this is an OK change to make. @spastorino can you take a look? Seems like you signed off on b0b5fb9

@spastorino
Copy link
Contributor

Can you squash your commits?

large form with only one file, you can quickly hit the MultipartPartLimitError
Add test for not hitting the multipart limit
@johnnaegle johnnaegle force-pushed the only_increment_open_file_count_for_fileparts branch from 3611741 to 7331561 Compare March 9, 2015 17:42
@johnnaegle
Copy link
Contributor Author

squashed

spastorino added a commit that referenced this pull request Mar 11, 2015
…t_for_fileparts

Only count files (not all form elements) against the Multipart File Limit
@spastorino spastorino merged commit 7a3405f into rack:master Mar 11, 2015
@johnnaegle johnnaegle deleted the only_increment_open_file_count_for_fileparts branch March 11, 2015 18:25
@ZachBeta
Copy link

ZachBeta commented Apr 8, 2015

We're running into this issue as well. Any thoughts on when 1.6.1 will be released?

djcp added a commit to djcp/milk_steak that referenced this pull request Apr 16, 2015
boffbowsh added a commit to alphagov/whitehall that referenced this pull request May 15, 2015
This fixes a bug where all form fields in a request are counted against the multipart file limit:
rack/rack#814

This is an issue in production - some publications and organisations have a large number of dynamically added nested forms. When the form is submitted, a 500 is returned because the number of fields exceeds the limit. It only affects forms posted with multipart, e.g. ones that contain file fields.

Zendesk: https://govuk.zendesk.com/agent/tickets/1037587
Story: https://www.pivotaltracker.com/story/show/94668330
boffbowsh added a commit to alphagov/whitehall that referenced this pull request May 15, 2015
This fixes a bug where all form fields in a request are counted against the multipart file limit:
rack/rack#814

This is an issue in production - some publications and organisations have a large number of dynamically added nested forms. When the form is submitted, a 500 is returned because the number of fields exceeds the limit. It only affects forms posted with multipart, e.g. ones that contain file fields.

Zendesk: https://govuk.zendesk.com/agent/tickets/1037587
Story: https://www.pivotaltracker.com/story/show/94668330
@sjain
Copy link

sjain commented May 22, 2015

The fix seems to be in master, but not in version 1.6.1. When is next version expected?

@paneq
Copy link

paneq commented Jun 17, 2015

Could you backport this to 1.5 and release?

spastorino added a commit that referenced this pull request Jun 17, 2015
…t_for_fileparts

Only count files (not all form elements) against the Multipart File Limit
Conflicts:
	lib/rack/multipart/parser.rb
spastorino added a commit that referenced this pull request Jun 17, 2015
…t_for_fileparts

Only count files (not all form elements) against the Multipart File Limit
Conflicts:
	lib/rack/multipart/parser.rb
spastorino added a commit that referenced this pull request Jun 17, 2015
…t_for_fileparts

Only count files (not all form elements) against the Multipart File Limit
@kuahyeow
Copy link

We have just hit this issue when Nginx (with Passenger) started erroring with a 502 with the error message:

upstream prematurely closed connection while reading response header from upstream

Turning on debug log level pointed to the Rack::Multipart::MultipartLimitError, which was announced recently as a security release for 1.5.4

We could workaround by setting the RACK_MULTIPART_PART_LIMIT env var for now, but it would be nice to release a proper fix for this which doesn't count non-files towards the limit ?

EDIT: 👍

@gchan
Copy link

gchan commented Jun 18, 2015

Thanks for resolving this issue and releasing new versions so promptly! @johnnaegle @tenderlove @spastorino ❤️

@wnm
Copy link

wnm commented Jul 19, 2017

I'm also getting a "Too many open files - Maximum file multiparts in content reached" error.

I'm using rails's accepts_nested_attributes_for to edit multiple entries in one huge form.

So there are multiple input fields on one page, but even if none of them is beeing changed (an image uploaded) I still get the error.

Any ideas on how to fix that?

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

Successfully merging this pull request may close these issues.

None yet

9 participants