[
https://issues.apache.org/jira/browse/FILEUPLOAD-295?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16775950#comment-16775950
]
Stephan Markwalder commented on FILEUPLOAD-295:
-----------------------------------------------
Hi Jochen,
I'm ok if this bug is not fixed. It is indeed an edge case. We have been
running into it after upgrading from version 1.3.3 to version 1.4 (in which
FILEUPLOAD-258 has been fixed).
Before, we have been using a sizeThreshold of 0 for a very long time, assuming
that this would cause DiskFileItem to always store data on disk, never in
memory. This was our fault no 1. Unfortunately, the JavaDoc for DiskFileItem's
constructor is not very precise:
{quote}{{sizeThreshold}} - The threshold, in bytes, below which items will be
retained in memory and above which they will be stored as a file.
{quote}
But what happens when the item size is equal to the threshold? :)
Our fault no 2 then was that people have used the return value of
DiskFileItem.getStoreLocation() without a check of DiskFileItem.isInMemory()
upfront (because of the assumption made above).
Together, those two faults have lead to NullPointerExceptions in our code after
the upgrade to version 1.4.
When analyzing the issue, I though that it would be sufficient to set
sizeThreshold to -1. No matter what the actual file size will be, it is for
sure 0 or greater. According to JavaDoc, this then should guarantee that items
will be stored as files on disk. But when writing some tests for DiskFileItem
to explore how it deals with empty files, we noticed that it behaves
differently depending on whether/how data is written into the DiskFileItem's
OutputStream. In some cases, an empty file is created, in others not.
Unfortunately, DiskFileItem then somehow is in an inconsistent state.
isInMemory() returns false, getStoreLocation() returns a File object, but the
file does not exist. As a result, we have seen FileNotFoundExceptions in some
of our components, even the ones where people have thought about an upfront
check with isInMemory(). Our "fix" now is to not trust DiskFileItem and the
information it returns for isInMemory() or getStoreLocation(), but always go
for an extra check whether the file actually exists.
If this bug is not fixed, it has at least been filed once. If other people run
into the same or a similar issue when upgrading to 1.4, they at least now have
a chance to find this ticket and maybe get some insights in how to solve them
for themselves.
> DiskFileItem: getStoreLocation() may return non-existing file
> -------------------------------------------------------------
>
> Key: FILEUPLOAD-295
> URL: https://issues.apache.org/jira/browse/FILEUPLOAD-295
> Project: Commons FileUpload
> Issue Type: Bug
> Affects Versions: 1.4
> Reporter: Stephan Markwalder
> Priority: Minor
>
> *How to reproduce*
> # Create a DiskFileItem with threshold set to -1 (force save to disk).
> # Get OutputStream from DiskFileItem.
> # Close OutputStream without calling any write(...) method (e.g., because
> the uploaded file is empty).
> # Test the return value of the following methods:
> ** isInMemory() --> returns false (OK)
> ** getSize() --> returns 0 (OK)
> ** getStoreLocation() --> returns a File object (OK), but the file does not
> exist (FAILURE).
> I think this is an inconsistency. If isInMemory() returns false adn
> getStoreLocation() returns a File object, the file should also exist.
> Java code (run with -ea to enable assertions):
> {code:java}
> // create a DiskFileItem
> int sizeThreshold = -1; // always store to disk
> File repository = null; // use temporary folder
> DiskFileItem item = new DiskFileItem("file", "text/plain", false, "file.txt",
> sizeThreshold, repository);
> OutputStream outputStream = item.getOutputStream();
> // do not write to stream <-- IMPORTANT
> outputStream.close();
> // assert that data has been stored to disk
> assert !item.isInMemory(); // pass
> assert item.getSize() == 0; // pass
> assert item.getStoreLocation() != null; // pass
> assert item.getStoreLocation().isFile(); // fails
> {code}
> When adding a call to outputStream.write(new byte[0]), the behavior changes
> and the empty file is created on disk.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
