Issue Details (XML | Word | Printable)

Key: FILEUPLOAD-109
Type: Improvement Improvement
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Unassigned
Reporter: Amichai Rothman
Votes: 1
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
Commons FileUpload

MultipartStream.discardBodyData() implementation is redundant.

Created: 19/May/06 07:01 AM   Updated: 10/Jun/06 02:15 AM
Return to search
Component/s: None
Affects Version/s: 1.1 Final
Fix Version/s: None

Time Tracking:
Not Specified

File Attachments:
  Size
Text File Licensed for inclusion in ASF works commons-fileupload-1.1-bug-109.patch 2006-05-19 07:02 AM Amichai Rothman 2 kB
Text File Licensed for inclusion in ASF works commons-fileupload-1.patch 2006-05-22 07:47 PM Jochen Wiedmann 4 kB

Resolution Date: 10/Jun/06 02:15 AM


 Description  « Hide
MultipartStream.discardBodyData() is identical to readBodyData() - copied and pasted, but with output removed.

discardBodyData() can be changed to call readBodyData() with a NullInputStream (from commons io package, which is already a dependency). This is much more readable and maintainable, and much less error prone.

(Alternatively, readBodyData() can be modified to handle a null output stream by performing a null check before writing into it, and skipping the writes if output is null. then discardBodyData() can call it with a null argument. However, the first solution seems more elegant.)



 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Amichai Rothman added a comment - 19/May/06 07:02 AM
The suggested fix using NullInputStream.

Jochen Wiedmann added a comment - 22/May/06 07:47 PM
Funny, but I came to the same conclusion just yesterday. In case someone's interested, here's my version with the check for null. (Which I personally prefer, because I believe it has less overhead.)

Jochen Wiedmann added a comment - 10/Jun/06 02:15 AM
Sorry, but I have choosen the null approach, although I admit that the /dev/null solution has its charm.