commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Neidhart <thomas.neidh...@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 20:44:21 GMT
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. If you too think catching Exception would
be sufficient than let's change it to that.

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


Mime
View raw message