forwardClose of IO connection may result in exception

[greg9504]

While testing IO connections with a Banner Engineering XS26 safety controller I found that an exception would be generated during the forwardClose call.
Code change to fix is here:
https://github.com/greg9504/ST-node-ethernet-ip/tree/ioForwardCloseNoOTconnID

Current Behavior

Exception during forwardClose, while attempting to decode the OTconnID id. It seems that the client (XS26) did not send back the OTconnID in the message.

Expected Behavior

Check length of data before attempting to decode. The decoded OTconnID does not appear to be used anyway, so a default of 1 is set in this case.

Possible Solution (Optional)

        let OTconnID = 1;// id not sent back by some clients
        //some clients may not include OTconnID in forwardClose response
        //found while testing with Banner Engineering XS26 safetly controller
        if (data.length > 0) {
            OTconnID = data.readUInt32LE(0); // first 4 Bytes are O->T connection ID 
        }
        super.id_conn = OTconnID;

Context

Can not use lib with Banner XS26 for IO connections without fix.

Steps to Reproduce (for bugs only)

Setup IO connection with Banner Engineering XS26 then close connection...

function GenericEthernetIPclient() {
var ioscanner;                      // Connection Manager for IO exchange
var ioconnections = [];             //  IO connections
// open the io connection

var makeIOConnection = function {
// simplified
ioscanner = new STEthernetIp.IO.Scanner(2222, '0.0.0.0');
const ioconn = ioscanner.addConnection(config, module.rpi, addr, ioport, false);
}
...
// close the connection
    var _disconnectAll = async function () {
        const result = await _closeScannerWrapper();
        for (let ioconn of ioconnections) {
            if (ioconn) {
                ioconn.run = false;
                try {
                    console.log('closing io connection...')
                    await ioconn.tcpController.disconnect();
                    console.log('io connection closed');
                } catch (error) {
                    console.log('Error disconnecting io connection');
                    console.log(error);
                    ioconn.tcpController.destroy();
                    ioconn.tcpController._removeControllerEventHandlers();
                    console.log('forced io connection closed');
                }
                ioconn = undefined;
            }
        }
        ioconnections = [];
    }

// close the UDP listening port
var _closeScanner = function (successCallback, errorCallback) {
        try {
            if (ioscanner) {
                ioscanner.socket.close(() => {
                    ioscanner = undefined;
                    console.log('scanner closed');
                    successCallback(true);
                });
            } else {
                console.log('scanner not open, success');
                successCallback(true);
            }
        } catch (err) {
            errorCallback(false);
        }
    }
    var _closeScannerWrapper = function () {
        return new Promise((resolve, reject) => {
            _closeScanner((successResponse) => {
                resolve(successResponse);
            }, (errorResponse) => {
                reject(errorResponse);
                return;
            });
        });
    }

call makeIOConnection then later call disconnectAll.

Your Environment

• Controller Type (eg 1756-L83E/B): Banner Engineering XS26-2DE Safety Controller
• Controller Firmware (eg 30.11): FID1

[SerafinTech]

ForwardClose has not been worked on very much. I'll check to see what improvements can be made for your use case.

[greg9504]

Hello Jason,
OK, I can send you a Wireshark file if you like. My proposed change above to src/io/tcp/index.ts should probably be

if (data.length >= 4) <-- instead of 0
to be safe.