DefaultFileItem should not catch Exception

• Key: FILEUPLOAD-70
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Unassigned
• Reporter: David Graham

Environment

Operating System: other
Platform: Other

Description

There are several places in DefaultFileItem that catch Exception instead of the
specific exceptions thrown by the surrounded code. A better approach would be
to catch the specific exceptions (which I suspect will be IOException).

Comments

[Mohan Kishore]

Created an attachment (id=5823)
Minor exception-handling changes

[David Graham]

The proposed patch swallows IOExceptions in a "catch (Exception)" block. The
IOException should propagate out of the method.

[Mohan Kishore]

Any particular one in mind or all of them? Am swallowing all exceptions for the
xxx.close() methods. Otherwise, these may hide other more relevant exceptions.
Alternative might be something like this:

IOException ex = null;
try {
fout = new FileOutputStream(file);
fout.write(get());
} catch (IOException ioe) {
ex = ioe;
} finally {
try { if (fout != null) fout.close(); } catch (IOException ioe) { if (ex != null) ex = ioe; }
}
if (ex != null) throw ex;

[David Graham]

There should be no "catch Exception" blocks. Just let any IOException propagate
out of the method. An exception thrown during close() will not hide any other
exception because they would have been thrown first anyways.

[Mohan Kishore]

i think i see what you are saying.. am attaching a patch without all those
finally clauses. Have left in one IOexception swallowing block in the get()
method, which would necessiate changing the multiple public method
signatures... Other changes are backwards compatible.

[Mohan Kishore]

Created an attachment (id=5824)
Removed the finally clauses for close() methods

[Martin Cooper]

Fixed in the 20030428 nightly build.