Feature #8370: Constants MAX_MULTIPART_LENGTH in cgi/core.rb

• Status: Closed
• Assignee: xibbar (Takeyuki FUJIOKA)
• Target version: 2.2.0

[xibbar (Takeyuki FUJIOKA)]

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.rb

The 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

[xibbar (Takeyuki FUJIOKA)]

• Description updated (diff)

[nobu (Nobuyoshi Nakada)]

• Description updated (diff)

[hsbt (Hiroshi SHIBATA)]

• Target version changed from 2.1.0 to 2.2.0

[leriksen (Leif Eriksen)]

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

[mame (Yusuke Endoh)]

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 [e-mail]

[leriksen (Leif Eriksen)]

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 ?

[xibbar (Takeyuki FUJIOKA)]

Do you want to set the value of MAX_MULTIPART_LENGTH every request ?
I think request length is only one value in web server.

[leriksen (Leif Eriksen)]

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.

[nobu (Nobuyoshi Nakada)]

• Subject changed from Constants MAX_MULTIPART_LENGTH in cgi\core.rb to Constants MAX_MULTIPART_LENGTH in cgi/core.rb
• Description updated (diff)

[leriksen (Leif Eriksen)]

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

[nobu (Nobuyoshi Nakada)]

• Tracker changed from Bug to Feature

Adding new methods is a feature request.

[leriksen (Leif Eriksen)]

OK I'll remove the getter/setters and resubmit.

[leriksen (Leif Eriksen)]

https://github.com/ruby/ruby/pull/633

[nobu (Nobuyoshi Nakada)]

I meant "adding new feature".

And you didn't have to resubmit, pushing new changeset should be enough.

[leriksen (Leif Eriksen)]

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.

[xibbar (Takeyuki FUJIOKA)]

• 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 [e-mail]