Socket get input stream reading wrong values few times

Question

I have a Java Modbus/TCP application which is reading constantly data from a device.

This is working normal 99.9% of times, but after a weekend working, it could enter in a strange mode, in which for some seconds, I am getting fake values of my Read Multiple Holding Registers functions.

I have checked by using Modscan application, and fake values appear on the client site, which means that the server device is answering properly.

The answer that I can get is an Byte array filled of 0's, 1's and some times other random values.

Here is my Modbus/TCP answer reading:

private byte[] getModbusReply(){
    byte[] reply = null;
    int transactionId;
    int protocol;
    int tcpLen;
    int id;

    int replyCode;
    int mbLen = 1;

    try{
        InputStream is = socket.getInputStream();
        transactionId = (is.read()<<8)+is.read();
        protocol = (is.read()<<8)+is.read();
        tcpLen = (is.read()<<8)+is.read();
        id = is.read();
        replyCode = is.read();

        if(replyCode>0x3F){
            mbLen = 1;
        }else{
            switch(replyCode){
            case 0x03:
            case 0x04:
                mbLen = is.read();
                break;
            case 0x10:
            case 0x06:
                mbLen = 4;
                break;
            default://unsupported Modbus Methods
                return null;
            }
        }

        reply = new byte[mbLen+1];
        reply[0] = (byte)replyCode;
        for(int i=1;i<reply.length;i++){
            int res=is.read();
            if(res<0){
                //Modbus Stream Reading is returning -1
                return null;
            }
            reply[i] = (byte)res;
        }

    }catch(Exception e){
        e.printStackTrace();
        return null;
    }
    return reply;
}

Return null is processed outside the function as a wrong exception.

I had add 2 protections:

• read() method returns -1 after EOF, so I add:

      int res=is.read();
      if(res<0){
          //Modbus Stream Reading is returning -1
          return null;
      }
  

• Return null for unsupported Modbus/TCP methods:

      default:///unsupported Modbus Methods
          return null;
  

Maybe I am missing something more on stream reading to which I have no protection.

Answer 1

1. It isn't sufficient to just return null when something goes wrong or you get something you don't understand. That way you're just leaving the connection open and in an unsynchronized state where you have no reason to believe the next byte is the beginning of a new message. So, from then on you will read unintelligible junk. You need to close the connection and reopen it. Or, implement the rest of the protocol.

2. You aren't checking for end of stream at nine out of the ten points you read from it.

3. You should also have a good look at DataInputStream. It already does everything that you're already doing the hard way. Note particularly readShort() and readFully().