commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1638340 - in /commons/proper/fileupload/trunk/src: changes/changes.xml main/java/org/apache/commons/fileupload/FileUploadBase.java
Date Tue, 02 Dec 2014 22:20:12 GMT
On 2 December 2014 at 20:44, Thomas Neidhart <thomas.neidhart@gmail.com> wrote:
> On 12/02/2014 06:35 PM, sebb wrote:
>> On 11 November 2014 at 20:09,  <tn@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-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
>

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


Mime
View raw message