You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by bu...@apache.org on 2006/04/14 20:19:05 UTC

DO NOT REPLY [Bug 39316] New: - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316

           Summary: CommonsMultipartRequestHandler will lose parameters when
                    a uploading file is too big
           Product: Struts
           Version: 1.2.9
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Apps
        AssignedTo: dev@struts.apache.org
        ReportedBy: wshao@perpetual.com


I am trying to implement an upload feature for our website by using struts. 
Everything works fine except when I am trying to upload a file with size bigger
than I configured in the struts-config, all the parameters get lost after struts
decides the file size is too big.  Since I am trying to redirect the user to a
page with some specific IDs, I will need some parameters to be replayed.  So I
followed this class's code and tried to see how it gets lost.  Here is wut happens:

in the function handleRequest(HttpServletRequest request) at line 166:
        try {
            items = upload.parseRequest(request);
        } catch (DiskFileUpload.SizeLimitExceededException e) {
            // Special handling for uploads that are too big.
            request.setAttribute(
                    MultipartRequestHandler.ATTRIBUTE_MAX_LENGTH_EXCEEDED,
                    Boolean.TRUE);
            return;
        }

So when the class get an SizeLimitExceededException from DiskFileUpload, it will
not try to replay the rest of the parameters.  And as a result, I lost the
parameter I passed in for action class to redirect the response.  Think it
through, I feel it's a struts bug rather than a commons file upload bug. 
Because commons file upload should throw an exception when the file is too big
and struts still should replay the rest of the parameters and skip the file
parameters.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316





------- Additional Comments From martinc@apache.org  2006-04-15 00:52 -------
I'm not trying to sound superior, and if I am coming across that way, I
apologise. However, I'd say that comments of your own such as "your current way
of implementation is not serving that purpose.  Please try and validate it." and
"Timewisely, it doesn't look like what you said is valid." haven't helped the
tone of the conversation either. ;-)

If you explore the slowness issue, you will find that the delay does not occur
in FileUpload or in Struts. It is a container issue over which neither code base
has any control.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316





------- Additional Comments From martinc@apache.org  2006-04-14 22:56 -------
The code in comment #3 does not make sense. The exception, and the sizeMax value
you are referring to, apply to the _request_ size, not the size of a particular
uploaded item. So, the exception will be thrown from parseRequest() if the
request exceeds sizeMax. In that situation, you will never reach the loop,
unless sizeMax is -1, and you will certainly never encounter a situation in
which an individual item is larger than the permitted size for the entire request.

If you set sizeMax to -1, _everything_ will be uploaded to the server before the
loop ever executes. Everything. All of the parts, all of the files, regardless
of how large they are. That is what the size exception is designed to protect
your app from.

Could Struts provide a means of preventing your app from seeing items over a
given size? Yes. But it cannot stop those items from being uploaded to your
server first, which usually defeats the purpose entirely. To do that requires
changes to Commons FileUpload, so that it skips data over a (different) maximum
size for a particular item.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316


wshao@perpetual.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED




------- Additional Comments From wshao@perpetual.com  2006-04-15 00:39 -------
Martin, I really don't get why your manner is so superior.  This doesn't really
help you to look like a superior figure or super engineer.  I am not saying that
exception is not being thrown right after the upload starts.  All I am saying is
that it's very slow.  And with your implementation, I still wait long time to
see an error message - An ERROR message, not an exception.  I don't really care
how this is all working behind commons fileupload coz it's supposed to be
unknown to me.  Even how Struts' implemented is supposed to be unknown to me. 
Thanks for your hint.  I am already using the ServletFileUpload instead of
DiskFileUpload in the multipart class and I think you can at least fix some
deprecated code by using the latest common fileupload.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316





------- Additional Comments From wshao@perpetual.com  2006-04-15 00:12 -------
So I just tested to upload a 1 Gb file with DiskFileUpload's sizeMax set to be
1Mb.  I am not sure if the exception is thrown after the uploading passing 1Mb.
But it takes really long time till I get a error message.

Let's go back to this size controlling.  If your concern is that the configged
max size should apply to request's size and not to a single file's size, that's
fine to me.  Still, it's not hard to add up all items' sizes, which should be
very close to request's size.  If you concern is that the request should be
stopped before too much data was uploaded, it seems to me that your current way
of implementation is not serving that purpose.  Please try and validate it.  My
testing might not be valid, since I just tested it on my local machine and I
don't want to go further and track down Commons FileUpload source code, plus I
don't want to support multiple file boxes in one form either.  Timewisely, it
doesn't look like what you said is valid.  It might be shorter than uploading
the whole thing, but it still comsumes a lot of bandwidth.

The thing is that by using this CommonsMultipartRequestHandler, if the request
size passes the size set in struts-config, I will lose all field information and
this is where the painth comes from - all my parameters were set to 0 or
nothing.  Of course, nothing cannot be achieved without using struts.  Nobody
needs struts to breathe.  People just love struts' for its convenience.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316





------- Additional Comments From martinc@apache.org  2006-04-15 00:28 -------
The size exception is thrown as soon as parseRequest() is called, because it is
based on the HTTP header value. The actual parsing of the request never even
starts. The delay you are seeing is a container issue, and not something either
FileUpload or Struts can do anything about.

