Wrong echo response assumption

[eKristensen]

rust-ping wrongly assumes that the first package after a sent echo request always will be the response here:

rust-ping/src/ping.rs

Line 65 in e4b4432

socket.read(&mut buffer)?;

It may be the case that there are other ICMP packages in the same socket. [1]

A simple case where the next package received is not the reply is when you ping yourself. In that case the first package received is the just-sent echo request and not the reply. Therefore this crate will give a false negative on the ping-ability of a host.

Instead of assuming the response of a echo request is next package that arrives on the socket, this crate should listen on the socket and ignore any packages that do not match (instead of reporting an internal error) until the timeout ends. Of course Ok should be returned if a correct response is seen.

Please refer to the implementation of the the "normal" ping where the package is explicitly ignored in the ping4_parse_reply function: https://github.com/iputils/iputils/blob/1c081524ee463842631c708d060119ebc7ca699f/ping/ping.c#L1668 (I chose IPv4 as an example)

I do not have a suggested implementation yet. I have only just identified the issue right now.

[1] Not my words, see the "normal" ping implementation comments here: https://github.com/iputils/iputils/blob/1c081524ee463842631c708d060119ebc7ca699f/ping/ping.c#L1580

[aisk]

There was a pending PR on it, see #7.

[eKristensen]

There was a pending PR on it, see #7.

I did take a look at #7, but as far as I can tell it does not fix the underlying issue that I am seeing here. While there is a loop to retry, the check that currently makes the function return an internal error is still unchanged/unguarded. It should "continue" to something similar to restart the loop instead of returning internal error.

Also, I am not sure why #7 got stuck as a pending PR?

[aisk]

Sorry, I understand what you mean now. Yes, this is not the issue that #7 wants to resolve. It only ignoreds the mismatch ident reply message, but as you point out, the decode error should also be ignored.

#7 has some pending review suggestions which have not been updated. I'll continue the work if no one wants to investigate it a few days later.

[eKristensen]

I'm relatively new to Rust, to make a PR for this issue would properly be fun. I hope I'll have time for it soon :) - I have got an idea for how to fix it.

[aisk]

Please don't have any burden, any issues and PRs will be appreciate!

[aisk]

closed by #20