steveohara / j2mod

Enhanced Modbus library implemented in the Java programming language
Apache License 2.0
267 stars 111 forks source link

Handle TCP Fragmentation cases #29

Closed DanSalt closed 7 years ago

DanSalt commented 8 years ago

I'm diagnosing an issue whereby, for some devices, TCP fragmentation occurs and the J2mod library is not waiting for the rest of the message payload, thus filling the remainder with random bytes, causing response values to be incorrect.

My use case is: Read Input Register, length 2 I'm using version 2.0 of the library.

Using Wireshark, I can see the message sent successfully, and I see the response. The response should be 12 bytes (8 header and 4 bytes payload). The first frame contains bytes 0-11, and the later frame contains byte 12. Wireshark correctly figures out that the PDU is split over multiple TCP frames, and reconstructs the messages. But J2mod does not wait for the second frame. Instead, my Response contains the correct data for bytes 0-11, plus a random byte for the 12th.

I confess that I don't know enough about the underlying implementation to be able to suggest what should change, nor have I found an easy way to reproduce this issue locally so I can test without connecting to a field device.

The tool 'ModPoll' seems to handle this use-case, since I'm able to run my code and get an incorrect value, and the ModPoll gives the correct value. The two requests and responses look identical in Wireshark (attached image).

image

Hope this is enough information to be helping to diagnose the problem.

Thanks! Dan

DanSalt commented 8 years ago

Hi,

After some more debug, I've located the source of the issue. In ModbusTCPTransport, in ReadRequest and ReadResponse, the code is reading from the dataInputStream for 'count' bytes, but not checking that 'count' bytes were actually read. In the use-case above (where the TCP frames are fragmented), it requires multiple reads on the stream to fully complete the PDU. When I modified the code to read until it had read 'count' bytes, then the problem was resolved.

I can send/submit my fix to you if you'd like? Although I haven't yet checked if this code appears anywhere else in the library.

steveohara commented 8 years ago

Hello Dan,

Thanks for the debugging and yes please, submit a pull request and I will happily add it in.

Thanks, Steve

steveohara commented 8 years ago

Hey Dan, If your're not keen or don't have the time to create a pull request, cut & paste the code changes here and I'll add them in.