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

2016-09-23 Thread Chris Hegarty 

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:

http://cr.openjdk.java.net/~robm/8159410/webrev.03/


Thanks Rob. This latest version looks good to me.

-Chris.


-Rob

On 22/09/16 06:12, Chris Hegarty wrote:



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 triggering another MS curiosity, the changes look fine


+1

-Chris.


regards
Mark

On 21/09/2016 19:45, Rob McKenna wrote:

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 above this code. These are winsock error codes which I would expect 
IcmpSendEcho to use, but in our testing it actually returned the system error 
codes in at least one situation:

https://msdn.microsoft.com/en-gb/library/windows/desktop/ms740668%28v=vs.85%29.aspx
https://msdn.microsoft.com/en-us/library/windows/desktop/ms681383%28v=vs.85%29.aspx

-Rob

On 21/09/16 06:32, Seán Coffey wrote:

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 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,

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 is
required in Inet6Address.c as well ? Thanks, Vyom

On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:

Hi folks,

I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob

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

2016-09-22 Thread Rob McKenna 
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, Chris Hegarty wrote:
> 
> > 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 triggering another MS curiosity, the changes look fine
> 
> +1 
> 
> -Chris.
> 
> > regards
> > Mark
> > 
> > On 21/09/2016 19:45, Rob McKenna wrote:
> >> 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 above this code. These are winsock error codes which I would 
> >>> expect IcmpSendEcho to use, but in our testing it actually returned the 
> >>> system error codes in at least one situation:
> >>> 
> >>> https://msdn.microsoft.com/en-gb/library/windows/desktop/ms740668%28v=vs.85%29.aspx
> >>> https://msdn.microsoft.com/en-us/library/windows/desktop/ms681383%28v=vs.85%29.aspx
> >>> 
> >>>   -Rob
> >>> 
> >>> On 21/09/16 06:32, Seán Coffey wrote:
>  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 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,
> >> 
> >> 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 is
> >> required in Inet6Address.c as well ? Thanks, Vyom
> >> 
> >> On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:
> >>> Hi folks,
> >>> 
> >>> I'd like to fix a bug caused by an incorrect assumption. The 
> >>> IcmpSendEcho* calls can actually return a similar set of errors 
> >>> regardless of whether the call itself failed or succeeded. This 
> >>> change checks that both the call and the ping were successful. In 
> >>> addition to that it takes a number of common failure causes into 
> >>> account before deciding to throw an exception.
> >>> 
> >>> http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> >>> 
> >>>   -Rob
> > 
>

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

2016-09-22 Thread Chris Hegarty 

> 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 triggering another MS curiosity, the changes look fine

+1 

-Chris.

> regards
> Mark
> 
> On 21/09/2016 19:45, Rob McKenna wrote:
>> 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 above this code. These are winsock error codes which I would 
>>> expect IcmpSendEcho to use, but in our testing it actually returned the 
>>> system error codes in at least one situation:
>>> 
>>> https://msdn.microsoft.com/en-gb/library/windows/desktop/ms740668%28v=vs.85%29.aspx
>>> https://msdn.microsoft.com/en-us/library/windows/desktop/ms681383%28v=vs.85%29.aspx
>>> 
>>> -Rob
>>> 
>>> On 21/09/16 06:32, Seán Coffey wrote:
 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 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,
>> 
>> 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 is
>> required in Inet6Address.c as well ? Thanks, Vyom
>> 
>> On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:
>>> Hi folks,
>>> 
>>> I'd like to fix a bug caused by an incorrect assumption. The 
>>> IcmpSendEcho* calls can actually return a similar set of errors 
>>> regardless of whether the call itself failed or succeeded. This change 
>>> checks that both the call and the ping were successful. In addition to 
>>> that it takes a number of common failure causes into account before 
>>> deciding to throw an exception.
>>> 
>>> http://cr.openjdk.java.net/~robm/8159410/webrev.01/
>>> 
>>> -Rob
>

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

2016-09-22 Thread Mark Sheppard 


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, Rob McKenna wrote:

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 above this code. These are winsock error codes which I would expect 
IcmpSendEcho to use, but in our testing it actually returned the system error 
codes in at least one situation:

https://msdn.microsoft.com/en-gb/library/windows/desktop/ms740668%28v=vs.85%29.aspx
https://msdn.microsoft.com/en-us/library/windows/desktop/ms681383%28v=vs.85%29.aspx

-Rob

On 21/09/16 06:32, Seán Coffey wrote:

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 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,

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 is
required in Inet6Address.c as well ? Thanks, Vyom

On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:

Hi folks,

I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob

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

