views:

781

answers:

3

Hi folks :)

I`m trying to read an image from an URL (with the java package java.net.URL) to a byte[]. "Everything" works fine, except that the content isnt being enterely read from the stream (the image is corrupt, it doesnt contain all the image data)... The byte array is being persisted in a database (BLOB). I really dont know what the correct approach is, maybe you can give me a tip :)

This is my first approach (code formatted, removed unnecessary informations...):

URL u = new URL("http://localhost:8080/images/anImage.jpg");
int contentLength = u.openConnection().getContentLength();
Inputstream openStream = u.openStream();
byte[] binaryData = new byte[contentLength];
openStream.read(binaryData);
openStream.close();

My second approach was this one (as you'll see the contentlength is being fetched another way):

URL u = new URL(content);
openStream = u.openStream();
int contentLength = openStream.available();
byte[] binaryData = new byte[contentLength];
openStream.read(binaryData);
openStream.close();

Both of the code result in a corrupted image... I already read this post from stackoverflow

A: 

The content length is just a HTTP header. You cannot trust it. Just read everything you can from the stream.

Available is definitely wrong. It's just the number of bytes that can be read without blocking.

Another issue is your resource handling. Closing the stream has to happen in any case. try/catch/finally will do that.

Thomas Jung
Thx for your answer. I omit the try/catch in my code posting.But how can I know the exact length of the stream? I have to allocate the byte[], so I do have to provide a length. Allocate a fixed length (say 1024) and reading from an position to an offset, check if stream contains data, copying to a new array, merging all byte[] couldnt be the best solution...
tim.kaufner
+5  A: 

There's no guarantee that the content length you're provided is actually correct. Try something akin to the following:

ByteArrayOutputStream bais = new ByteArrayOutputStream();
InputStream is = null;
try {
  is = url.openStream ();
  byte[] byteChunk = new byte[4096]; // Or whatever size you want to read in at a time.
  int n;

  while ( (n = is.read(byteChunk)) > 0 ) {
    bais.write(byteChunk, 0, n);
  }
}
catch (IOException e) {
  System.err.printf ("Failed while reading bytes from %s: %s", url.toExternalForm(), e.getMessage());
  e.printStackTrace ();
  // Perform any other exception handling that's appropriate.
}
finally {
  if (is != null) { is.close(); }
}

You'll then have the image data in bais, from which you can get a byte array by calling bais.toByteArray().

This code is untested (I just wrote it in the answer box), but it's a reasonably close approximation to what I think you're after.

RTBarnard
Please never write an empty catch-block, not even in an example! Put at least `e.printStackTrace()` there! Examples have a tendency to become production code and we all have to work with that later on.
Joachim Sauer
You're absolutely right; thanks for pointing that out. I've added more meaningful exception handling to the example.
RTBarnard
Use http://commons.apache.org/io/api-1.4/org/apache/commons/io/IOUtils.html#toByteArray(java.io.InputStream) . This will make code look much cleaner.
Adi
Thx for your answer, your approach did work :) I have to write the data bit by bit (or a defined chunk value) to a ByteArrayOutputStream (which will on end be outputted with .toByteArray(), that was my fault...
tim.kaufner
+3  A: 

Just extending Barnards's answer with commons-io. Separate answer because I can not format code in comments.

InputStream is = null;
try {
  is = url.openStream ();
  byte[] imageBytes = IOUtils.toByteArray(is);
}
catch (IOException e) {
  System.err.printf ("Failed while reading bytes from %s: %s", url.toExternalForm(), e.getMessage());
  e.printStackTrace ();
  // Perform any other exception handling that's appropriate.
}
finally {
  if (is != null) { is.close(); }
}

http://commons.apache.org/io/api-1.4/org/apache/commons/io/IOUtils.html#toByteArray(java.io.InputStream)

Adi
Thats another nice solution, but I will use the first one (because we wont include too much external libs).
tim.kaufner