On 2 December 2014 at 20:44, Thomas Neidhart <thomas.neidh...@gmail.com> wrote:
> On 12/02/2014 06:35 PM, sebb wrote:
>> On 11 November 2014 at 20:09, <t...@apache.org> wrote:
>>> Author: tn
>>> Date: Tue Nov 11 20:09:32 2014
>>> New Revision: 1638340
>>>
>>> URL: http://svn.apache.org/r1638340
>>> Log:
>>> [FILEUPLOAD-242] Do not silently swallow all Throwables.
>>>
>>> Modified:
>>> commons/proper/fileupload/trunk/src/changes/changes.xml
>>>
>>> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java
>>>
>>> Modified: commons/proper/fileupload/trunk/src/changes/changes.xml
>>> URL:
>>> http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/changes/changes.xml?rev=1638340&r1=1638339&r2=1638340&view=diff
>>> ==============================================================================
>>> --- commons/proper/fileupload/trunk/src/changes/changes.xml (original)
>>> +++ commons/proper/fileupload/trunk/src/changes/changes.xml Tue Nov 11
>>> 20:09:32 2014
>>> @@ -44,6 +44,7 @@ The <action> type attribute can be add,u
>>>
>>> <body>
>>> <release version="1.4" date="TBA" description="TBA">
>>> + <action issue="FILEUPLOAD-242" dev="tn" type="fix">FileUploadBase -
>>> should not silently catch and ignore all Throwables</action>
>>> <action issue="FILEUPLOAD-257" dev="tn" type="fix">Fix Javadoc 1.8.0
>>> errors</action>
>>> <action issue="FILEUPLOAD-234" dev="tn" type="fix">Fix section
>>> "Resource cleanup" of the user guide</action>
>>> <action issue="FILEUPLOAD-237" dev="tn" type="fix">Fix streaming
>>> example: use FileItem.getInputStream() instead of openStream()</action>
>>>
>>> Modified:
>>> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java
>>> URL:
>>> http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java?rev=1638340&r1=1638339&r2=1638340&view=diff
>>> ==============================================================================
>>> ---
>>> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java
>>> (original)
>>> +++
>>> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java
>>> Tue Nov 11 20:09:32 2014
>>> @@ -366,8 +366,8 @@ public abstract class FileUploadBase {
>>> for (FileItem fileItem : items) {
>>> try {
>>> fileItem.delete();
>>> - } catch (Throwable e) {
>>> - // ignore it
>>> + } catch (Throwable t) {
>>> + handleThrowable(t);
>>
>> In this case, surely it would make more sense to just catch Exception?
>>
>> I agree that it is vital that TD and VME are not ignored, but other
>> Throwables should be not be ignored either here.
>
> I thought about that too but it was very unclear why it was catching
> Throwable in the first place.
It may just have been laziness.
> If you too think catching Exception would
> be sufficient than let's change it to that.
Yes, I think that would be enough.
Though I wonder why it does not report problems deleting the files.
AFAICT the FileCleaningTracker keeps track of files that it cannot delete.
> Thomas
>
>>
>>> }
>>> }
>>> }
>>> @@ -375,6 +375,25 @@ public abstract class FileUploadBase {
>>> }
>>>
>>> /**
>>> + * Checks whether the supplied Throwable is one that needs to be
>>> + * rethrown and swallows all others.
>>> + * @param t the Throwable to check
>>> + */
>>> + private void handleThrowable(Throwable t) {
>>> + if (t instanceof ThreadDeath) {
>>> + throw (ThreadDeath) t;
>>> + }
>>> + if (t instanceof StackOverflowError) {
>>> + // Swallow silently - it should be recoverable
>>> + return;
>>> + }
>>> + if (t instanceof VirtualMachineError) {
>>> + throw (VirtualMachineError) t;
>>> + }
>>> + // All other instances of Throwable will be silently swallowed
>>> + }
>>> +
>>> + /**
>>> * Processes an <a href="http://www.ietf.org/rfc/rfc1867.txt";>RFC
>>> 1867</a>
>>> * compliant <code>multipart/form-data</code> stream.
>>> *
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>> For additional commands, e-mail: dev-h...@commons.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