I've already said that there is an open issue against Commons FileUpload to
provide a better solution for aborted uploads, and this issue has been flagged
as a duplicate of that issue. Unless you set sizeMax to -1, which is setting a
Commons FileUpload parameter, there is no possible way for you to get partial
results without changes to Commons FileUpload. It doesn't matter whether you're
using Struts or something else - FileUpload still behaves that way.

I'm afraid that if you want to know the finer details of how all this works, you
are going to have to "track down" the FileUpload code. I frankly don't have the
time to keep explaining it one paragraph at a time in the middle of an
acknowledged enhancement request. But I will give you a hint:

http://jakarta.apache.org/site/downloads/downloads_commons-fileupload.cgi


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316





------- Additional Comments From wshao@perpetual.com  2006-04-14 21:03 -------
Well, I understand what you are saying.  But why not let Commons FileUpload pass
this item to struts and let struts decide it's a oversized or not?

So I just write a new CommonsMultipartRequestHandler (since this
CommonsMultipartRequestHandler is not in a nutshell) in our own package and do this:

// set maxsize to no limit
upload.setSizeMax(-1);

...

// check the file size, if the file size is greater than the size
// in moduleconfig, set the flag and forward other parameters
        while (iter.hasNext()) {
            FileItem item = (FileItem) iter.next();

            if (item.isFormField()) {
                addTextParameter(request, item);
            } else {
                long size = item.getSize();
                if (size > getSizeMax(ac)) {
                   
request.setAttribute(MultipartRequestHandler.ATTRIBUTE_MAX_LENGTH_EXCEEDED,
Boolean.TRUE);
                    continue;
                }
                
                addFileParameter(item);
            }
        }

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316





------- Additional Comments From wshao@perpetual.com  2006-04-14 18:50 -------
More to this class.  I am trying to override
CommonsMultipartRequestHandler.handleRequest(HttpServletRequest request) by
creating a subclass.  And I am having a bad time since there are 3 private
members (elementsText, elementsFile, elementsAll) are initialized in this
function.  As a result, I have to copy and paste pretty much everything from
this class.  Default scope class CommonsFormFile is not visible to the sub
class, too.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316





------- Additional Comments From wshao@perpetual.com  2006-04-14 23:12 -------
Well, I already consider that situation.  Yes, the file will be uploaded to the
temp dir, no matter how large it is.  But I don't really think it's going to be
a big issue for, at least for our web application.  Coz we specified the size
limit before the users upload, we told them it's not going to pass and I can
barely imagine some1 will upload a super huge file by eating his/her own upload
bandwidth, considering currently most Cable/DSL still don't have a good upload
speed.  And if it's a hacker who wants to consume our bandwidth.  Well, we got
nothing to stop him with this struts MultiPart class.  He/she can write a bot to
keep uploading, no matter how big the size limit is out there, 1000 times of 1Mb
is still 1 Gb.  Comparing to the malfunctioning class, I think this is more
reasonable to our web application.

For you, of course, you have to think about serving more general purpose.  But I
really doubt this is a more concern than a malfunctioning class.  Comparing pros
and cons, I think this way is gain.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316





------- Additional Comments From martinc@apache.org  2006-04-14 23:24 -------
If all you want is to check the size of items _after_ they have been uploaded,
and you're OK with setting sizeMax to -1, which you say you are, then you don't
need anything further from Struts in the first place. All you need to do is put
the size checking loop in your Action, after FileUpload is done. That's it. Done.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316


martinc@apache.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |DUPLICATE




------- Additional Comments From martinc@apache.org  2006-04-14 19:03 -------
I really don't understand what you mean by "replaying" parameters. It's a file
upload, not a movie. ;-)

In any case, it _is_ an issue - and a known one - with Commons FileUpload. When
the size exception is thrown from FileUpload, there is no way for a client to
obtain the fields that have been parsed up to that point. And since the input
stream has been consumed, there is no way that any other code (e.g. in Struts)
can re-parse that stream some other way to extract the fields.

Marking as a duplicate of #38936, which also calls for a way to obtain those
parameters that have already been parsed in the face of problems completing the
parse.

*** This bug has been marked as a duplicate of 38936 ***

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316





------- Additional Comments From wshao@perpetual.com  2006-04-15 01:13 -------
Sorry for bad English.  Well, I noticed my test is not necessary valid for not
too much time I can put on this.  But still, combining all the situations and
practical user facts, I still think for current Commons FileUpload
implementation, it might be better for struts not to catch that exception,
because not many people will want to upload files others won't accept.  Of
course, stopping oversizing files at runtime is cool.  But comparing to losing
all request fields, I think more struts users would like to let web users to
wait.  Anyway, it's your call.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


DO NOT REPLY [Bug 39316] - CommonsMultipartRequestHandler will lose parameters when a uploading file is too big

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39316>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39316





------- Additional Comments From jochen@apache.org  2006-08-07 00:19 -------
Note, that the reporters issues may now be handled by disabling sizeMax and
using the streaming API.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org