You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Eric Hermanson <er...@alum.mit.edu> on 2007/08/30 05:04:21 UTC

FileUploadBase.setFileSizeMax & MultipartStream.ItemInputStream.makeAvailable()

setFileSizeMax is apparently intended to prevent the file upload if  
the file about to be uploaded is larger than a given size.  However,  
the exception does not get processed until AFTER the entire file is  
uploaded to the server (thereby defeating the purpose of the limit).   
I went into the debugger and noticed that

	MultipartStream.ItemInputStream.close()

is calling

	MultipartStream.ItemInputStream.makeAvailable()

and the call to InputStream.read() in that method is actually what's  
causing the entirety of the oversized file upload data to be  
incorrectly read.  The FileUploadBase.FileSizeLimitExceededException  
is correctly being raised, but the subsequent buffer read during  
close is resulting in all of the data being read regardless.  This  
seems like a bug to me, but I am not all that familiar with the code.




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


Re: FileUploadBase.setFileSizeMax & MultipartStream.ItemInputStream.makeAvailable()

Posted by Jochen Wiedmann <jo...@gmail.com>.
On 8/30/07, Eric Hermanson <er...@alum.mit.edu> wrote:

> exception.  However, the exception isn't getting *processed* until
> AFTER all of the data is read.  This is because of what appears to be
> a bug in MultipartStream.ItemInputStream.close().

Ah, I undestand now. It's not a bug, it's a ... most possibly the
exception is catched by the handler that intends to ensure proper
release of resources.

File a bug in Jira, please.


-- 
"Besides, manipulating elections is under penalty of law, resulting in
a preventative effect against manipulating elections.

The german government justifying the use of electronic voting machines
and obviously  believing that we don't need a police, because all
illegal actions are forbidden.

http://dip.bundestag.de/btd/16/051/1605194.pdf

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


Re: FileUploadBase.setFileSizeMax & MultipartStream.ItemInputStream.makeAvailable()

Posted by Eric Hermanson <er...@alum.mit.edu>.
As I mentioned, the LimitedInputStream *IS* correctly raising the  
exception.  However, the exception isn't getting *processed* until  
AFTER all of the data is read.  This is because of what appears to be  
a bug in MultipartStream.ItemInputStream.close().  After the  
LimitedInputStream correctly raises the file size exception, someone  
attempts to close the MultipartStream, but the close() method  
repeatedly calls 'makeAvailable()' which ends up reading the rest of  
the file upload data anyways, REGARDLESS of the size limit exception  
being raised.  This is clearly a bug, is it not?

Please do correct me if I'm wrong.

- Eric


On Aug 30, 2007, at 2:13 AM, Jochen Wiedmann wrote:

> On 8/30/07, Eric Hermanson <er...@alum.mit.edu> wrote:
>
>> setFileSizeMax is apparently intended to prevent the file upload if
>> the file about to be uploaded is larger than a given size.  However,
>> the exception does not get processed until AFTER the entire file is
>> uploaded to the server (thereby defeating the purpose of the limit).
>
> That's not the case. If you are setting this property, then the file
> items input stream is wrapped by an instance of LimitedInputStream.
> That instance raises the exception as soon as more than fileSizeMax
> bytes are returned to the caller. It may certainly look so, if the
> uploaded file is, for example, 10.5 MB and the limit is 10MB. Apart
> from that, it is quite possible that the exception is ignored by the
> browser and the browser continues to shuffle data to the server. But
> that's nothing we can control.
>
> I admit, that an additional improvement could be made: We might check,
> if the file items headers (as opposed to the request headers) contain
> a content-length entry, which exceeds the limit. However, I admit that
> this would help, because I've seen few cases where the item does
> contain a content-length header. Patches welcome, though.
>
>> I went into the debugger and noticed that
>>
>>         MultipartStream.ItemInputStream.close()
>>
>> is calling
>>
>>         MultipartStream.ItemInputStream.makeAvailable()
>>
>> and the call to InputStream.read() in that method is actually what's
>> causing the entirety of the oversized file upload data to be
>> incorrectly read.
>
> This may be the case, if you haven't read all the data before invoking
> the close() method.
>
>
> Jochen
>
>
> -- 
> "Besides, manipulating elections is under penalty of law, resulting in
> a preventative effect against manipulating elections.
>
> The german government justifying the use of electronic voting machines
> and obviously  believing that we don't need a police, because all
> illegal actions are forbidden.
>
> http://dip.bundestag.de/btd/16/051/1605194.pdf
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>


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


Re: FileUploadBase.setFileSizeMax & MultipartStream.ItemInputStream.makeAvailable()

Posted by Jochen Wiedmann <jo...@gmail.com>.
On 8/30/07, Eric Hermanson <er...@alum.mit.edu> wrote:

> setFileSizeMax is apparently intended to prevent the file upload if
> the file about to be uploaded is larger than a given size.  However,
> the exception does not get processed until AFTER the entire file is
> uploaded to the server (thereby defeating the purpose of the limit).

That's not the case. If you are setting this property, then the file
items input stream is wrapped by an instance of LimitedInputStream.
That instance raises the exception as soon as more than fileSizeMax
bytes are returned to the caller. It may certainly look so, if the
uploaded file is, for example, 10.5 MB and the limit is 10MB. Apart
from that, it is quite possible that the exception is ignored by the
browser and the browser continues to shuffle data to the server. But
that's nothing we can control.

I admit, that an additional improvement could be made: We might check,
if the file items headers (as opposed to the request headers) contain
a content-length entry, which exceeds the limit. However, I admit that
this would help, because I've seen few cases where the item does
contain a content-length header. Patches welcome, though.

> I went into the debugger and noticed that
>
>         MultipartStream.ItemInputStream.close()
>
> is calling
>
>         MultipartStream.ItemInputStream.makeAvailable()
>
> and the call to InputStream.read() in that method is actually what's
> causing the entirety of the oversized file upload data to be
> incorrectly read.

This may be the case, if you haven't read all the data before invoking
the close() method.


Jochen


-- 
"Besides, manipulating elections is under penalty of law, resulting in
a preventative effect against manipulating elections.

The german government justifying the use of electronic voting machines
and obviously  believing that we don't need a police, because all
illegal actions are forbidden.

http://dip.bundestag.de/btd/16/051/1605194.pdf

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