2016-09-21 Thread Rob McKenna 
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 above this code. These are winsock error codes which I would expect 
> IcmpSendEcho to use, but in our testing it actually returned the system error 
> codes in at least one situation:
> 
> https://msdn.microsoft.com/en-gb/library/windows/desktop/ms740668%28v=vs.85%29.aspx
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms681383%28v=vs.85%29.aspx
> 
>   -Rob
> 
> On 21/09/16 06:32, Seán Coffey wrote:
> > 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 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,
> > >>
> > >>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 is
> > >>required in Inet6Address.c as well ? Thanks, Vyom
> > >>
> > >>On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:
> > >>>Hi folks,
> > >>>
> > >>>I'd like to fix a bug caused by an incorrect assumption. The 
> > >>>IcmpSendEcho* calls can actually return a similar set of errors 
> > >>>regardless of whether the call itself failed or succeeded. This change 
> > >>>checks that both the call and the ping were successful. In addition to 
> > >>>that it takes a number of common failure causes into account before 
> > >>>deciding to throw an exception.
> > >>>
> > >>>http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> > >>>
> > >>> -Rob
> > >>
> > >
> >

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

2016-09-21 Thread Rob McKenna 
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 system error 
codes in at least one situation:

https://msdn.microsoft.com/en-gb/library/windows/desktop/ms740668%28v=vs.85%29.aspx
https://msdn.microsoft.com/en-us/library/windows/desktop/ms681383%28v=vs.85%29.aspx

-Rob

On 21/09/16 06:32, Seán Coffey wrote:
> 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 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,
> >>
> >>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 is
> >>required in Inet6Address.c as well ? Thanks, Vyom
> >>
> >>On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:
> >>>Hi folks,
> >>>
> >>>I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
> >>>calls can actually return a similar set of errors regardless of whether 
> >>>the call itself failed or succeeded. This change checks that both the call 
> >>>and the ping were successful. In addition to that it takes a number of 
> >>>common failure causes into account before deciding to throw an exception.
> >>>
> >>>http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> >>>
> >>>   -Rob
> >>
> >
>

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

2016-09-21 Thread Rob McKenna 
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 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. 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
> >>+ && (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 is required in
> >>Inet6Address.c as well ? Thanks, Vyom
> >>
> >>
> >>On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:
> >>>Hi folks,
> >>>
> >>>I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
> >>>calls can actually return a similar set of errors regardless of whether 
> >>>the call itself failed or succeeded. This change checks that both the call 
> >>>and the ping were successful. In addition to that it takes a number of 
> >>>common failure causes into account before deciding to throw an exception.
> >>>
> >>>http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> >>>
> >>>   -Rob
>

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

2016-09-21 Thread Vyom Tewari 
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. 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
+ && (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 is required in
Inet6Address.c as well ? Thanks, Vyom


On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:

Hi folks,

I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob

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

2016-09-21 Thread Seán Coffey 

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 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,

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 
is required in Inet6Address.c as well ? Thanks, Vyom


On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:

Hi folks,

I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob

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

2016-09-21 Thread Rob McKenna 
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
> + && (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 is required in
> Inet6Address.c as well ? Thanks, Vyom
> 
> 
> On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:
> >Hi folks,
> >
> >I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
> >calls can actually return a similar set of errors regardless of whether the 
> >call itself failed or succeeded. This change checks that both the call and 
> >the ping were successful. In addition to that it takes a number of common 
> >failure causes into account before deciding to throw an exception.
> >
> >http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> >
> > -Rob
>

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

2016-09-21 Thread Rob McKenna 
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 21/09/16 05:35, Mark Sheppard wrote:
> 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 bug caused by an incorrect assumption. The IcmpSendEcho* 
> >calls can actually return a similar set of errors regardless of whether the 
> >call itself failed or succeeded. This change checks that both the call and 
> >the ping were successful. In addition to that it takes a number of common 
> >failure causes into account before deciding to throw an exception.
> >
> >http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> >
> > -Rob
>

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

2016-09-21 Thread Mark Sheppard 


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,

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 
is required in Inet6Address.c as well ? Thanks, Vyom


On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:

Hi folks,

I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob

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

2016-09-21 Thread Mark Sheppard 

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 bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob

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

2016-09-21 Thread Vyom Tewari 

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 is 
required in Inet6Address.c as well ? Thanks, Vyom



On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:

Hi folks,

I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob

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

2016-09-21 Thread Langer, Christoph 
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 [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Rob
> McKenna
> Sent: Mittwoch, 21. September 2016 17:17
> To: net-dev@openjdk.java.net
> Subject: RFR - 8159410: InetAddress.isReachable returns true for non existing 
> IP
> addresses
> 
> Hi folks,
> 
> I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho*
> calls can actually return a similar set of errors regardless of whether the 
> call
> itself failed or succeeded. This change checks that both the call and the ping
> were successful. In addition to that it takes a number of common failure 
> causes
> into account before deciding to throw an exception.
> 
> http://cr.openjdk.java.net/~robm/8159410/webrev.01/
> 
>   -Rob

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

2016-09-21 Thread Rob McKenna 
Hi folks,

I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob