# HG changeset patch # User chegar # Date 1529060012 -3600 # Node ID 79e371a6462c807b532daeade55e45fbada7ddb3 # Parent ba60eaef37d7f496a52365555d5a5552d862bd32 http-client-banch: connect failures should mostly result in ConnectExeption diff -r ba60eaef37d7 -r 79e371a6462c src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java --- 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); + } } } diff -r ba60eaef37d7 -r 79e371a6462c src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java --- 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; } diff -r ba60eaef37d7 -r 79e371a6462c src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java --- 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}. diff -r ba60eaef37d7 -r 79e371a6462c test/jdk/java/net/httpclient/ConnectExceptionTest.java --- /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 = List.of(new Proxy(Proxy.Type.HTTP, + InetSocketAddress.createUnresolved("proxy.invalid", 8080))); + @Override public List 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 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 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 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 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 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); + } + } + } +} diff -r ba60eaef37d7 -r 79e371a6462c test/jdk/java/net/httpclient/noPermissions.policy --- /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}/*" { };