malformed requests handling

[dnatalini]

Hallo, this is my first time on github, and this is a very nice package.

I made some tests using jsmodbus as a TCP server and a Java client based on jamod.

It turned out that jamod has a bug that makes it send malformed requests when writing multiple registers, and this made the server crash.

Unfortunately these things can happen, and I think it would be useful some sort of sanity check of the requests, and a way to trigger an error response to the client.

I could contribute some code on this, and if I understand well how github works I should fork this project and make a pull request, is this right?

I found that the requests are processed in modbus-tcp-server-client.js, inside onSocketData(): do you think this is the right place to insert checks and handling code? or maybe this sort of error handling could be shared with the serial part?

As you can see, I'm looking for suggestions.

Thanks, cheers.

[stefanpoeter]

HI @dnatalini,

what do you mean when you say it sends malformed requests when writing multiple registers? Can you explain that in more detail?

I could contribute some code on this, and if I understand well how github works I should fork this project and make a pull request, is this right?

That is right!

I found that the requests are processed in modbus-tcp-server-client.js, inside onSocketData(): do you think this is the right place to insert checks and handling code? or maybe this sort of error handling could be shared with the serial part?

The modbus-tcp-server-client.js file is the right place to go when you want to check incoming modbus requests.

[dnatalini]

what do you mean when you say it sends malformed requests when writing multiple registers? Can you explain that in more detail?

I didn't dig into jamod code, but a collegue sent me a test client program based on it, and it crashes the server. So I created a JS client with the very same request using your package, and added a log in onSocketData() to see the incoming buffer in both cases:

Java program request: <Buffer 00 00 00 00 00 00 01 10 00 00 00 08 10 00 01 00 02 00 03 00 04 00 05 00 06 00 07 00 08>

JavaScript program request: <Buffer 00 01 00 00 00 17 00 10 00 00 00 08 10 00 01 00 02 00 03 00 04 00 05 00 06 00 07 00 08>

The request is based on the client examples: client.writeMultipleRegisters(0, Buffer.from([0x00, 0x01, 0x00, 0x02, 0x00, 0x03, 0x00, 0x04, 0x00, 0x05, 0x00, 0x06, 0x00, 0x07, 0x00, 0x08])).then(function (resp) { console.log(resp); }, console.error);

The problem is: the java client doesn't properly set the message length, and the server creates a zero-length buffer. The server then tries to read the function code from the buffer, and throws "Index out of range".

The idea is that the server could easily check that the message is malformed, and answer with an error code, but I can't find in your package a function to do so: does it exist? should I create a new "onException(exception_code)" handler in modbus-tcp-server-client.js?

[stefanpoeter]

The exceptions are handled in the handler/server folder. There you find all implementations for the function codes. You can also see error handling in these files.

The idea is that the server could easily check that the message is malformed, and answer with an error code

I just checked the modbus specification and there is no exception code for a malformed request. The problem with a malformed request (especially for your case) is, that we do not know, when the next request starts, since we are not able to determine the length of the current request buffer.

There are two things we should define:

1. A malformed request should not crash the server. So, by checking the length and trashing the current buffer we avoid unwanted exceptions in the modbus server.
2. A client sending malformed requests should be disconnected. And the request should not be answered.

What do you think?

[stefanpoeter]

Anything?

[dnatalini]

Sorry for the delay.

1. is obviously ok for me
2. I think that the server should behave in a way that makes it clear what went wrong. Besides this, maybe the exception code 3 could be applicable for this case:

   A value contained in the query data field is not an allowable value for server. This indicates a fault in the structure of the remainder of a complex request, such as that the implied length is incorrect. It specifically does NOT mean that a data item submitted for storage in a register has a value outside the expectation of the application program, since the MODBUS protocol is unaware of the significance of any particular value of any particular register.

[stefanpoeter]

I am ok when you send an exception code 3 and close the connection afterwards since the following data cannot be processed anymore.

[stefanpoeter]

@dnatalini Do you still need this feature?

[dnatalini]

Hallo, I've been overwhelmed by too much urgent stuff, I'm sorry. This is still on my todo list, but to be honest I have no idea of when I'll be able to work on it.

[stefanpoeter]

Maybe you want to implement it in the v3.0.0-alpha branch. I'll close this issue and you come back whenever you want to.

[stefanpoeter]

@dnatalini I don't know why I didn't thought of that earlier but why repair a well working library when you can fix the broken jamod lib?