Index out of range exceptions in Esp32Coprocessor.Encoder.cs on RaiseWiFiDisconnected

[doingnz]

When the Wifi disconnects, events are raised that will call Esp32WiFiAdapter.RaiseWiFiDisconnected(...) with an assumed to be valid byte[] payload parameter. In turn this calls Encoders.ExtractDisconnectEventData(payload, 0); to decode the reason for the disconnect.

Encoders.ExtractDisconnectEventData(byte[] buffer, int offset) does not check it is called with valid payload data and simply indexes into the byte array in a for...loop. (buffer[index + offset] != 0). etc.

This will cause System.IndexOutOfRangeException: Index was outside the bounds of the array. if for example payload has zero bytes length.

Issue WildernessLabs/Meadow_Issues#497 means the application simple stops execution without any error messages.

Should all the methods in Encoders.cs at a minimum have try/catch clauses? Simple sanity checks would also fail faster than try catch.

            if (buffer.Length == 0)
            {
                return (disconnectEventData); 
            }

[ctacke]

The OS team is investigating this. The fix feels right, but there's a question of why bad data is even getting in here that is being chased.

[doingnz]

Currently Meadow will crash / shutdown any time wifi is turned off rather than the expected behaviour of being notified by an event. Personally I think this is a p0 bug.

I appreciate the team wants to understand why RaiseWiFiDisconnected is called with a zero length payload[] and given the method ExtractDisconnectEventData is auto generated, it appears the way forward, even if only till some other solution is found, is to apply some very simple sanity checks in RaiseWiFiDisconnected as follows.

Let me know if you want me to make a new PR with this change?

    protected void RaiseWiFiDisconnected(StatusCodes statusCode, byte[] payload)
    {
        ClearNetworkDetails();
        DisconnectEventData disconnectEventData = new DisconnectEventData
        {
            Reason = (byte)WiFiDisconnectedReason.Unspecified
        };
        if (payload.Length != 0)
        {
            try
            {
                disconnectEventData = Encoders.ExtractDisconnectEventData(payload, 0);
            }
            catch (Exception e)
            {
                // ignore exception and use default "WiFiDisconnectedReason.Unspecified"
            }
        }

        string reason = DisconnectReason(disconnectEventData);
        lock (_lock)
        {
            RaiseNetworkDisconnected(new NetworkDisconnectionEventArgs(reason));
        }
    }

[adrianstevens]

@NevynUK any progress on this one?