Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

On 22/09/16 19:22, Rob McKenna wrote: Thanks folks, I've been running some testing here and noticed that IP_REQ_TIMED_OUT can also be returned from IcmpSendEcho (as opposed to only being an error code in an ICMP_ECHO_REPLY) Updated webrev here:

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

Thanks folks, I've been running some testing here and noticed that IP_REQ_TIMED_OUT can also be returned from IcmpSendEcho (as opposed to only being an error code in an ICMP_ECHO_REPLY) Updated webrev here: http://cr.openjdk.java.net/~robm/8159410/webrev.03/ -Rob On 22/09/16 06:12,

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

> On 22 Sep 2016, at 18:04, Mark Sheppard wrote: > > > it is good that you added the additional error code, "cover all bases", as > they say. > In any case your exception handling will inform if something has been > missed, should it occur. > So at the risk of

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

it is good that you added the additional error code, "cover all bases", as they say. In any case your exception handling will inform if something has been missed, should it occur. So at the risk of triggering another MS curiosity, the changes look fine regards Mark On 21/09/2016 19:45,

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

The link would be handy: http://cr.openjdk.java.net/~robm/8159410/webrev.02/ -Rob On 21/09/16 07:44, Rob McKenna wrote: > I've updated the webrev here with the copyright year (thanks Christoph) and > extra error codes. I overlooked the codes from the old implementation of > tcp_ping4

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

I've updated the webrev here with the copyright year (thanks Christoph) and extra error codes. I overlooked the codes from the old implementation of tcp_ping4 above this code. These are winsock error codes which I would expect IcmpSendEcho to use, but in our testing it actually returned the

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

Yup. To elabourate: If we set a small timeout for a faraway host with a high ping, IcmpSendEcho can report success even if the rtt exceeded the timeout, hence the need for this explicit check. -Rob On 21/09/16 11:07, Vyom Tewari wrote: > So InetAddress.isReachable() will return false

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

So InetAddress.isReachable() will return false if the underline API IcmpSendEcho return with Status== IP_SUCESS and RoundTripTime > timeout. Vyom On Wednesday 21 September 2016 10:39 PM, Rob McKenna wrote: Unfortunately the behaviour described is undocumented and was found the hard way.

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

spotted an interesting blog on the MSDN timeout issue here : https://www.frameflow.com/ping-utility-flaw-in-windows-api-creating-false-timeouts/ Regards, Sean. On 21/09/16 17:42, Mark Sheppard wrote: the IcmpSendEcho series of calls come with some idiosyncrasies in that there is a minimum

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

Unfortunately the behaviour described is undocumented and was found the hard way. This part of the code is a necessity though. -Rob On 21/09/16 09:48, Vyom Tewari wrote: > Hi Rob, > > Do you really think this extra check is required ? > > if (pEchoReply->Status == IP_SUCCESS > + &&

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

It's absolutely worth looking into and I'll get going on that, but I'd rather deal with it separately to the ipv4 stuff. IcmpSendEcho already appears to behave somewhat strangely, so I wouldn't necessarily assume that the ipv4 and ipv6 code will end up being identical. -Rob On

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

the IcmpSendEcho series of calls come with some idiosyncrasies in that there is a minimum timeout that they can handle think it is about 1000msecs. isReachable can specify a finer grained timeout hence the need for timeout check regards Mark On 21/09/2016 17:18, Vyom Tewari wrote: Hi Rob,

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

Hi Rob, this looks good ... do you think there is any need to replicate these changes in Inet6AddressImpl.c ? (or leave it alone and don't trouble trouble until trouble troubles you :-) regards Mark regards Mark On 21/09/2016 16:16, Rob McKenna wrote: Hi folks, I'd like to fix a

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

Hi Rob, Do you really think this extra check is required ? if (pEchoReply->Status == IP_SUCCESS + && (int)pEchoReply->RoundTripTime <= timeout) I did not found any doc(MSDN) which explains this. I think in case of IP_SUCCESS "RoundTripTime" is always less than timeout. I think similar changes

RE: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

Hi Rob, this looks like a nice fix and I can't see anything besides the copyright year which you will for sure update when submitting. :) Unfortunately I'm not a reviewer so you'll have to get another real review. Best regards Christoph > -Original Message- > From: net-dev