search

Cloud-Automation / node-modbus

Modbus TCP Client/Server implementation for Node.JS
466 stars 174 forks source link

Sanity checks on response header in tcp client #150

Closed psorowka closed 6 years ago

psorowka commented 6 years ago

Until now, the only packet validation by the TCP Client was checking the return function code. However, if an invalid function code is returned, only the command promise is rejected but no consequences for the socket are being made. In this commit, I added sanity checks on the returned modbus header, meaning that the expected requestId as well as the protocolVersion (0x0000) are validated on every packet. If either of these is invalid, an error is thrown and the socket is destroyed immediately.

Bonus: added error messages to promise rejections if function code is invalid

stefanpoeter commented 6 years ago

Good morning @psorowka,

thanks for your update. Since these changes introduce minor API changes (which are still downward compatible) I would make this v2.3.0. Do you agree?

psorowka commented 6 years ago

Yep absolutely.

one more thing though. I purposely did not change the behavior in case of an invalid fc in the received message, only added the error in the rejection for making things clearer.

However I feel that reading an invalid fc should probably not only reject a single promise but also destroy the socket. What do you think?

stefanpoeter commented 6 years ago

Can you change the version in the package.json to 2.3.0 then?

I would say adding an error object to a reject callback is a change in the API. You also fire more rejects than before, so this is also a change in the API. Hence the minor version increase.

Haven't thought about shutting down the socket but I think it is a good way of handling unwanted packages or unspecified behaviour.