mtools icon indicating copy to clipboard operation
mtools copied to clipboard

Fix infinite loop when SocketServerCollector receives invalid packet

Open mroch opened this issue 5 years ago • 0 comments

When in --ip-mode=server, if a rogue client connects, it causes the server to go into an infinite loop trying to "retry" reading in _blockingread: https://github.com/matthewwall/mtools/blob/fa488f98b48bb734c2be94ef4ad4f24c56790443/bin/btmon.py#L2324-L2347

READ_RETRIES == 0. for each invalid character, _read raises ReadError, so it keeps reading until it hits EOF. then it raises an EmptyReadError and waits RETRY_WAIT (60 seconds), then tries to _read from the still-closed socket. This errors, ad nauseum.

This behavior makes sense for serial, since the only way to synchronize is to consume until you hit the packet header. But IP packets will never recover, so we don't need the _blockingread behavior and can just call _read.

There was one further issue: if the client disconnects quickly, then self._conn.shutdown() throws because the socket is already closed. shutdown is unnecessary in this case so we just ignore exceptions.

Test:

  • run bin/btmon.py --packet-format=gem48ptbin --ip --ip-mode=server --ip-port=5000 --debug --print

  • run telnet localhost 5000, send something like HELLO, ^], ctrl-d to disconnect

    2020/10/20 17:41:42 SOCKET: waiting for connection
    2020/10/20 17:41:45 reading 1 of 1 packets
    2020/10/20 17:41:51 SOCKET: read 1 of 1 bytes from socket: 48
    2020/10/20 17:41:51 expected START_HEADER0 0xfe, got 0x48
    Traceback (most recent call last):
      File "bin/btmon.py", line 2568, in read
        self._read(packet_format)
      File "bin/btmon.py", line 2318, in _read
        packets.extend(packet_format.read(self))
      File "bin/btmon.py", line 1675, in read
        return self._read1(collector, self.DATA_BYTES_LENGTH, self.PACKET_ID)
      File "bin/btmon.py", line 1563, in _read1
        self._checkbyte(data, 'START_HEADER0', self.START_HEADER0)
      File "bin/btmon.py", line 1558, in _checkbyte
        (label, hex(evalue), hex(b)))
    ReadError: expected START_HEADER0 0xfe, got 0x48
    2020/10/20 17:41:51 SOCKET: closing connection
    2020/10/20 17:41:51 buffer info:
    2020/10/20 17:41:51 processing with PrintProcessor
    2020/10/20 17:41:51 SOCKET: waiting for connection
    
  • run telnet again, immediately ^], ctrl-d without sending anything

    2020/10/20 17:43:31 reading 1 of 1 packets
    2020/10/20 17:43:34 SOCKET: read 0 of 1 bytes from socket:
    2020/10/20 17:43:34 SOCKET: client closed connection unexpectedly
    2020/10/20 17:43:34 SOCKET: closing connection
    2020/10/20 17:43:34 buffer info:
    2020/10/20 17:43:34 processing with PrintProcessor
    2020/10/20 17:43:34 SOCKET: waiting for connection
    

Fixes #22

mroch avatar Oct 20 '20 21:10 mroch