Socket InputStream glitches while reading bytes in chunks

Question

I'm writing simple file transfer through socket connection and I have a problem while downloading a file in chunks of bytes. When I read one byte at a time everything works but when I try to read for example in chunks of 512 bytes, after downloading all the bytes of the file(I know the length thanks to my header), the socket starts receiving some random bytes and eventually there is a crash because 2x "%" were found and the server tries to count the length of incoming message, when there is none to begin with.

 @Override
  public void run() {

      while (client.isConnected() && !client.isClosed() && !client.isInputShutdown()){
          try {
                readResponse();
                clientWriter.write("0%"); /// IF IT CATCHES EXCEPTION SOCKET CLOSED ON THE OTHER SIDE(DON'T KNOW HOW TO CHECK IT IN ANOTHER WAY)
           } 
          catch (IOException e) {
               System.out.println(e.toString());
               return;
           } 
          catch (InterruptedException e) {
               System.out.println(e.toString());
               return;
           } 
      }

   }

private void readResponse() throws IOException, InterruptedException {

       int length = 0;
       int ch;
       FileOutputStream file = new FileOutputStream("holder");
       boolean bWrite = false;
       StringBuilder name = new StringBuilder();
       int percentageCount = 0;
       String filename="";
       length = 0;
       int l = 0;
       int old = 0;
       while ((ch=client.getInputStream().read())!=-1){
           if (!bWrite) {
               name.append(Character.toChars(ch));
           }
           if (ch==37 && !bWrite){
               if (percentageCount<1){
                   percentageCount++;
               } else {
                filename = name.toString();
                length = Integer.parseInt(filename.substring(0,filename.indexOf("%")));
                l = length;
                filename = filename.substring(filename.indexOf("%")+1);
                filename = filename.substring(0,filename.indexOf("%"));
               file = new FileOutputStream(filename);
               bWrite = true;
               break;
               }
           }
       }
       byte[] bytes = new byte[512];
       while (length>=512){
           client.getInputStream().read(bytes, 0, 512);
           file.write(bytes, 0, 512);
           length-=512;
       }
       if (length>0){
           bytes = new byte[length];
           client.getInputStream().read(bytes,0,length);
           file.write(bytes,0,length);
           length = 0;
       }
       file.flush();
       file.close();
    }

You need to cleanup the loops first. Dont do stuff like `client.getInputStream().read()` and try to remove all code which is not relevant to your question. It will be hard to find somebody who wants to read all that. — eckes, Jan 05 '15 at 22:17
one obvious problem is that you read a 512 byte buffer but you do not actually check how many bytes are returned. you must work with the return value of read (and only write readLen bytes). This happens 2 times in your code. Especially the second time with larger length will fail. — eckes, Jan 05 '15 at 22:21
remove anything else? i know it's a bit chaotic but it should all work — Greyshack, Jan 05 '15 at 22:21
ok I've done what you said and the crash is still there, even though I substract and write only the amount of read bytes — Greyshack, Jan 05 '15 at 22:25
The test `client.isConnected() && !client.isClosed() && !client.isInputShutdown()` is pointless. All these methods do is test whether *you* called the corresponding APIs *on this socket.* They don't tell you anything whatsoever about the state of the *connection.* For example, `isClosed()` doesn't magically become `true` when the *peer* closes the connection. — user207421, Jan 05 '15 at 23:30

eckes · Accepted Answer · 2015-01-05T23:12:00.950

2

The last part of the code can be changed to (untested):

   byte[] bytes = new byte[512];
   InputStream is = client.getInputStream();
   while (length>0) {
       int read = is.read(bytes, 0, Math.min(bytes.length, length));
       if (read > 0) {
         file.write(bytes, 0, read);
         length-=read;
       }
       if (read < 0)
         throw new IOException("end reached before all data read"; 
   }

This correctly checks the returned size, it avoids two loops and checks for end of stream.

BTW2: getInputStream() is pretty heavy weight, it should only be done once (the is from my example should be retrieved at the beginning of the method, especially before the read loop).

edited Jan 05 '15 at 23:12

answered Jan 05 '15 at 22:29

eckes

10,103
1
59
71

One more question, how can I best detect if the socket is closed on the other hand? – Greyshack Jan 05 '15 at 22:47
You cant detect it in all cases unless you send/receive some form of heartbeat message. You can switch on the [keepalive option](http://docs.oracle.com/javase/6/docs/api/java/net/Socket.html#setKeepAlive%28boolean%29) for the socket, but this will only detect it after hours (typical OS setting). But if the OS detects it, you will get -1 on read or an exception on write. You should include a "i am finished" message in your protocol so you can cleanly shut down. @Peter-Lawrey has a sample for the -1: http://stackoverflow.com/a/5562456/13189 – eckes Jan 05 '15 at 23:21

Socket InputStream glitches while reading bytes in chunks

1 Answers1