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. 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-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
