http-client-banch: connect failures should mostly result in ConnectExeption http-client-branch
authorchegar
Fri, 15 Jun 2018 11:53:32 +0100
branchhttp-client-branch
changeset 56762 79e371a6462c
parent 56756 ba60eaef37d7
child 56763 25821dd1d917
http-client-banch: connect failures should mostly result in ConnectExeption
src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java
src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java
src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java
test/jdk/java/net/httpclient/ConnectExceptionTest.java
test/jdk/java/net/httpclient/noPermissions.policy
--- a/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java	Wed Jun 13 19:11:47 2018 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java	Fri Jun 15 11:53:32 2018 +0100
@@ -32,6 +32,7 @@
 import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
 import java.net.Authenticator;
+import java.net.ConnectException;
 import java.net.CookieHandler;
 import java.net.ProxySelector;
 import java.net.http.HttpTimeoutException;
@@ -531,16 +532,21 @@
             final Throwable throwable = e.getCause();
             final String msg = throwable.getMessage();
 
-            if (throwable instanceof IllegalArgumentException)
+            if (throwable instanceof IllegalArgumentException) {
                 throw new IllegalArgumentException(msg, throwable);
-            else if (throwable instanceof SecurityException)
+            } else if (throwable instanceof SecurityException) {
                 throw new SecurityException(msg, throwable);
-            else if (throwable instanceof HttpTimeoutException)
+            } else if (throwable instanceof HttpTimeoutException) {
                 throw new HttpTimeoutException(msg);
-            else if (throwable instanceof IOException)
+            } else if (throwable instanceof ConnectException) {
+                ConnectException ce = new ConnectException(msg);
+                ce.initCause(throwable);
+                throw ce;
+            } else if (throwable instanceof IOException) {
                 throw new IOException(msg, throwable);
-            else
+            } else {
                 throw new IOException(msg, throwable);
+            }
         }
     }
 
--- a/src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java	Wed Jun 13 19:11:47 2018 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java	Fri Jun 15 11:53:32 2018 +0100
@@ -89,14 +89,16 @@
                 // complete async since the event runs on the SelectorManager thread
                 cf.completeAsync(() -> null, client().theExecutor());
             } catch (Throwable e) {
-                client().theExecutor().execute( () -> cf.completeExceptionally(e));
+                Throwable t = Utils.toConnectException(e);
+                client().theExecutor().execute( () -> cf.completeExceptionally(t));
+                close();
             }
         }
 
         @Override
         public void abort(IOException ioe) {
+            client().theExecutor().execute( () -> cf.completeExceptionally(ioe));
             close();
-            client().theExecutor().execute( () -> cf.completeExceptionally(ioe));
         }
     }
 
@@ -112,7 +114,7 @@
             try {
                  finished = AccessController.doPrivileged(pa);
             } catch (PrivilegedActionException e) {
-                cf.completeExceptionally(e.getCause());
+               throw e.getCause();
             }
             if (finished) {
                 if (debug.on()) debug.log("connect finished without blocking");
@@ -123,7 +125,8 @@
                 client().registerEvent(new ConnectEvent(cf));
             }
         } catch (Throwable throwable) {
-            cf.completeExceptionally(throwable);
+            cf.completeExceptionally(Utils.toConnectException(throwable));
+            close();
         }
         return cf;
     }
--- a/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java	Wed Jun 13 19:11:47 2018 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java	Fri Jun 15 11:53:32 2018 +0100
@@ -30,6 +30,7 @@
 import sun.net.www.HeaderParser;
 
 import javax.net.ssl.ExtendedSSLSession;
+import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLParameters;
 import javax.net.ssl.SSLSession;
 import java.io.ByteArrayOutputStream;
@@ -40,10 +41,12 @@
 import java.io.UncheckedIOException;
 import java.io.UnsupportedEncodingException;
 import java.lang.System.Logger.Level;
+import java.net.ConnectException;
 import java.net.InetSocketAddress;
 import java.net.URI;
 import java.net.URLPermission;
 import java.net.http.HttpHeaders;
+import java.net.http.HttpTimeoutException;
 import java.nio.ByteBuffer;
 import java.nio.CharBuffer;
 import java.nio.charset.CharacterCodingException;
@@ -951,6 +954,20 @@
         return address;
     }
 
