You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2014/12/02 18:13:18 UTC
Re: svn commit: r1638380 - in /commons/proper/fileupload/trunk/src:
changes/changes.xml main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
On 11 November 2014 at 20:45, <tn...@apache.org> wrote:
> Author: tn
> Date: Tue Nov 11 20:45:16 2014
> New Revision: 1638380
>
> URL: http://svn.apache.org/r1638380
> Log:
> [FILEUPLOAD-252] Do not lose IO exception when closing the stream.
>
> Modified:
> commons/proper/fileupload/trunk/src/changes/changes.xml
> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.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=1638380&r1=1638379&r2=1638380&view=diff
> ==============================================================================
> --- commons/proper/fileupload/trunk/src/changes/changes.xml (original)
> +++ commons/proper/fileupload/trunk/src/changes/changes.xml Tue Nov 11 20:45:16 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-252" dev="tn" type="fix">DiskFileItem#write() could lose original IO exception</action>
> <action issue="FILEUPLOAD-258" dev="tn" type="fix">DiskFileItem#getStoreLocation() wrongly returned a File object for items stored in memory</action>
> <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>
>
> Modified: commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
> URL: http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java?rev=1638380&r1=1638379&r2=1638380&view=diff
> ==============================================================================
> --- commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java (original)
> +++ commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java Tue Nov 11 20:45:16 2014
> @@ -389,9 +389,7 @@ public class DiskFileItem
> fout = new FileOutputStream(file);
> fout.write(get());
> } finally {
> - if (fout != null) {
> - fout.close();
> - }
> + IOUtils.closeQuietly(fout);
I think that is only a partial solution.
If the writes complete without any exception, then this will hide a
close error that occurs in normal use.
This might happen if buffered data cannot be written when closing.
Surely this should be detected as well?
One possible solution would be to add another fout.close() just before
the finally clause.
> }
> } else {
> File outputFile = getStoreLocation();
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1638380 - in /commons/proper/fileupload/trunk/src:
changes/changes.xml main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
Posted by sebb <se...@gmail.com>.
On 5 December 2014 at 09:08, Thomas Neidhart <th...@gmail.com> wrote:
> I have committed it already, but the commit mail seems to have been lost
> during the svn outage.
Or it may still be in the queue - there was a large backlog.
I have written a tool to check for missing SVN messages.
I will run this later and get INFRA to resend any ones that are missing.
> On Tue, Dec 2, 2014 at 9:42 PM, Thomas Neidhart <th...@gmail.com>
> wrote:
>
>> On 12/02/2014 06:13 PM, sebb wrote:
>> > On 11 November 2014 at 20:45, <tn...@apache.org> wrote:
>> >> Author: tn
>> >> Date: Tue Nov 11 20:45:16 2014
>> >> New Revision: 1638380
>> >>
>> >> URL: http://svn.apache.org/r1638380
>> >> Log:
>> >> [FILEUPLOAD-252] Do not lose IO exception when closing the stream.
>> >>
>> >> Modified:
>> >> commons/proper/fileupload/trunk/src/changes/changes.xml
>> >>
>> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.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=1638380&r1=1638379&r2=1638380&view=diff
>> >>
>> ==============================================================================
>> >> --- commons/proper/fileupload/trunk/src/changes/changes.xml (original)
>> >> +++ commons/proper/fileupload/trunk/src/changes/changes.xml Tue Nov 11
>> 20:45:16 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-252" dev="tn"
>> type="fix">DiskFileItem#write() could lose original IO exception</action>
>> >> <action issue="FILEUPLOAD-258" dev="tn"
>> type="fix">DiskFileItem#getStoreLocation() wrongly returned a File object
>> for items stored in memory</action>
>> >> <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>
>> >>
>> >> Modified:
>> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
>> >> URL:
>> http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java?rev=1638380&r1=1638379&r2=1638380&view=diff
>> >>
>> ==============================================================================
>> >> ---
>> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
>> (original)
>> >> +++
>> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
>> Tue Nov 11 20:45:16 2014
>> >> @@ -389,9 +389,7 @@ public class DiskFileItem
>> >> fout = new FileOutputStream(file);
>> >> fout.write(get());
>> >> } finally {
>> >> - if (fout != null) {
>> >> - fout.close();
>> >> - }
>> >> + IOUtils.closeQuietly(fout);
>> >
>> > I think that is only a partial solution.
>> >
>> > If the writes complete without any exception, then this will hide a
>> > close error that occurs in normal use.
>> > This might happen if buffered data cannot be written when closing.
>> > Surely this should be detected as well?
>> >
>> > One possible solution would be to add another fout.close() just before
>> > the finally clause.
>>
>> yes makes sense, I will change it like that.
>>
>> Thomas
>>
>> >
>> >
>> >> }
>> >> } else {
>> >> File outputFile = getStoreLocation();
>> >>
>> >>
>> >
>> > ---------------------------------------------------------------------
>> > 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: svn commit: r1638380 - in /commons/proper/fileupload/trunk/src:
changes/changes.xml main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
Posted by Thomas Neidhart <th...@gmail.com>.
I have committed it already, but the commit mail seems to have been lost
during the svn outage.
On Tue, Dec 2, 2014 at 9:42 PM, Thomas Neidhart <th...@gmail.com>
wrote:
> On 12/02/2014 06:13 PM, sebb wrote:
> > On 11 November 2014 at 20:45, <tn...@apache.org> wrote:
> >> Author: tn
> >> Date: Tue Nov 11 20:45:16 2014
> >> New Revision: 1638380
> >>
> >> URL: http://svn.apache.org/r1638380
> >> Log:
> >> [FILEUPLOAD-252] Do not lose IO exception when closing the stream.
> >>
> >> Modified:
> >> commons/proper/fileupload/trunk/src/changes/changes.xml
> >>
> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.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=1638380&r1=1638379&r2=1638380&view=diff
> >>
> ==============================================================================
> >> --- commons/proper/fileupload/trunk/src/changes/changes.xml (original)
> >> +++ commons/proper/fileupload/trunk/src/changes/changes.xml Tue Nov 11
> 20:45:16 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-252" dev="tn"
> type="fix">DiskFileItem#write() could lose original IO exception</action>
> >> <action issue="FILEUPLOAD-258" dev="tn"
> type="fix">DiskFileItem#getStoreLocation() wrongly returned a File object
> for items stored in memory</action>
> >> <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>
> >>
> >> Modified:
> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
> >> URL:
> http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java?rev=1638380&r1=1638379&r2=1638380&view=diff
> >>
> ==============================================================================
> >> ---
> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
> (original)
> >> +++
> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
> Tue Nov 11 20:45:16 2014
> >> @@ -389,9 +389,7 @@ public class DiskFileItem
> >> fout = new FileOutputStream(file);
> >> fout.write(get());
> >> } finally {
> >> - if (fout != null) {
> >> - fout.close();
> >> - }
> >> + IOUtils.closeQuietly(fout);
> >
> > I think that is only a partial solution.
> >
> > If the writes complete without any exception, then this will hide a
> > close error that occurs in normal use.
> > This might happen if buffered data cannot be written when closing.
> > Surely this should be detected as well?
> >
> > One possible solution would be to add another fout.close() just before
> > the finally clause.
>
> yes makes sense, I will change it like that.
>
> Thomas
>
> >
> >
> >> }
> >> } else {
> >> File outputFile = getStoreLocation();
> >>
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
>
Re: svn commit: r1638380 - in /commons/proper/fileupload/trunk/src:
changes/changes.xml main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
Posted by Thomas Neidhart <th...@gmail.com>.
On 12/02/2014 06:13 PM, sebb wrote:
> On 11 November 2014 at 20:45, <tn...@apache.org> wrote:
>> Author: tn
>> Date: Tue Nov 11 20:45:16 2014
>> New Revision: 1638380
>>
>> URL: http://svn.apache.org/r1638380
>> Log:
>> [FILEUPLOAD-252] Do not lose IO exception when closing the stream.
>>
>> Modified:
>> commons/proper/fileupload/trunk/src/changes/changes.xml
>> commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.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=1638380&r1=1638379&r2=1638380&view=diff
>> ==============================================================================
>> --- commons/proper/fileupload/trunk/src/changes/changes.xml (original)
>> +++ commons/proper/fileupload/trunk/src/changes/changes.xml Tue Nov 11 20:45:16 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-252" dev="tn" type="fix">DiskFileItem#write() could lose original IO exception</action>
>> <action issue="FILEUPLOAD-258" dev="tn" type="fix">DiskFileItem#getStoreLocation() wrongly returned a File object for items stored in memory</action>
>> <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>
>>
>> Modified: commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java
>> URL: http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java?rev=1638380&r1=1638379&r2=1638380&view=diff
>> ==============================================================================
>> --- commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java (original)
>> +++ commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/disk/DiskFileItem.java Tue Nov 11 20:45:16 2014
>> @@ -389,9 +389,7 @@ public class DiskFileItem
>> fout = new FileOutputStream(file);
>> fout.write(get());
>> } finally {
>> - if (fout != null) {
>> - fout.close();
>> - }
>> + IOUtils.closeQuietly(fout);
>
> I think that is only a partial solution.
>
> If the writes complete without any exception, then this will hide a
> close error that occurs in normal use.
> This might happen if buffered data cannot be written when closing.
> Surely this should be detected as well?
>
> One possible solution would be to add another fout.close() just before
> the finally clause.
yes makes sense, I will change it like that.
Thomas
>
>
>> }
>> } else {
>> File outputFile = getStoreLocation();
>>
>>
>
> ---------------------------------------------------------------------
> 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