Downloaded files are corrupted when buffer length is > 1

Question

I'm trying to write a function which downloads a file at a specific URL. The function produces a corrupt file unless I make the buffer an array of size 1 (as it is in the code below).

The ternary statement above the buffer initialization (which I plan to use) along with hard-coded integer values other than 1 will manufacture a corrupted file.

Note: MAX_BUFFER_SIZE is a constant, defined as 8192 (2^13) in my code.

public static void downloadFile(String webPath, String localDir, String fileName) {
    try {
        File localFile;
        FileOutputStream writableLocalFile;
        InputStream stream;

        url = new URL(webPath);
        HttpURLConnection connection = (HttpURLConnection) url.openConnection();

        int size = connection.getContentLength(); //File size in bytes
        int read = 0; //Bytes read

        localFile = new File(localDir);

        //Ensure that directory exists, otherwise create it.
        if (!localFile.exists())
            localFile.mkdirs();

        //Ensure that file exists, otherwise create it.
        //Note that if we define the file path as we do below initially and call mkdirs() it will create a folder with the file name (I.e. test.exe). There may be a better alternative, revisit later.
        localFile = new File(localDir + fileName);
        if (!localFile.exists())
            localFile.createNewFile();

        writableLocalFile = new FileOutputStream(localFile);
        stream = connection.getInputStream();

        byte[] buffer;
        int remaining;
        while (read != size) {
            remaining = size - read; //Bytes still to be read
            //remaining > MAX_BUFFER_SIZE ? MAX_BUFFER_SIZE : remaining
            buffer = new byte[1]; //Adjust buffer size according to remaining data (to be read).

            read += stream.read(buffer); //Read buffer-size amount of bytes from the stream.
            writableLocalFile.write(buffer, 0, buffer.length); //Args: Bytes to read, offset, number of bytes
        }

        System.out.println("Read " + read + " bytes.");

        writableLocalFile.close();
        stream.close();
    } catch (Throwable t) {
        t.printStackTrace();
    }
}

The reason I've written it this way is so I may provide a real time progress bar to the user as they are downloading. I've removed it from the code to reduce clutter.

Answer 1

len = stream.read(buffer);
read += len;
writableLocalFile.write(buffer, 0, len); 

You must not use buffer.length as the bytes read, you need to use the return value of the read call. Because it might return a short read and then your buffer contains junk (0 bytes or data from previous reads) after the read bytes.

And besides calculating the remaining and using dynamic buffers just go for 16k or something like that. The last read will be short, which is fine.