+    public static Throwable toConnectException(Throwable e) {
+        if (e == null) return null;
+        e = getCompletionCause(e);
+        if (e instanceof ConnectException) return e;
+        if (e instanceof SecurityException) return e;
+        if (e instanceof SSLException) return e;
+        if (e instanceof Error) return e;
+        if (e instanceof HttpTimeoutException) return e;
+        Throwable cause = e;
+        e = new ConnectException(e.getMessage());
+        e.initCause(cause);
+        return e;
+    }
+
     /**
      * Returns the smallest (closest to zero) positive number {@code m} (which
      * is also a power of 2) such that {@code n <= m}.
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/net/httpclient/ConnectExceptionTest.java	Fri Jun 15 11:53:32 2018 +0100
@@ -0,0 +1,167 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @summary Expect ConnectException for all non-security related connect errors
+ * @bug 8204864
+ * @run testng/othervm ConnectExceptionTest
+ * @run testng/othervm/java.security.policy=noPermissions.policy ConnectExceptionTest
+ */
+
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Proxy;
+import java.net.ProxySelector;
+import java.net.SocketAddress;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+import static java.lang.System.out;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
+
+public class ConnectExceptionTest {
+
+    static final ProxySelector INVALID_PROXY = new ProxySelector() {
+        final List<Proxy> proxy = List.of(new Proxy(Proxy.Type.HTTP,
+                InetSocketAddress.createUnresolved("proxy.invalid", 8080)));
+        @Override public List<Proxy> select(URI uri) { return proxy; }
+        @Override public void connectFailed(URI uri, SocketAddress sa, IOException ioe) { }
+        @Override public String toString() { return "INVALID_PROXY"; }
+    };
+
+    static final ProxySelector NO_PROXY = new ProxySelector() {
+        @Override public List<Proxy> select(URI uri) { return List.of(Proxy.NO_PROXY); }
+        @Override public void connectFailed(URI uri, SocketAddress sa, IOException ioe) { }
+        @Override public String toString() { return "NO_PROXY"; }
+    };
+
+    @DataProvider(name = "uris")
+    public Object[][] uris() {
+        return new Object[][]{
+            { "http://test.invalid/",  NO_PROXY       },
+            { "https://test.invalid/", NO_PROXY       },
+            { "http://test.invalid/",  INVALID_PROXY  },
+            { "https://test.invalid/", INVALID_PROXY  },
+        };
+    }
+
+    @Test(dataProvider = "uris")
+    void testSynchronousGET(String uriString, ProxySelector proxy) throws Exception {
+        out.printf("%n---%ntestSynchronousGET starting uri:%s, proxy:%s%n", uriString, proxy);
+        HttpClient client = HttpClient.newBuilder().proxy(proxy).build();
+
+        URI uri = URI.create(uriString);
+        HttpRequest request = HttpRequest.newBuilder(uri).build();
+        try {
+            HttpResponse<String> response = client.send(request, BodyHandlers.ofString());
+            fail("UNEXPECTED response: " + response + ", body:" + response.body());
+        } catch (ConnectException ioe) {
+            out.println("Caught expected: " + ioe);
+            //ioe.printStackTrace(out);
+        } catch (SecurityException expectedIfSMIsSet) {
+            out.println("Caught expected: " + expectedIfSMIsSet);
+            assertTrue(System.getSecurityManager() != null);
+        }
+    }
+
+    @Test(dataProvider = "uris")
+    void testSynchronousPOST(String uriString, ProxySelector proxy) throws Exception {
+        out.printf("%n---%ntestSynchronousPOST starting uri:%s, proxy:%s%n", uriString, proxy);
+        HttpClient client = HttpClient.newBuilder().proxy(proxy).build();
+
+        URI uri = URI.create(uriString);
+        HttpRequest request = HttpRequest.newBuilder(uri)
+                .POST(BodyPublishers.ofString("Does not matter"))
+                .build();
+        try {
+            HttpResponse<String> response = client.send(request, BodyHandlers.ofString());
+            fail("UNEXPECTED response: " + response + ", body:" + response.body());
+        } catch (ConnectException ioe) {
+            out.println("Caught expected: " + ioe);
+            //ioe.printStackTrace(out);
+        } catch (SecurityException expectedIfSMIsSet) {
+            out.println("Caught expected: " + expectedIfSMIsSet);
+            assertTrue(System.getSecurityManager() != null);
+        }
+    }
+
+    @Test(dataProvider = "uris")
+    void testAsynchronousGET(String uriString, ProxySelector proxy) throws Exception {
+        out.printf("%n---%ntestAsynchronousGET starting uri:%s, proxy:%s%n", uriString, proxy);
+        HttpClient client = HttpClient.newBuilder().proxy(proxy).build();
+
+        URI uri = URI.create(uriString);
+        HttpRequest request = HttpRequest.newBuilder(uri).build();
+        try {
+            HttpResponse<String> response = client.sendAsync(request, BodyHandlers.ofString()).get();
+            fail("UNEXPECTED response: " + response + ", body:" + response.body());
+        } catch (ExecutionException ee) {
+            Throwable t = ee.getCause();
+            if (t instanceof ConnectException) {
+                out.println("Caught expected: " + t);
+            } else if (t instanceof SecurityException) {
+                out.println("Caught expected: " + t);
+                assertTrue(System.getSecurityManager() != null);
+            } else {
+                t.printStackTrace(out);
+                fail("Unexpected exception: " + t);
+            }
+        }
+    }
+
+    @Test(dataProvider = "uris")
+    void testAsynchronousPOST(String uriString, ProxySelector proxy) throws Exception {
+        out.printf("%n---%ntestAsynchronousPOST starting uri:%s, proxy:%s%n", uriString, proxy);
+        HttpClient client = HttpClient.newBuilder().proxy(proxy).build();
+
+        URI uri = URI.create(uriString);
+        HttpRequest request = HttpRequest.newBuilder(uri)
+                .POST(BodyPublishers.ofString("Does not matter"))
+                .build();
+        try {
+            HttpResponse<String> response = client.sendAsync(request, BodyHandlers.ofString()).get();
+            fail("UNEXPECTED response: " + response + ", body:" + response.body());
+        } catch (ExecutionException ee) {
+            Throwable t = ee.getCause();
+            if (t instanceof ConnectException) {
+                out.println("Caught expected: " + t);
+            } else if (t instanceof SecurityException) {
+                out.println("Caught expected: " + t);
+                assertTrue(System.getSecurityManager() != null);
+            } else {
+                t.printStackTrace(out);
+                fail("Unexpected exception: " + t);
+            }
+        }
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/net/httpclient/noPermissions.policy	Fri Jun 15 11:53:32 2018 +0100
@@ -0,0 +1,24 @@
+//
+// Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+// DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+//
+// This code is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License version 2 only, as
+// published by the Free Software Foundation.
+//
+// This code is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+// version 2 for more details (a copy is included in the LICENSE file that
+// accompanied this code).
+//
+// You should have received a copy of the GNU General Public License version
+// 2 along with this work; if not, write to the Free Software Foundation,
+// Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+//
+// Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+// or visit www.oracle.com if you need additional information or have any
+// questions.
+//
+
+grant codeBase "file:${test.classes}/*" { };