From 6c8c0314a06d87f2b97247310676273f0df8ec76 Mon Sep 17 00:00:00 2001 From: Stephane Landelle Date: Tue, 12 Mar 2013 12:25:36 +0100 Subject: [PATCH] Refactor non proxy hosts handling, fix #202 in 1.7.x --- .../apache/ApacheAsyncHttpProvider.java | 5 +- .../grizzly/GrizzlyAsyncHttpProvider.java | 52 +++---------------- .../grizzly/GrizzlyResponseFuture.java | 11 ++-- .../providers/jdk/JDKAsyncHttpProvider.java | 7 ++- .../netty/NettyAsyncHttpProvider.java | 47 ++++++----------- .../providers/netty/NettyConnectListener.java | 8 ++- .../providers/netty/NettyResponseFuture.java | 10 +++- .../java/com/ning/http/util/ProxyUtils.java | 14 +++++ .../com/ning/http/client/async/ProxyTest.java | 15 ++++++ 9 files changed, 81 insertions(+), 88 deletions(-) diff --git a/src/main/java/com/ning/http/client/providers/apache/ApacheAsyncHttpProvider.java b/src/main/java/com/ning/http/client/providers/apache/ApacheAsyncHttpProvider.java index 062ab4c689..561464d72f 100644 --- a/src/main/java/com/ning/http/client/providers/apache/ApacheAsyncHttpProvider.java +++ b/src/main/java/com/ning/http/client/providers/apache/ApacheAsyncHttpProvider.java @@ -340,9 +340,8 @@ private HttpMethodBase createMethod(HttpClient client, Request request) throws I throw new IllegalStateException(String.format("Invalid Method", methodName)); } - ProxyServer proxyServer = request.getProxyServer() != null ? request.getProxyServer() : config.getProxyServer(); - boolean avoidProxy = ProxyUtils.avoidProxy(proxyServer, request); - if (!avoidProxy) { + ProxyServer proxyServer = ProxyUtils.getProxyServer(config, request); + if (proxyServer != null) { if (proxyServer.getPrincipal() != null) { Credentials defaultcreds = new UsernamePasswordCredentials(proxyServer.getPrincipal(), proxyServer.getPassword()); diff --git a/src/main/java/com/ning/http/client/providers/grizzly/GrizzlyAsyncHttpProvider.java b/src/main/java/com/ning/http/client/providers/grizzly/GrizzlyAsyncHttpProvider.java index 1f5ce1b993..83fc8cc63d 100644 --- a/src/main/java/com/ning/http/client/providers/grizzly/GrizzlyAsyncHttpProvider.java +++ b/src/main/java/com/ning/http/client/providers/grizzly/GrizzlyAsyncHttpProvider.java @@ -200,8 +200,8 @@ public GrizzlyAsyncHttpProvider(final AsyncHttpClientConfig clientConfig) { public ListenableFuture execute(final Request request, final AsyncHandler handler) throws IOException { - final GrizzlyResponseFuture future = - new GrizzlyResponseFuture(this, request, handler); + final ProxyServer proxy = ProxyUtils.getProxyServer(clientConfig, request); + final GrizzlyResponseFuture future = new GrizzlyResponseFuture(this, request, handler, proxy); future.setDelegate(SafeFutureImpl.create()); final CompletionHandler connectHandler = new CompletionHandler() { @Override @@ -816,24 +816,6 @@ public NextAction handleEvent(final FilterChainContext ctx, } -// @Override -// public NextAction handleRead(FilterChainContext ctx) throws IOException { -// Object message = ctx.getMessage(); -// if (HttpPacket.isHttp(message)) { -// final HttpPacket packet = (HttpPacket) message; -// HttpResponsePacket responsePacket; -// if (HttpContent.isContent(packet)) { -// responsePacket = (HttpResponsePacket) ((HttpContent) packet).getHttpHeader(); -// } else { -// responsePacket = (HttpResponsePacket) packet; -// } -// if (HttpStatus.SWITCHING_PROTOCOLS_101.statusMatches(responsePacket.getStatus())) { -// return ctx.getStopAction(); -// } -// } -// return super.handleRead(ctx); -// } - // ----------------------------------------------------- Private Methods @@ -861,9 +843,8 @@ private boolean sendAsGrizzlyRequest(final Request request, builder.header(Header.Host, uri.getHost() + ':' + uri.getPort()); } } - final ProxyServer proxy = getProxyServer(request); - boolean avoidProxy = ProxyUtils.avoidProxy(proxy, request); - final boolean useProxy = !(avoidProxy || proxy == null); + final ProxyServer proxy = ProxyUtils.getProxyServer(config, request); + final boolean useProxy = proxy != null; if (useProxy) { if ((secure || httpCtx.isWSRequest) && !httpCtx.isTunnelEstablished(ctx.getConnection())) { secure = false; @@ -956,18 +937,6 @@ private void convertToUpgradeRequest(final HttpTransactionContext ctx) { ctx.requestUrl = sb.toString(); } - - private ProxyServer getProxyServer(Request request) { - - ProxyServer proxyServer = request.getProxyServer(); - if (proxyServer == null) { - proxyServer = config.getProxyServer(); - } - return proxyServer; - - } - - private void addHeaders(final Request request, final HttpRequestPacket requestPacket) { @@ -2329,8 +2298,7 @@ Connection obtainConnection(final Request request, final GrizzlyResponseFuture requestFuture) throws IOException, ExecutionException, InterruptedException, TimeoutException { - final Connection c = (obtainConnection0(request, - requestFuture)); + final Connection c = obtainConnection0(request, requestFuture); DO_NOT_CACHE.set(c, Boolean.TRUE); return c; @@ -2341,10 +2309,7 @@ void doAsyncConnect(final Request request, final CompletionHandler connectHandler) throws IOException, ExecutionException, InterruptedException { - ProxyServer proxy = getProxyServer(request); - if (ProxyUtils.avoidProxy(proxy, request)) { - proxy = null; - } + ProxyServer proxy = requestFuture.getProxy(); final URI uri = request.getURI(); String host = ((proxy != null) ? proxy.getHost() : uri.getHost()); int port = ((proxy != null) ? proxy.getPort() : uri.getPort()); @@ -2363,10 +2328,7 @@ private Connection obtainConnection0(final Request request, throws IOException, ExecutionException, InterruptedException, TimeoutException { final URI uri = request.getURI(); - ProxyServer proxy = getProxyServer(request); - if (ProxyUtils.avoidProxy(proxy, request)) { - proxy = null; - } + final ProxyServer proxy = requestFuture.getProxy(); String host = ((proxy != null) ? proxy.getHost() : uri.getHost()); int port = ((proxy != null) ? proxy.getPort() : uri.getPort()); int cTimeout = provider.clientConfig.getConnectionTimeoutInMs(); diff --git a/src/main/java/com/ning/http/client/providers/grizzly/GrizzlyResponseFuture.java b/src/main/java/com/ning/http/client/providers/grizzly/GrizzlyResponseFuture.java index fc17b39f16..d4116ad4c0 100644 --- a/src/main/java/com/ning/http/client/providers/grizzly/GrizzlyResponseFuture.java +++ b/src/main/java/com/ning/http/client/providers/grizzly/GrizzlyResponseFuture.java @@ -14,6 +14,7 @@ package com.ning.http.client.providers.grizzly; import com.ning.http.client.AsyncHandler; +import com.ning.http.client.ProxyServer; import com.ning.http.client.Request; import com.ning.http.client.listenable.AbstractListenableFuture; @@ -42,7 +43,7 @@ public class GrizzlyResponseFuture extends AbstractListenableFuture { private final AsyncHandler handler; private final GrizzlyAsyncHttpProvider provider; private final Request request; - + private final ProxyServer proxy; private Connection connection; FutureImpl delegate; @@ -53,12 +54,13 @@ public class GrizzlyResponseFuture extends AbstractListenableFuture { GrizzlyResponseFuture(final GrizzlyAsyncHttpProvider provider, final Request request, - final AsyncHandler handler) { + final AsyncHandler handler, + final ProxyServer proxy) { this.provider = provider; this.request = request; this.handler = handler; - + this.proxy = proxy; } @@ -208,4 +210,7 @@ private void closeConnection() { } + public ProxyServer getProxy() { + return proxy; + } } diff --git a/src/main/java/com/ning/http/client/providers/jdk/JDKAsyncHttpProvider.java b/src/main/java/com/ning/http/client/providers/jdk/JDKAsyncHttpProvider.java index 7552246838..c413d13bd7 100644 --- a/src/main/java/com/ning/http/client/providers/jdk/JDKAsyncHttpProvider.java +++ b/src/main/java/com/ning/http/client/providers/jdk/JDKAsyncHttpProvider.java @@ -131,11 +131,10 @@ public ListenableFuture execute(Request request, AsyncHandler handler, throw new IOException(String.format("Too many connections %s", config.getMaxTotalConnections())); } - ProxyServer proxyServer = request.getProxyServer() != null ? request.getProxyServer() : config.getProxyServer(); + ProxyServer proxyServer = ProxyUtils.getProxyServer(config, request); Realm realm = request.getRealm() != null ? request.getRealm() : config.getRealm(); - boolean avoidProxy = ProxyUtils.avoidProxy(proxyServer, request); Proxy proxy = null; - if (!avoidProxy && (proxyServer != null || realm != null)) { + if (proxyServer != null || realm != null) { try { proxy = configureProxyAndAuth(proxyServer, realm); } catch (AuthenticationException e) { @@ -497,7 +496,7 @@ private void configure(URI uri, HttpURLConnection urlConnection, Request request String ka = config.getAllowPoolingConnection() ? "keep-alive" : "close"; urlConnection.setRequestProperty("Connection", ka); - ProxyServer proxyServer = request.getProxyServer() != null ? request.getProxyServer() : config.getProxyServer(); + ProxyServer proxyServer = ProxyUtils.getProxyServer(config, request); boolean avoidProxy = ProxyUtils.avoidProxy(proxyServer, uri.getHost()); if (!avoidProxy) { urlConnection.setRequestProperty("Proxy-Connection", ka); diff --git a/src/main/java/com/ning/http/client/providers/netty/NettyAsyncHttpProvider.java b/src/main/java/com/ning/http/client/providers/netty/NettyAsyncHttpProvider.java index 2419c2f843..b13e3377b7 100644 --- a/src/main/java/com/ning/http/client/providers/netty/NettyAsyncHttpProvider.java +++ b/src/main/java/com/ning/http/client/providers/netty/NettyAsyncHttpProvider.java @@ -595,26 +595,14 @@ public void operationComplete(ChannelFuture cf) { } - private static boolean isProxyServer(AsyncHttpClientConfig config, Request request) { - ProxyServer proxyServer = request.getProxyServer(); - if (proxyServer == null) { - proxyServer = config.getProxyServer(); - } - if (proxyServer == null) { - return false; - } else { - return !ProxyUtils.avoidProxy(proxyServer, request); - } - } - protected final static HttpRequest buildRequest(AsyncHttpClientConfig config, Request request, URI uri, - boolean allowConnect, ChannelBuffer buffer) throws IOException { + boolean allowConnect, ChannelBuffer buffer, ProxyServer proxyServer) throws IOException { String method = request.getMethod(); - if (allowConnect && (isProxyServer(config, request) && isSecure(uri))) { + if (allowConnect && proxyServer != null && isSecure(uri)) { method = HttpMethod.CONNECT.toString(); } - return construct(config, request, new HttpMethod(method), uri, buffer); + return construct(config, request, new HttpMethod(method), uri, buffer, proxyServer); } private static SpnegoEngine getSpnegoEngine() { @@ -627,7 +615,8 @@ private static HttpRequest construct(AsyncHttpClientConfig config, Request request, HttpMethod m, URI uri, - ChannelBuffer buffer) throws IOException { + ChannelBuffer buffer, + ProxyServer proxyServer) throws IOException { String host = AsyncHttpProviderUtils.getHost(uri); @@ -640,7 +629,7 @@ private static HttpRequest construct(AsyncHttpClientConfig config, nettyRequest = new DefaultHttpRequest(HttpVersion.HTTP_1_0, m, AsyncHttpProviderUtils.getAuthority(uri)); } else { String path = null; - if (isProxyServer(config, request)) + if (proxyServer != null) path = uri.toString(); else if (uri.getRawQuery() != null) path = uri.getRawPath() + "?" + uri.getRawQuery(); @@ -690,7 +679,6 @@ else if (uri.getRawQuery() != null) nettyRequest.addHeader(HttpHeaders.Names.PROXY_AUTHORIZATION, auth.get(0)); } } - ProxyServer proxyServer = request.getProxyServer() != null ? request.getProxyServer() : config.getProxyServer(); Realm realm = request.getRealm() != null ? request.getRealm() : config.getRealm(); if (realm != null && realm.getUsePreemptiveAuth()) { @@ -754,8 +742,7 @@ else if (uri.getRawQuery() != null) nettyRequest.setHeader(HttpHeaders.Names.CONNECTION, "keep-alive"); } - boolean avoidProxy = ProxyUtils.avoidProxy(proxyServer, request); - if (!avoidProxy) { + if (proxyServer != null) { if (!request.getHeaders().containsKey("Proxy-Connection")) { nettyRequest.setHeader("Proxy-Connection", "keep-alive"); } @@ -958,7 +945,9 @@ private ListenableFuture doConnect(final Request request, final AsyncHand throw new IOException("WebSocket method must be a GET"); } - ProxyServer proxyServer = request.getProxyServer() != null ? request.getProxyServer() : config.getProxyServer(); + ProxyServer proxyServer = ProxyUtils.getProxyServer(config, request); + boolean useProxy = proxyServer != null; + URI uri; if (useRawUrl) { uri = request.getRawURI(); @@ -981,17 +970,14 @@ private ListenableFuture doConnect(final Request request, final AsyncHand bufferedBytes = f.getNettyRequest().getContent(); } - boolean avoidProxy = ProxyUtils.avoidProxy(proxyServer, uri.getHost()); - boolean useProxy = !(avoidProxy || proxyServer == null); - boolean useSSl = isSecure(uri) && !useProxy; if (channel != null && channel.isOpen() && channel.isConnected()) { - HttpRequest nettyRequest = buildRequest(config, request, uri, f == null ? false : f.isConnectAllowed(), bufferedBytes); + HttpRequest nettyRequest = buildRequest(config, request, uri, f == null ? false : f.isConnectAllowed(), bufferedBytes, proxyServer); if (f == null) { - f = newFuture(uri, request, asyncHandler, nettyRequest, config, this); + f = newFuture(uri, request, asyncHandler, nettyRequest, config, this, proxyServer); } else { - nettyRequest = buildRequest(config, request, uri, f.isConnectAllowed(), bufferedBytes); + nettyRequest = buildRequest(config, request, uri, f.isConnectAllowed(), bufferedBytes, proxyServer); f.setNettyRequest(nettyRequest); } f.setState(NettyResponseFuture.STATE.POOLED); @@ -1729,10 +1715,11 @@ public static NettyResponseFuture newFuture(URI uri, AsyncHandler asyncHandler, HttpRequest nettyRequest, AsyncHttpClientConfig config, - NettyAsyncHttpProvider provider) { + NettyAsyncHttpProvider provider, + ProxyServer proxyServer) { NettyResponseFuture f = new NettyResponseFuture(uri, request, asyncHandler, nettyRequest, - requestTimeout(config, request.getPerRequestConfig()), config.getIdleConnectionTimeoutInMs(), provider, request.getConnectionPoolKeyStrategy()); + requestTimeout(config, request.getPerRequestConfig()), config.getIdleConnectionTimeoutInMs(), provider, request.getConnectionPoolKeyStrategy(), proxyServer); if (request.getHeaders().getFirstValue("Expect") != null && request.getHeaders().getFirstValue("Expect").equalsIgnoreCase("100-Continue")) { @@ -2145,6 +2132,7 @@ public void handle(final ChannelHandlerContext ctx, final MessageEvent e) throws HttpRequest nettyRequest = future.getNettyRequest(); AsyncHandler handler = future.getAsyncHandler(); Request request = future.getRequest(); + ProxyServer proxyServer = future.getProxyServer(); HttpResponse response = null; try { if (e.getMessage() instanceof HttpResponse) { @@ -2194,7 +2182,6 @@ public void handle(final ChannelHandlerContext ctx, final MessageEvent e) throws } Realm newRealm = null; - ProxyServer proxyServer = request.getProxyServer() != null ? request.getProxyServer() : config.getProxyServer(); final FluentCaseInsensitiveStringsMap headers = request.getHeaders(); final RequestBuilder builder = new RequestBuilder(future.getRequest()); diff --git a/src/main/java/com/ning/http/client/providers/netty/NettyConnectListener.java b/src/main/java/com/ning/http/client/providers/netty/NettyConnectListener.java index 301399c36e..8625643cd7 100644 --- a/src/main/java/com/ning/http/client/providers/netty/NettyConnectListener.java +++ b/src/main/java/com/ning/http/client/providers/netty/NettyConnectListener.java @@ -18,8 +18,11 @@ import com.ning.http.client.AsyncHandler; import com.ning.http.client.AsyncHttpClientConfig; +import com.ning.http.client.ProxyServer; import com.ning.http.client.Request; import com.ning.http.util.AllowAllHostnameVerifier; +import com.ning.http.util.ProxyUtils; + import org.jboss.netty.buffer.ChannelBuffer; import org.jboss.netty.channel.Channel; import org.jboss.netty.channel.ChannelFuture; @@ -137,9 +140,10 @@ public Builder(AsyncHttpClientConfig config, Request request, AsyncHandler as } public NettyConnectListener build(final URI uri) throws IOException { - HttpRequest nettyRequest = NettyAsyncHttpProvider.buildRequest(config, request, uri, true, buffer); + ProxyServer proxyServer = ProxyUtils.getProxyServer(config, request); + HttpRequest nettyRequest = NettyAsyncHttpProvider.buildRequest(config, request, uri, true, buffer, proxyServer); if (future == null) { - future = NettyAsyncHttpProvider.newFuture(uri, request, asyncHandler, nettyRequest, config, provider); + future = NettyAsyncHttpProvider.newFuture(uri, request, asyncHandler, nettyRequest, config, provider, proxyServer); } else { future.setNettyRequest(nettyRequest); future.setRequest(request); diff --git a/src/main/java/com/ning/http/client/providers/netty/NettyResponseFuture.java b/src/main/java/com/ning/http/client/providers/netty/NettyResponseFuture.java index 1a21b6793c..560ed65e4f 100755 --- a/src/main/java/com/ning/http/client/providers/netty/NettyResponseFuture.java +++ b/src/main/java/com/ning/http/client/providers/netty/NettyResponseFuture.java @@ -17,6 +17,7 @@ import com.ning.http.client.AsyncHandler; import com.ning.http.client.ConnectionPoolKeyStrategy; +import com.ning.http.client.ProxyServer; import com.ning.http.client.Request; import com.ning.http.client.listenable.AbstractListenableFuture; import org.jboss.netty.channel.Channel; @@ -87,6 +88,7 @@ enum STATE { private final AtomicBoolean throwableCalled = new AtomicBoolean(false); private boolean allowConnect = false; private final ConnectionPoolKeyStrategy connectionPoolKeyStrategy; + private final ProxyServer proxyServer; public NettyResponseFuture(URI uri, Request request, @@ -95,7 +97,8 @@ public NettyResponseFuture(URI uri, int responseTimeoutInMs, int idleConnectionTimeoutInMs, NettyAsyncHttpProvider asyncHttpProvider, - ConnectionPoolKeyStrategy connectionPoolKeyStrategy) { + ConnectionPoolKeyStrategy connectionPoolKeyStrategy, + ProxyServer proxyServer) { this.asyncHandler = asyncHandler; this.responseTimeoutInMs = responseTimeoutInMs; @@ -105,6 +108,7 @@ public NettyResponseFuture(URI uri, this.uri = uri; this.asyncHttpProvider = asyncHttpProvider; this.connectionPoolKeyStrategy = connectionPoolKeyStrategy; + this.proxyServer = proxyServer; if (System.getProperty(MAX_RETRY) != null) { maxRetry = Integer.valueOf(System.getProperty(MAX_RETRY)); @@ -127,6 +131,10 @@ public ConnectionPoolKeyStrategy getConnectionPoolKeyStrategy() { return connectionPoolKeyStrategy; } + public ProxyServer getProxyServer() { + return proxyServer; + } + /** * {@inheritDoc} */ diff --git a/src/main/java/com/ning/http/util/ProxyUtils.java b/src/main/java/com/ning/http/util/ProxyUtils.java index ea448b6272..cf03ab78a5 100644 --- a/src/main/java/com/ning/http/util/ProxyUtils.java +++ b/src/main/java/com/ning/http/util/ProxyUtils.java @@ -12,6 +12,7 @@ */ package com.ning.http.util; +import com.ning.http.client.AsyncHttpClientConfig; import com.ning.http.client.ProxyServer; import com.ning.http.client.ProxyServer.Protocol; import com.ning.http.client.Request; @@ -58,6 +59,19 @@ public class ProxyUtils { */ public static final String PROXY_PASSWORD = PROPERTY_PREFIX + "password"; + /** + * @param config the global config + * @param request the request + * @return the proxy server to be used for this request (can be null) + */ + public static ProxyServer getProxyServer(AsyncHttpClientConfig config, Request request) { + ProxyServer proxyServer = request.getProxyServer(); + if (proxyServer == null) { + proxyServer = config.getProxyServer(); + } + return ProxyUtils.avoidProxy(proxyServer, request) ? null : proxyServer; + } + /** * Checks whether proxy should be used according to nonProxyHosts settings of it, or we want to go directly to * target host. If null proxy is passed in, this method returns true -- since there is NO proxy, we diff --git a/src/test/java/com/ning/http/client/async/ProxyTest.java b/src/test/java/com/ning/http/client/async/ProxyTest.java index f73d84d639..385bee937f 100644 --- a/src/test/java/com/ning/http/client/async/ProxyTest.java +++ b/src/test/java/com/ning/http/client/async/ProxyTest.java @@ -125,6 +125,21 @@ public void testNonProxyHosts() throws IOException, ExecutionException, TimeoutE } } + @Test(groups = { "standalone", "default_provider" }) + public void testNonProxyHostIssue202() throws IOException, ExecutionException, TimeoutException, InterruptedException { + AsyncHttpClient client = getAsyncHttpClient(null); + try { + String target = "http://127.0.0.1/" + port1 + "/"; + Future f = client.prepareGet(target).setProxyServer(new ProxyServer("127.0.0.1", port1 - 1).addNonProxyHost("127.0.0.1")).execute(); + Response resp = f.get(3, TimeUnit.SECONDS); + assertNotNull(resp); + assertEquals(resp.getStatusCode(), HttpServletResponse.SC_OK); + assertEquals(resp.getHeader("target"), "/"); + } finally { + client.close(); + } + } + @Test(groups = { "standalone", "default_provider" }) public void testProxyProperties() throws IOException, ExecutionException, TimeoutException, InterruptedException { Properties originalProps = System.getProperties();