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

[Chris Hegarty]

> On 24 Feb 2015, at 19:27, Seán Coffey <sean.coffey at oracle.com> wrote:
> 
> 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.

Good catch Sean.

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

Looks good.

-Chris.


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