RFR: 7178362: Socket impls should ignore unsupported proxy types rather than throwing

[Seán Coffey]

Thanks for suggested edits Chris. Taken them in. Shouldn't we also keep 
check for possibility of (p == null) ? After all, we could be dealing 
with bad proxy selector. I added  checks for null and also updated test 
case to test for same. It should help preserve behaviour in that area.

updated webrev : 
http://cr.openjdk.java.net/~coffeys/webrev.7178362.v2/webrev/

regards,
Sean.

On 24/02/2015 17:41, Chris Hegarty wrote:
> Sean,
>
> Thanks for moving this forward. I think it might be possible to simplify this by flipping the logic, like:
>
> diff --git a/src/java.base/share/classes/java/net/SocksSocketImpl.java b/src/java.base/share/classes/java/net/SocksSocketImpl.java
> --- a/src/java.base/share/classes/java/net/SocksSocketImpl.java
> +++ b/src/java.base/share/classes/java/net/SocksSocketImpl.java
> @@ -388,14 +388,13 @@
>               }
>               while (iProxy.hasNext()) {
>                   p = iProxy.next();
> -                if (p == null || p == Proxy.NO_PROXY) {
> +                if (p.type() != Proxy.Type.SOCKS) {
>                       super.connect(epoint, remainingMillis(deadlineMillis));
>                       return;
>                   }
> -                if (p.type() != Proxy.Type.SOCKS)
> -                    throw new SocketException("Unknown proxy type : " + p.type());
> +
>                   if (!(p.address() instanceof InetSocketAddress))
> -                    throw new SocketException("Unknow address type for proxy: " + p);
> +                    throw new SocketException("Unknown address type for proxy: " + p);
>                   // Use getHostString() to avoid reverse lookups
>                   server = ((InetSocketAddress) p.address()).getHostString();
>                   serverPort = ((InetSocketAddress) p.address()).getPort();
> @@ -707,13 +706,12 @@
>               }
>               while (iProxy.hasNext()) {
>                   p = iProxy.next();
> -                if (p == null || p == Proxy.NO_PROXY) {
> +                if (p.type() != Proxy.Type.SOCKS) {
>                       return;
>                   }
> -                if (p.type() != Proxy.Type.SOCKS)
> -                    throw new SocketException("Unknown proxy type : " + p.type());
> +
>                   if (!(p.address() instanceof InetSocketAddress))
> -                    throw new SocketException("Unknow address type for proxy: " + p);
> +                    throw new SocketException("Unknown address type for proxy: " + p);
>                   // Use getHostString() to avoid reverse lookups
>                   server = ((InetSocketAddress) p.address()).getHostString();
>                   serverPort = ((InetSocketAddress) p.address()).getPort();
>
>
> -Chris.
>
> On 24 Feb 2015, at 17:24, Seán Coffey <sean.coffey at oracle.com> wrote:
>
>> This issue relates to another bug that I own : JDK-8062305
>>
>> It seems to be an area that causes alot of issue (for plugin mainly) and the suggestion from Chris in 7178362 bug report to return a direct connection type for bad ProxySelector implementations seems appropriate.
>>
>> webrev : http://cr.openjdk.java.net/~coffeys/webrev.7178362/webrev/
>> bug report : https://bugs.openjdk.java.net/browse/JDK-7178362
>>
>> regards,
>> Sean.