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