Skip to content

Connection pooling broken when using proxy over SSL #364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
andywokr opened this issue Aug 26, 2013 · 7 comments
Closed

Connection pooling broken when using proxy over SSL #364

andywokr opened this issue Aug 26, 2013 · 7 comments
Assignees
Labels
Milestone

Comments

@andywokr
Copy link

The connection pool logic in NettyAsyncHttpProvider does not appear to be working when proxying over SSL.

URI connectionKeyUri = useProxy? proxyServer.getURI() : uri;
channel = lookupInCache(connectionKeyUri, request.getConnectionPoolKeyStrategy());

What appears to be happening is that request #1 creates a new connection keyed to the proxy address and properly issues an HTTP CONNECT for the first host. Request #2 targets a different host via the same proxy, pulls the cached connection since the proxy address is the same but then ends up sending the request to wrong host since that connection is still connected to the first host.

I can hack up NettyAsyncHttpProvider.java if needed by creating a fake URI in the format : scheme://targetHost.proxyHost:proxyPort/ when ssl + proxy is used. The proper solution however would seem to require changing the ConnectionPoolKeyStrategy interface since a URI alone is not enough to correctly compute the pool key.

The following code will re-create the problem.

Builder builder = new AsyncHttpClientConfig.Builder()
    .setAllowPoolingConnection(true);
    .setAllowSslConnectionPool(true);
try {
    // provide our own SSLContext that accepts all certicates
    SSLContext sslContext = SSLContext.getInstance("SSL");
    sslContext.init(null, new TrustManager[] { new X509ExtendedTrustManager() {
        public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
        }

        public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
        }

        public X509Certificate[] getAcceptedIssuers() {
            return null;
        }

        public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException {
        }

        public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException {
        }

        public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException {
        }

        public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException {
        }
    } }, null);
    builder.setSSLContext(sslContext);
} catch (Exception e) {
    LOGGER.error("SSL not supported by JVM!", e);
}

AsyncHttpClient httpClient = new AsyncHttpClient(builder.build());

RequestBuilder requestBuilder1 = new RequestBuilder("GET")
    .setUrl("https://www.google.com/");
    .setProxyServer(new ProxyServer(ProxyServer.Protocol.HTTPS, args[0], Integer.parseInt(args[1])));

httpClient.executeRequest(requestBuilder1.build(), new AsyncHandler<Response>() {
    private final Response.ResponseBuilder builder = new Response.ResponseBuilder();

    public void onThrowable(Throwable t) {
    }

    public AsyncHandler.STATE onBodyPartReceived(HttpResponseBodyPart bodyPart) throws Exception {
        builder.accumulate(bodyPart);
        return STATE.CONTINUE;
    }

    public AsyncHandler.STATE onStatusReceived(HttpResponseStatus status) throws Exception {
        builder.accumulate(status);
        return STATE.CONTINUE;
    }

    public AsyncHandler.STATE onHeadersReceived(HttpResponseHeaders headers) throws Exception {
        builder.accumulate(headers);
        return STATE.CONTINUE;
    }

    public Response onCompleted() throws Exception {
        Response response = builder.build();
        System.out.println(response.getStatusCode());
        System.out.println(response.getResponseBody());
        return response;
    }
});

RequestBuilder requestBuilder2 = new RequestBuilder("GET")
    .setUrl("https://www.facebook.com/");
    .setProxyServer(new ProxyServer(ProxyServer.Protocol.HTTPS, args[0], Integer.parseInt(args[1])));

httpClient.executeRequest(requestBuilder2.build(), new AsyncHandler<Response>() {
    private final Response.ResponseBuilder builder = new Response.ResponseBuilder();

    public void onThrowable(Throwable t) {
    }

    public AsyncHandler.STATE onBodyPartReceived(HttpResponseBodyPart bodyPart) throws Exception {
        builder.accumulate(bodyPart);
        return STATE.CONTINUE;
    }

    public AsyncHandler.STATE onStatusReceived(HttpResponseStatus status) throws Exception {
        builder.accumulate(status);
        return STATE.CONTINUE;
    }

    public AsyncHandler.STATE onHeadersReceived(HttpResponseHeaders headers) throws Exception {
        builder.accumulate(headers);
        return STATE.CONTINUE;
    }

    public Response onCompleted() throws Exception {
        Response response = builder.build();
        System.out.println(response.getStatusCode());
        System.out.println(response.getResponseBody());
        return response;
    }
});
@peterox
Copy link

peterox commented Nov 18, 2013

You are seeing the google content twice because you are passing requestBuilder1.build() to executeRequest both times.

@slandelle
Copy link
Contributor

Like @peterox said, there might be an issue with connection pool w/ proxies, but the example you provided is flawed and you send requestBuilder1.build() instead requestBuilder2.build() for the second request.

@McManCSU
Copy link

McManCSU commented Dec 6, 2013

I too have seen this problem before in our setup (http://stackoverflow.com/questions/18021722/ssl-and-http-proxy-with-java-client-issues-connectionclosedexception). Another symptom is that the response body of the messages would be scrambled. Our simplest solution was to create a new connection for each SSL termination (disable the ssl conn pool - terribly inefficient I know). As you might expect, we more recently we have been having problems with the proliferation of connections - I'd love to see this fixed!

@ghost ghost assigned slandelle Jan 22, 2014
@slandelle
Copy link
Contributor

Fixing sample code to properly demonstrate problem.

@slandelle
Copy link
Contributor

What if we make the pool key proxyURI (optional) + connectionPoolKeyStrategy.getKey(requestURI)?

@jfarcand
Copy link
Contributor

@slandelle That would work :-)

@slandelle
Copy link
Contributor

Will go this way for AHC 1, not to break connectionPoolKeyStrategy, and will change connectionPoolKeyStrategy in AHC 2 so that it takes the proxyURI.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants