Feature #8370
closedConstants MAX_MULTIPART_LENGTH in cgi/core.rb
Description
Reported by Andreas Kraus via gmail.
hi xibbar,
I try to understand why the
MAX_MULTIPART_LENGTH
is a Constant and i can't change it.
If i uload a Multipart file which is larger than 128 MB raise an error "too large multipart data.",
but why i can't change this value to upload larger files.The Constant comes with this Change:
https://github.com/ruby/ruby/commit/10e9b638069d9e40233242693814b86c672e423e#lib/cgi/core.rbThe only sense i see, is that the Author of cgialt uses max 128MB files und build in this Constant ...
I would like to know why this constant is in place and how to change it's behaviour.
My requirement is to upload files larger than the given limit of 128MB.regards,
Andreas
Updated by xibbar (Takeyuki FUJIOKA) over 11 years ago
- Description updated (diff)
Updated by nobu (Nobuyoshi Nakada) over 11 years ago
- Description updated (diff)
Updated by hsbt (Hiroshi SHIBATA) over 10 years ago
- Target version changed from 2.1.0 to 2.2.0
Updated by leriksen (Leif Eriksen) about 10 years ago
From my reading of RFC-1867, this constant/constraint is not required.
If a server detects space constraints for an upload, it can terminate the connection at any time.
Servers can indicate a maximum length via the MAXLENGTH attribute, but clients are not required to limit themselves to it.
Clients are generally required to supply an overall content-length for the upload - servers can act on that at the commencement of transmission, or terminate at a later time according to whatever policy has been set up.
There is no reason, that I can see, to artificially limit the upload size on the client side, the server can accept or reject at any time.
I will raise a git pull request from my commit https://github.com/leriksen/ruby/commit/77f3a6bbb92e3f395851e7998556d7df160f4da1
Updated by mame (Yusuke Endoh) about 10 years ago
There is no reason, that I can see, to artificially limit the upload size on the client side
I think that MAX_MULTIPART_LENGTH is the limit on the server side.
The limit itself is actually required to prevent shortage of server resource.
I agree that the limit should be configuable by users.
In fact, the author of cgialt, Makoto Kuwata, said in the proposal,
http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-dev/33606
簡単化のため制限値は定数で指定してますが、必要であればクラス変数や
インスタンス変数で指定できるようにしてください。
For simplicity, the limit is specified by a constant,
but if needed, please use class variable or instance variable
to specify it.
So, what we need is not a patch that just removes the check,
but a patch that enables a user to configure it.
--
Yusuke Endoh mame@ruby-lang.org
Updated by leriksen (Leif Eriksen) about 10 years ago
OK, yeah I got the wrong end of the conversation - CGI is all about the server side.....
OK an idea comes to mind
A new option in the options_hash in the constructor - currently only :tag_maker and :accept_charset are defined
The multipart handling is in the private method initialize_query(), which is only referenced in the constructor. So the only opportunity to set this up is from CGI.new.
I propose a new option in the initialize method - :max_multipart_length
It will default to the old value of 128 *1024 *1024 bytes, and can take as a value either a simple integer scalar, or a lambda. A lambda will allow the user to use more complex logic than a simple integer to determine the size of multipart forms to accept.
Would that work ?
Updated by xibbar (Takeyuki FUJIOKA) about 10 years ago
Do you want to set the value of MAX_MULTIPART_LENGTH every request ?
I think request length is only one value in web server.
Updated by leriksen (Leif Eriksen) about 10 years ago
Well I'm only trying to solve the problem in the CGI library, not the underlying web server, and the only place that will reference MAX_MULTIPART_LENGTH is in the request parsing. That happens only in the CGI constructor. I dont think I should stray too far from that use-case.
If a user does not specify a MAX_MULTIPART_LENGTH in the constructor, it will default to 128 * 1024 *1024 bytes. Otherwise a user can specify a new length, or a lambda that will calculate it at parse time (say there is a need to query the file system, or get a size from a users account etc).
I will add a getter/setter pair, though I cant think of a use-case for them. But Makoto did mention using an instance variable, so might as well complete that part.
I'll go ahead with this plan, and note the commit/pull-request in my next comment.
Updated by nobu (Nobuyoshi Nakada) about 10 years ago
- Subject changed from Constants MAX_MULTIPART_LENGTH in cgi\core.rb to Constants MAX_MULTIPART_LENGTH in cgi/core.rb
- Description updated (diff)
Updated by leriksen (Leif Eriksen) about 10 years ago
OK done.
Used the pattern for passing accept_charset, which can be read/modified by a class method, and read by an instance method. That seems reasonable, let me know if a setter instance methods is also required, will do as a separate piece.
I updated the multipart tests to also take an options hash, to make constructing the CGI instance under test a little easier.
Pull request - https://github.com/ruby/ruby/pull/632
Commit - https://github.com/leriksen/ruby/commit/edcd051115165a2cafbf8892081418c621201c37
Updated by nobu (Nobuyoshi Nakada) about 10 years ago
- Tracker changed from Bug to Feature
Adding new methods is a feature request.
Updated by leriksen (Leif Eriksen) about 10 years ago
OK I'll remove the getter/setters and resubmit.
Updated by leriksen (Leif Eriksen) about 10 years ago
Updated by nobu (Nobuyoshi Nakada) about 10 years ago
I meant "adding new feature".
And you didn't have to resubmit, pushing new changeset should be enough.
Updated by leriksen (Leif Eriksen) about 10 years ago
OK noted - just trying to be super polite. Please lt me know if there is anything else I need to complete or consider for this fix.
Updated by xibbar (Takeyuki FUJIOKA) about 10 years ago
- Status changed from Open to Closed
- % Done changed from 0 to 100
Applied in changeset r46392.
- lib/cgi/core.rb: Provide a mechanism to specify the
max_multipart_length of multipart data.
[Feature #8370] patch by Leif Eriksen leif.eriksen.au@gmail.com