Read InputStream and Write to a File

Question

This is related to my previous question - DataInputStream giving java.io.EOFException

In that Client-Server app there is method to retrieve file sent from server and save to a file.

Client.java -

 public void receiveFile(InputStream is, String fileName) throws Exception {
        int filesize = 6022386;
        int bytesRead;
        int current = 0;
        byte[] mybytearray = new byte[filesize];

        System.out.println("Receving File!");
        FileOutputStream fos = new FileOutputStream("RECEIVED_"+fileName);
        BufferedOutputStream bos = new BufferedOutputStream(fos);
        bytesRead = is.read(mybytearray, 0, mybytearray.length);
        current = bytesRead; 
        System.out.println(bytesRead);
        do {
            bytesRead = is.read(mybytearray, current,
                    (mybytearray.length - current));
            System.out.println(bytesRead);
            if (bytesRead >= 0)
                current += bytesRead;
        } while (bytesRead > -1);
        System.out.println("Loop done");
        bos.write(mybytearray, 0, current);
        bos.flush();
        bos.close();
    }
}

Server.Java

public void sendFile(OutputStream os, String fileName) throws Exception {
    File myFile = new File(fileName);
    byte[] mybytearray = new byte[(int) myFile.length() + 1];
    FileInputStream fis = new FileInputStream(myFile);
    BufferedInputStream bis = new BufferedInputStream(fis);
    bis.read(mybytearray, 0, mybytearray.length);
    System.out.println("Sending File!");
    os.write(mybytearray, 0, mybytearray.length);
    os.flush();
    bis.close();
}

As you can see there are several stranded outs in client's receiveFile method. here is the output i received.

The issue is that method don't complete its task and never reached to System.out.println("Loop done");

What's the issue ?

Answer 1

I'll come to the loop error later.

There is one little byte too much at the source:

public void sendFile(OutputStream os, String fileName) throws Exception {
    File myFile = new File(fileName);
    if (myFile.length() > Integer.MAX_VALUE) {
        throw new IllegalStateException();
    }

Either

    byte[] mybytearray = Files.readAllBytes(myFile.toPath());

Or

    byte[] mybytearray = new byte[(int) myFile.length()]; // No +1.
    // BufferedInputStream here not needed.
    try (BufferedInputStream bis = new BufferedInputStream(
            new FileInputStream(myFile))) {
        bis.read(mybytearray);
    } // Always closed.

and then

    System.out.println("Sending File!");
    os.write(mybytearray);
    os.flush();
}

Alternatively I have added the java 7 Files.readAllBytes. It could even be simpler using Files.copy.

I just see the main error is mentioned already. Basically there is a misconception: you may read a byte array in its entirety. It will block till the end is read. If there is less to read ("end-of-file" reached), then the number of bytes is returned. So to all purposes you might read it in a whole.

One often sees similar code to read a fixed size (power of 2) block (say 4096) repeated and written to the output stream.

Again java 7 Files simplifies all:

    Files.copy(is, Paths.get("RECEIVED_" + fileName));

---

In Short:

public void receiveFile(InputStream is, String fileName) throws Exception {
    Files.copy(is, Paths.get("RECEIVED_" + fileName),
        StandardCopyOption.REPLACE_EXISTING);
}

public void sendFile(OutputStream os, String fileName) throws Exception {
    Files.copy(Paths.get(fileName), os);
}

Answer 2

You probably want your condition to read:

} while (bytesRead > -1 && current < mybytearray.length);
//                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

At the moment, when you have read the entire file, your read statement keeps reading zero bytes (mybytearray.length - current) which succeeds and returns zero, so your loop never terminates.

Answer 3

It would be much simpler to not keep appending to the input buffer

public void receiveFile(InputStream istream, String fileName) throws Exception {
  System.err.println("Receving File!");
  try (BufferedOutputStream ostream = new BufferedOutputStream(
      new FileOutputStream("RECEIVED_" + fileName))) {
    int bytesRead;
    byte[] mybytearray = new byte[1024];

    do { 
      bytesRead = istream.read(mybytearray);
      if (bytesRead > 0) {
        ostream.write(mybytearray, 0, bytesRead);
      }
    } while (bytesRead != -1);
    System.err.println("Loop done");
    ostream.flush();
  }
}

If you can use Guava, you can use ByteStreams.copy(java.io.InputStream, java.io.OutputStream) (Javadoc)