Socket recvfrom buffer too small for receiving responses to large ICMP echo requests

[sjaddd]

Issue

In icmplib/sockets.py, the response buffer allocated by the receive method isn't big enough, when the echo request payload_size is greater than 996 bytes.

response = self._sock.recvfrom(1024)

Using examples/ping.py for testing:

• On Ubuntu (root), the calculated bytes_received value is incorrect, as response is truncated.

• On Windows 10, the self._sock.recvfrom(1024) triggers the below exception, which is swallowed within the icmplib/ping.py ping method by:

  except ICMPLibError:
      pass
  

  ICMPLibError: [WinError 10040] A message sent on a datagram socket was larger than the internal message buffer or some other network limit, or the buffer used to receive a datagram into was smaller than the datagram itself

  The flow on effect is that RTTs aren't calculated, and the host.is_alive is incorrectly set to False,

Fix

Simply allocating a buffer that can hold the max possible ICMP packet size (i.e. IPv4 max packet size) seems to work:

response = self._sock.recvfrom(65535)

Example

(Windows), sending 3 pings:

(venv_icmplib) D:\icmplib\examples>python ping.py 
ICMP payload size: 996
host.address: 10.0.0.2
host.min_rtt: 1.0
host.avg_rtt: 1.0
host.max_rtt: 1.279
host.rtts: [1.2786388397216797, 0.9996891021728516, 0.9996891021728516]
host.packets_sent: 3
host.packets_received: 3
host.packet_loss 0.0
host.jitter: 0.139
host.is_alive: True

(venv_icmplib) D:\icmplib\examples>python ping.py
ICMP payload size: 997
host.address: 10.0.0.2
host.min_rtt: 0.0
host.avg_rtt: 0.0
host.max_rtt: 0.0
host.rtts: []
host.packets_sent: 3
host.packets_received: 0
host.packet_loss 1.0
host.jitter: 0.0
host.is_alive: False

This is a nice tool, hope this helps!

[ValentinBELYN]

Hi @sjaddd,

Thank you for your very detailed issue! A fix will be released in the next version.

I think the buffer size should depend on the size of the request sent to avoid unnecessary memory consumption. Alternatively, I can also offer users the option to set the buffer size. I have to do some tests.

[sjaddd]

Great, thanks - looking forward to the next release 😃

[pyfisch]

Hi @ValentinBELYN,

great library! There have been multiple releases since. Is the issue fixed?

[ValentinBELYN]

Hi @pyfisch,

Unfortunately this issue is not fixed. I haven't had time to take care of this project lately but I plan to resume development this weekend for a release next week at the latest.

[pyfisch]

No problem. I planned to use the library to send large pings but now I don't need to do that anymore.

[tkekan]

@ValentinBELYN Can you please let me know if there is a plan to add buffer size to the receive api ? We need this for one of our use case.