views:

813

answers:

2

I'm trying to read a remote binary file (say, image) from internet like this:

HttpURLConnection connection = (HttpURLConnection) myUrl.openConnection(); //myUrl - URL object pointing for some location
if(connection.getResponseCode() == 200){
    File temp = File.createTempFile("blabla", fileName); //fileName - string name of file
    FileOutputStream out = new FileOutputStream(temp);
    int fileSize = Integer.parseInt(connection.getHeaderField("content-length"));
    int counter = 0;
    DataInputStream in = new DataInputStream(connection.getInputStream());
    byte ch[] = new byte[1024];
    System.out.println(counter);
    while((counter += in.read(ch)) > 0){
        out.write(ch);
        if(counter == fileSize){
            out.close();
            break;
        }
    }
}

Locally of with local web server (localhost) it works perfectly.

But. Then myUrl is URL of file on some remote web server - it returns unexpected results. For instance, from sources of given files it seems that it repeats some packages (I think because of corruption of previous ones or someting) and the resulting file usually is about 10% bigger than original one because of this repeats. So file is corrupted and cannot be opened correctly with image viewers.

How can I solve this?

+3  A: 

read does not necessarily read the entire buffer (particularly if it is at the end of the stream).

So change your loop:

for (;;) {
    int len = in.read(ch);
    if (len == -1) {
        break;
    }
    out.write(ch, 0, len);
}

Perhaps put that code in a method somewhere.

Also note:

  • There is no point in using DataInputStream here (although readFully is often useful).
  • Always close resource (such as streams) with the usual idiom:

    final Resource resource = acquire(); try { use(resource); } finally { resource.close(); }

  • Probably wont make much difference, but a buffer size of 1024 is a bit small. I tend to default to 8192 arbitrarily.
Tom Hawtin - tackline
Just for completion, most Java programmers prefer the shorter variant: int len; while ((len = in.read(ch)) >= 0) { out.write(ch, 0, len); }Saves one condition, a break and reuses the variable on the stack, which makes it a little less error prone.
Peter Walser
A prefer to avoid side-effects. Reusing variables is not best-practice!
Tom Hawtin - tackline
A: 

Oh, just minute after asking found solution on another thread.

Thank you, Tom, for your time and absolutely fast reply.

Arturas