Why is my image coming out garbled?

Question

I've got some Java code using a servlet and Apache Commons FileUpload to upload a file to a set directory. It's working fine for character data (e.g. text files) but image files are coming out garbled. I can open them but the image doesn't look like it should. Here's my code:

Servlet

protected void doPost(HttpServletRequest request, HttpServletResponse response)
    throws ServletException, IOException {
    try {
      String customerPath = "\\leetest\\";

      // Check that we have a file upload request
      boolean isMultipart = ServletFileUpload.isMultipartContent(request);

      if (isMultipart) {
        // Create a new file upload handler
        ServletFileUpload upload = new ServletFileUpload();

        // Parse the request
        FileItemIterator iter = upload.getItemIterator(request);
        while (iter.hasNext()) {
          FileItemStream item = iter.next();
          String name = item.getFieldName();
          if (item.isFormField()) {
            // Form field.  Ignore for now
          } else {
            BufferedInputStream stream = new BufferedInputStream(item
                .openStream());
            if (stream == null) {
              LOGGER
                  .error("Something went wrong with fetching the stream for field "
                      + name);
            }

            byte[] bytes = StreamUtils.getBytes(stream);
            FileManager.createFile(customerPath, item.getName(), bytes);

            stream.close();
          }
        }
      }
    } catch (Exception e) {
      throw new UploadException("An error occured during upload: "
          + e.getMessage());
    }
}

StreamUtils.getBytes(stream) looks like:

public static byte[] getBytes(InputStream src, int buffsize)
      throws IOException {
    ByteArrayOutputStream byteStream = new ByteArrayOutputStream();
    byte[] buff = new byte[buffsize];
    while (true) {
      int nBytesRead = src.read(buff);
      if (nBytesRead < 0) {
        break;
      }
      byteStream.write(buff);
    }

    byte[] result = byteStream.toByteArray();
    byteStream.close();

    return result;
}

And finally FileManager.createFile looks like:

public static void createFile(String customerPath, String filename,
      byte[] fileData) throws IOException {
    customerPath = getFullPath(customerPath + filename);
    File newFile = new File(customerPath);
    if (!newFile.getParentFile().exists()) {
      newFile.getParentFile().mkdirs();
    }

    FileOutputStream outputStream = new FileOutputStream(newFile);
    outputStream.write(fileData);
    outputStream.close();
  }

Can anyone spot what I'm doing wrong?

Cheers, Lee

Answer 1

Are you sure that the image isn't coming through garbled or that you aren't dropping some packets on the way in.

Answer 2

It's possible but I'm not sure where it could be happening. If packets were being dropped I would expect to see them being dropped in text output too. I think I might just be using FileUpload improperly.

Answer 3

I don't know what difference it makes, but there seems to be a mismatch of method signatures. The getBytes() method called in your doPost() method has only one argument:

byte[] bytes = StreamUtils.getBytes(stream);

while the method source you included has two arguments:

public static byte[] getBytes(InputStream src, int buffsize)

Hope that helps.

Answer 4

Can you perform a checksum on your original file, and the uploaded file and see if there is any immediate differences?

If there are then you can look at performing a diff, to determine the exact part(s) of the file that are missing changed.

Things that pop to mind is beginning or end of stream, or endianness.

Answer 5

@Ben Good spot but it's not that. All the method accepting just a InputStream does is pass it onto the other method with a default buffer size.

@Craig I'll give it a shot and see what happens.

Answer 6

One thing I don't like is here in this block from StreamUtils.getBytes():

 1 while (true) {
 2   int nBytesRead = src.read(buff);
 3   if (nBytesRead < 0) {
 4     break;
 5   }
 6   byteStream.write(buff);
 7 }

At line 6, it writes the entire buffer, no matter how many bytes are read in. I am not convinced this will always be the case. It would be more correct like this:

 1 while (true) {
 2   int nBytesRead = src.read(buff);
 3   if (nBytesRead < 0) {
 4     break;
 5   } else {
 6     byteStream.write(buff, 0, nBytesRead);
 7   }
 8 }

Note the 'else' on line 5, along with the two additional parameters (array index start position and length to copy) on line 6.

I could imagine that for larger files, like images, the buffer returns before it is filled (maybe it is waiting for more). That means you'd be unintentionally writing old data that was remaining in the tail end of the buffer. This is almost certainly happening most of the time at EoF, assuming a buffer > 1 byte, but extra data at EoF is probably not the cause of your corruption...it is just not desirable.

Answer 7

I'd just use commons io Then you could just do an IOUtils.copy(InputStream, OutputStream);

It's got lots of other useful utility methods.