From 545fa0f72b08fe5ce12d280ce98cf2c43d201fa7 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 24 Aug 2015 12:31:08 +0200 Subject: [PATCH] 475605 - Add support for multi-homed destinations. If DNS lookup returns multiple IP addresses, HttpClient tries to connect to the first; failing that, to the second, and so on. --- .../client/AbstractHttpClientTransport.java | 3 +- .../org/eclipse/jetty/client/HttpClient.java | 31 ++++- .../jetty/client/HttpClientTransport.java | 6 +- .../eclipse/jetty/client/HttpClientTest.java | 52 ++++++++- .../http/HttpClientTransportOverHTTP2.java | 15 +-- .../jetty/util/SocketAddressResolver.java | 106 +++++++++--------- 6 files changed, 140 insertions(+), 73 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpClientTransport.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpClientTransport.java index e4335979eaa..b37dee5fa8a 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpClientTransport.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpClientTransport.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.client; import java.io.IOException; +import java.net.InetSocketAddress; import java.net.SocketAddress; import java.nio.channels.SelectionKey; import java.nio.channels.SocketChannel; @@ -85,7 +86,7 @@ public abstract class AbstractHttpClientTransport extends ContainerLifeCycle imp } @Override - public void connect(SocketAddress address, Map context) + public void connect(InetSocketAddress address, Map context) { SocketChannel channel = null; try diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 845527b336e..93337516af7 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.net.CookieManager; import java.net.CookiePolicy; import java.net.CookieStore; +import java.net.InetSocketAddress; import java.net.Socket; import java.net.SocketAddress; import java.net.URI; @@ -547,15 +548,14 @@ public class HttpClient extends ContainerLifeCycle protected void newConnection(final HttpDestination destination, final Promise promise) { Origin.Address address = destination.getConnectAddress(); - resolver.resolve(address.getHost(), address.getPort(), new Promise() + resolver.resolve(address.getHost(), address.getPort(), new Promise>() { @Override - public void succeeded(SocketAddress socketAddress) + public void succeeded(List socketAddresses) { Map context = new HashMap<>(); context.put(HttpClientTransport.HTTP_DESTINATION_CONTEXT_KEY, destination); - context.put(HttpClientTransport.HTTP_CONNECTION_PROMISE_CONTEXT_KEY, promise); - transport.connect(socketAddress, context); + connect(socketAddresses, 0, context); } @Override @@ -563,6 +563,29 @@ public class HttpClient extends ContainerLifeCycle { promise.failed(x); } + + private void connect(List socketAddresses, int index, Map context) + { + context.put(HttpClientTransport.HTTP_CONNECTION_PROMISE_CONTEXT_KEY, new Promise() + { + @Override + public void succeeded(Connection result) + { + promise.succeeded(result); + } + + @Override + public void failed(Throwable x) + { + int nextIndex = index + 1; + if (nextIndex == socketAddresses.size()) + promise.failed(x); + else + connect(socketAddresses, nextIndex, context); + } + }); + transport.connect(socketAddresses.get(index), context); + } }); } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClientTransport.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClientTransport.java index f4fe560106b..bf0bbe62367 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClientTransport.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClientTransport.java @@ -18,7 +18,7 @@ package org.eclipse.jetty.client; -import java.net.SocketAddress; +import java.net.InetSocketAddress; import java.util.Map; import org.eclipse.jetty.io.ClientConnectionFactory; @@ -64,8 +64,8 @@ public interface HttpClientTransport extends ClientConnectionFactory /** * Establishes a physical connection to the given {@code address}. * - * @param address the address to connect to + * @param address the address to connect to * @param context the context information to establish the connection */ - public void connect(SocketAddress address, Map context); + public void connect(InetSocketAddress address, Map context); } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java index 80268809646..14da017556e 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java @@ -22,10 +22,12 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.HttpCookie; +import java.net.InetAddress; +import java.net.InetSocketAddress; import java.net.URI; import java.net.URLEncoder; +import java.net.UnknownHostException; import java.nio.ByteBuffer; -import java.nio.channels.UnresolvedAddressException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -77,6 +79,8 @@ import org.eclipse.jetty.toolchain.test.annotation.Slow; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FuturePromise; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.Promise; +import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.Assert; import org.junit.Assume; @@ -852,7 +856,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest } @Test - public void testConnectThrowsUnresolvedAddressException() throws Exception + public void testConnectThrowsUnknownHostException() throws Exception { start(new EmptyServerHandler()); @@ -864,13 +868,55 @@ public class HttpClientTest extends AbstractHttpClientServerTest public void onComplete(Result result) { Assert.assertTrue(result.isFailed()); - Assert.assertTrue(result.getFailure() instanceof UnresolvedAddressException); + Throwable failure = result.getFailure(); + Assert.assertTrue(failure instanceof UnknownHostException); latch.countDown(); } }); Assert.assertTrue(latch.await(10, TimeUnit.SECONDS)); } + @Test + public void testConnectHostWithMultipleAddresses() throws Exception + { + // Likely that the DNS for google.com returns multiple addresses. + String host = "google.com"; + Assume.assumeTrue(InetAddress.getAllByName(host).length > 1); + + startClient(); + client.setFollowRedirects(false); // Avoid redirects from 80 to 443. + client.setSocketAddressResolver(new SocketAddressResolver.Async(client.getExecutor(), client.getScheduler(), client.getConnectTimeout()) + { + @Override + public void resolve(String host, int port, Promise> promise) + { + super.resolve(host, port, new Promise>() + { + @Override + public void succeeded(List result) + { + // Replace the first address with an invalid address so that we + // test that the connect operation iterates over the addresses. + result.set(0, new InetSocketAddress("idontexist", 80)); + promise.succeeded(result); + } + + @Override + public void failed(Throwable x) + { + promise.failed(x); + } + }); + } + }); + + // Response code may be 200 or 302; + // if no exceptions the test passes. + client.newRequest(host, 80) + .header(HttpHeader.CONNECTION, "close") + .send(); + } + @Test public void testCustomUserAgent() throws Exception { diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java index 893d8c2c70b..27a85d795de 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java @@ -20,7 +20,6 @@ package org.eclipse.jetty.http2.client.http; import java.io.IOException; import java.net.InetSocketAddress; -import java.net.SocketAddress; import java.util.Map; import org.eclipse.jetty.client.HttpClient; @@ -61,14 +60,10 @@ public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements addBean(client); super.doStart(); this.connectionFactory = client.getClientConnectionFactory(); - client.setClientConnectionFactory(new ClientConnectionFactory() + client.setClientConnectionFactory((endPoint, context) -> { - @Override - public org.eclipse.jetty.io.Connection newConnection(EndPoint endPoint, Map context) throws IOException - { - HttpDestination destination = (HttpDestination)context.get(HTTP_DESTINATION_CONTEXT_KEY); - return destination.getClientConnectionFactory().newConnection(endPoint, context); - } + HttpDestination destination = (HttpDestination)context.get(HTTP_DESTINATION_CONTEXT_KEY); + return destination.getClientConnectionFactory().newConnection(endPoint, context); }); } @@ -92,7 +87,7 @@ public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements } @Override - public void connect(SocketAddress address, Map context) + public void connect(InetSocketAddress address, Map context) { client.setConnectTimeout(httpClient.getConnectTimeout()); @@ -124,7 +119,7 @@ public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements } }; - client.connect(httpClient.getSslContextFactory(), (InetSocketAddress)address, listener, promise, context); + client.connect(httpClient.getSslContextFactory(), address, listener, promise, context); } @Override diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/SocketAddressResolver.java b/jetty-util/src/main/java/org/eclipse/jetty/util/SocketAddressResolver.java index ac4cb3d32db..a25c3d2d845 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/SocketAddressResolver.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/SocketAddressResolver.java @@ -18,9 +18,12 @@ package org.eclipse.jetty.util; +import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.SocketAddress; -import java.nio.channels.UnresolvedAddressException; +import java.net.UnknownHostException; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -40,12 +43,11 @@ public interface SocketAddressResolver /** * Resolves the given host and port, returning a {@link SocketAddress} through the given {@link Promise} * with the default timeout. - * - * @param host the host to resolve + * @param host the host to resolve * @param port the port of the resulting socket address * @param promise the callback invoked when the resolution succeeds or fails */ - public void resolve(String host, int port, Promise promise); + public void resolve(String host, int port, Promise> promise); /** *

Creates {@link SocketAddress} instances synchronously in the caller thread.

@@ -54,13 +56,18 @@ public interface SocketAddressResolver public static class Sync implements SocketAddressResolver { @Override - public void resolve(String host, int port, Promise promise) + public void resolve(String host, int port, Promise> promise) { try { - InetSocketAddress result = new InetSocketAddress(host, port); - if (result.isUnresolved()) - promise.failed(new UnresolvedAddressException()); + InetAddress[] addresses = InetAddress.getAllByName(host); + + List result = new ArrayList<>(addresses.length); + for (InetAddress address : addresses) + result.add(new InetSocketAddress(address, port)); + + if (result.isEmpty()) + promise.failed(new UnknownHostException()); else promise.succeeded(result); } @@ -135,60 +142,55 @@ public interface SocketAddressResolver } @Override - public void resolve(final String host, final int port, final Promise promise) + public void resolve(final String host, final int port, final Promise> promise) { - executor.execute(new Runnable() + executor.execute(() -> { - @Override - public void run() + Scheduler.Task task = null; + final AtomicBoolean complete = new AtomicBoolean(); + if (timeout > 0) { - Scheduler.Task task = null; - final AtomicBoolean complete = new AtomicBoolean(); - if (timeout > 0) + final Thread thread = Thread.currentThread(); + task = scheduler.schedule(() -> { - final Thread thread = Thread.currentThread(); - task = scheduler.schedule(new Runnable() - { - @Override - public void run() - { - if (complete.compareAndSet(false, true)) - { - promise.failed(new TimeoutException()); - thread.interrupt(); - } - } - }, timeout, TimeUnit.MILLISECONDS); - } - - try - { - long start = System.nanoTime(); - InetSocketAddress result = new InetSocketAddress(host, port); - long elapsed = System.nanoTime() - start; - if (LOG.isDebugEnabled()) - LOG.debug("Resolved {} in {} ms", host, TimeUnit.NANOSECONDS.toMillis(elapsed)); if (complete.compareAndSet(false, true)) { - if (result.isUnresolved()) - promise.failed(new UnresolvedAddressException()); - else - promise.succeeded(result); + promise.failed(new TimeoutException()); + thread.interrupt(); } - } - catch (Throwable x) + }, timeout, TimeUnit.MILLISECONDS); + } + + try + { + long start = System.nanoTime(); + InetAddress[] addresses = InetAddress.getAllByName(host); + long elapsed = System.nanoTime() - start; + if (LOG.isDebugEnabled()) + LOG.debug("Resolved {} in {} ms", host, TimeUnit.NANOSECONDS.toMillis(elapsed)); + + List result = new ArrayList<>(addresses.length); + for (InetAddress address : addresses) + result.add(new InetSocketAddress(address, port)); + + if (complete.compareAndSet(false, true)) { - if (complete.compareAndSet(false, true)) - promise.failed(x); - } - finally - { - if (task != null) - task.cancel(); - // Reset the interrupted status before releasing the thread to the pool - Thread.interrupted(); + if (result.isEmpty()) + promise.failed(new UnknownHostException()); + else + promise.succeeded(result); } } + catch (Throwable x) + { + if (complete.compareAndSet(false, true)) + promise.failed(x); + } + finally + { + if (task != null) + task.cancel(); + } }); } }