> On 24 Feb 2015, at 19:27, Seán Coffey <sean.cof...@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.cof...@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. >
