Skip to content

Commit 056127b

Browse files
author
Stephane Landelle
committed
Ensured Netty provider uses CONNECT when proxying ws, port AsyncHttpClient#657 on master
1 parent 148f202 commit 056127b

File tree

11 files changed

+68
-57
lines changed

11 files changed

+68
-57
lines changed

api/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public class AsyncHttpClientConfig {
9191
protected boolean strict302Handling;
9292

9393
protected ProxyServerSelector proxyServerSelector;
94-
protected boolean useRelativeURIsWithSSLProxies;
94+
protected boolean useRelativeURIsWithConnectProxies;
9595

9696
protected boolean compressionEnabled;
9797
protected String userAgent;
@@ -133,7 +133,7 @@ private AsyncHttpClientConfig(int connectionTimeout,//
133133
boolean strict302Handling, //
134134
ExecutorService applicationThreadPool,//
135135
ProxyServerSelector proxyServerSelector, //
136-
boolean useRelativeURIsWithSSLProxies, //
136+
boolean useRelativeURIsWithConnectProxies, //
137137
boolean compressionEnabled, //
138138
String userAgent,//
139139
Realm realm,//
@@ -167,7 +167,7 @@ private AsyncHttpClientConfig(int connectionTimeout,//
167167
this.removeQueryParamOnRedirect = removeQueryParamOnRedirect;
168168
this.strict302Handling = strict302Handling;
169169
this.proxyServerSelector = proxyServerSelector;
170-
this.useRelativeURIsWithSSLProxies = useRelativeURIsWithSSLProxies;
170+
this.useRelativeURIsWithConnectProxies = useRelativeURIsWithConnectProxies;
171171
this.compressionEnabled = compressionEnabled;
172172
this.userAgent = userAgent;
173173
this.applicationThreadPool = applicationThreadPool == null ? Executors.newCachedThreadPool() : applicationThreadPool;
@@ -493,13 +493,13 @@ public boolean isStrict302Handling() {
493493
}
494494

495495
/**
496-
* @return<code>true</code> if AHC should use relative URIs instead of absolute ones when talking with a SSL proxy,
497-
* otherwise <code>false</code>.
496+
* @return<code>true</code> if AHC should use relative URIs instead of absolute ones when talking with a SSL proxy
497+
* or WebSocket proxy, otherwise <code>false</code>.
498498
*
499-
* @since 1.7.12
499+
* @since 1.8.13
500500
*/
501-
public boolean isUseRelativeURIsWithSSLProxies() {
502-
return useRelativeURIsWithSSLProxies;
501+
public boolean isUseRelativeURIsWithConnectProxies() {
502+
return useRelativeURIsWithConnectProxies;
503503
}
504504

505505
/**
@@ -548,7 +548,7 @@ public static class Builder {
548548
private ProxyServerSelector proxyServerSelector = null;
549549
private boolean useProxySelector = defaultUseProxySelector();
550550
private boolean useProxyProperties = defaultUseProxyProperties();
551-
private boolean useRelativeURIsWithSSLProxies = defaultUseRelativeURIsWithSSLProxies();
551+
private boolean useRelativeURIsWithConnectProxies = defaultUseRelativeURIsWithConnectProxies();
552552
private boolean compressionEnabled = defaultCompressionEnabled();
553553
private String userAgent = defaultUserAgent();
554554
private ExecutorService applicationThreadPool;
@@ -962,15 +962,15 @@ public Builder setConnectionTTL(int connectionTTL) {
962962
}
963963

964964
/**
965-
* Configures this AHC instance to use relative URIs instead of absolute ones when talking with a SSL proxy.
965+
* Configures this AHC instance to use relative URIs instead of absolute ones when talking with a SSL or WebSocket proxy.
966966
*
967-
* @param useRelativeURIsWithSSLProxies
967+
* @param useRelativeURIsWithConnectProxies
968968
* @return this
969969
*
970970
* @since 1.7.2
971971
*/
972-
public Builder setUseRelativeURIsWithSSLProxies(boolean useRelativeURIsWithSSLProxies) {
973-
this.useRelativeURIsWithSSLProxies = useRelativeURIsWithSSLProxies;
972+
public Builder setUseRelativeURIsWithConnectProxies(boolean useRelativeURIsWithConnectProxies) {
973+
this.useRelativeURIsWithConnectProxies = useRelativeURIsWithConnectProxies;
974974
return this;
975975
}
976976

@@ -1116,7 +1116,7 @@ else if (hostnameVerifier == null)
11161116
strict302Handling, //
11171117
applicationThreadPool, //
11181118
proxyServerSelector, //
1119-
useRelativeURIsWithSSLProxies, //
1119+
useRelativeURIsWithConnectProxies, //
11201120
compressionEnabled, //
11211121
userAgent,//
11221122
realm,//

api/src/main/java/org/asynchttpclient/AsyncHttpClientConfigBean.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ void configureDefaults() {
5858
compressionEnabled = defaultCompressionEnabled();
5959
userAgent = defaultUserAgent();
6060
allowPoolingConnections = defaultAllowPoolingConnections();
61-
useRelativeURIsWithSSLProxies = defaultUseRelativeURIsWithSSLProxies();
61+
useRelativeURIsWithConnectProxies = defaultUseRelativeURIsWithConnectProxies();
6262
maxRequestRetry = defaultMaxRequestRetry();
6363
ioThreadMultiplier = defaultIoThreadMultiplier();
6464
allowPoolingSslConnections = defaultAllowPoolingSslConnections();

api/src/main/java/org/asynchttpclient/AsyncHttpClientConfigDefaults.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public static boolean defaultCompressionEnabled() {
6666
}
6767

6868
public static String defaultUserAgent() {
69-
return System.getProperty(ASYNC_CLIENT + "userAgent", "NING/1.0");
69+
return System.getProperty(ASYNC_CLIENT + "userAgent", "AHC/2.0");
7070
}
7171

7272
public static int defaultIoThreadMultiplier() {
@@ -89,8 +89,8 @@ public static boolean defaultAllowPoolingConnections() {
8989
return getBoolean(ASYNC_CLIENT + "allowPoolingConnections", true);
9090
}
9191

92-
public static boolean defaultUseRelativeURIsWithSSLProxies() {
93-
return getBoolean(ASYNC_CLIENT + "useRelativeURIsWithSSLProxies", true);
92+
public static boolean defaultUseRelativeURIsWithConnectProxies() {
93+
return getBoolean(ASYNC_CLIENT + "useRelativeURIsWithConnectProxies", true);
9494
}
9595

9696
public static int defaultMaxRequestRetry() {

api/src/main/java/org/asynchttpclient/util/AsyncHttpProviderUtils.java

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,16 @@
1414

1515
import static org.asynchttpclient.util.MiscUtils.isNonEmpty;
1616

17-
import org.asynchttpclient.AsyncHttpClientConfig;
18-
import org.asynchttpclient.AsyncHttpProvider;
19-
import org.asynchttpclient.HttpResponseBodyPart;
20-
import org.asynchttpclient.Request;
21-
import org.asynchttpclient.uri.UriComponents;
22-
2317
import java.io.IOException;
2418
import java.io.UnsupportedEncodingException;
2519
import java.nio.charset.Charset;
2620
import java.util.List;
2721

22+
import org.asynchttpclient.AsyncHttpClientConfig;
23+
import org.asynchttpclient.HttpResponseBodyPart;
24+
import org.asynchttpclient.Request;
25+
import org.asynchttpclient.uri.UriComponents;
26+
2827
/**
2928
* {@link org.asynchttpclient.AsyncHttpProvider} common utilities.
3029
*/
@@ -113,12 +112,6 @@ public final static String getNonEmptyPath(UriComponents uri) {
113112
return isNonEmpty(uri.getPath()) ? uri.getPath() : "/";
114113
}
115114

116-
public static String constructUserAgent(Class<? extends AsyncHttpProvider> httpProvider, AsyncHttpClientConfig config) {
117-
return new StringBuilder("AHC (").append(httpProvider.getSimpleName()).append(" - ").append(System.getProperty("os.name"))
118-
.append(" - ").append(System.getProperty("os.version")).append(" - ").append(System.getProperty("java.version"))
119-
.append(" - ").append(Runtime.getRuntime().availableProcessors()).append(" core(s))").toString();
120-
}
121-
122115
public static String parseCharset(String contentType) {
123116
for (String part : contentType.split(";")) {
124117
if (part.trim().startsWith("charset=")) {

api/src/test/java/org/asynchttpclient/websocket/ProxyTunnellingTest.java

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,37 +17,35 @@
1717
import static org.asynchttpclient.async.util.TestUtils.newJettyHttpsServer;
1818
import static org.testng.Assert.assertEquals;
1919

20+
import java.util.concurrent.CountDownLatch;
21+
import java.util.concurrent.atomic.AtomicReference;
22+
2023
import org.asynchttpclient.AsyncHttpClient;
2124
import org.asynchttpclient.AsyncHttpClientConfig;
2225
import org.asynchttpclient.ProxyServer;
2326
import org.eclipse.jetty.proxy.ConnectHandler;
2427
import org.eclipse.jetty.server.Server;
2528
import org.eclipse.jetty.websocket.server.WebSocketHandler;
2629
import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
27-
import org.testng.annotations.AfterClass;
28-
import org.testng.annotations.BeforeClass;
30+
import org.testng.annotations.AfterMethod;
2931
import org.testng.annotations.Test;
3032

31-
import java.util.concurrent.CountDownLatch;
32-
import java.util.concurrent.atomic.AtomicReference;
33-
3433
/**
3534
* Proxy usage tests.
3635
*/
3736
public abstract class ProxyTunnellingTest extends AbstractBasicTest {
3837

3938
private Server server2;
4039

41-
@BeforeClass(alwaysRun = true)
42-
public void setUpGlobal() throws Exception {
40+
public void setUpServers(boolean targetHttps) throws Exception {
4341
port1 = findFreePort();
4442
server = newJettyHttpServer(port1);
4543
server.setHandler(new ConnectHandler());
4644
server.start();
4745

4846
port2 = findFreePort();
4947

50-
server2 = newJettyHttpsServer(port2);
48+
server2 = targetHttps ? newJettyHttpsServer(port2) : newJettyHttpServer(port2);
5149
server2.setHandler(getWebSocketHandler());
5250
server2.start();
5351

@@ -64,27 +62,37 @@ public void configure(WebSocketServletFactory factory) {
6462
};
6563
}
6664

67-
@AfterClass(alwaysRun = true)
65+
@AfterMethod(alwaysRun = true)
6866
public void tearDownGlobal() throws Exception {
6967
server.stop();
7068
server2.stop();
7169
}
7270

73-
protected String getTargetUrl() {
74-
return String.format("wss://127.0.0.1:%d/", port2);
71+
@Test(timeOut = 60000)
72+
public void echoWSText() throws Exception {
73+
runTest(false);
7574
}
7675

7776
@Test(timeOut = 60000)
78-
public void echoText() throws Exception {
77+
public void echoWSSText() throws Exception {
78+
runTest(true);
79+
}
80+
81+
private void runTest(boolean secure) throws Exception {
82+
83+
setUpServers(secure);
84+
85+
String targetUrl = String.format("%s://127.0.0.1:%d/", secure ? "wss" : "ws", port2);
7986

80-
ProxyServer ps = new ProxyServer(ProxyServer.Protocol.HTTPS, "127.0.0.1", port1);
87+
// CONNECT happens over HTTP, not HTTPS
88+
ProxyServer ps = new ProxyServer(ProxyServer.Protocol.HTTP, "127.0.0.1", port1);
8189
AsyncHttpClientConfig config = new AsyncHttpClientConfig.Builder().setProxyServer(ps).setAcceptAnyCertificate(true).build();
8290
AsyncHttpClient asyncHttpClient = getAsyncHttpClient(config);
8391
try {
8492
final CountDownLatch latch = new CountDownLatch(1);
8593
final AtomicReference<String> text = new AtomicReference<String>("");
8694

87-
WebSocket websocket = asyncHttpClient.prepareGet(getTargetUrl()).execute(new WebSocketUpgradeHandler.Builder().addWebSocketListener(new WebSocketTextListener() {
95+
WebSocket websocket = asyncHttpClient.prepareGet(targetUrl).execute(new WebSocketUpgradeHandler.Builder().addWebSocketListener(new WebSocketTextListener() {
8896

8997
@Override
9098
public void onMessage(String message) {

providers/grizzly/src/main/java/org/asynchttpclient/providers/grizzly/filters/AsyncHttpClientFilter.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ private boolean sendAsGrizzlyRequest(
210210
final Request request = httpTxContext.getRequest();
211211
final UriComponents uri = request.getURI();
212212
boolean secure = Utils.isSecure(uri);
213+
boolean isWebSocket = isWSRequest(httpTxContext.getRequestUri());
213214

214215
// If the request is secure, check to see if an error occurred during
215216
// the handshake. We have to do this here, as the error would occur
@@ -219,7 +220,8 @@ private boolean sendAsGrizzlyRequest(
219220
return true;
220221
}
221222

222-
if (isUpgradeRequest(httpTxContext.getHandler()) && isWSRequest(httpTxContext.getRequestUri())) {
223+
224+
if (isUpgradeRequest(httpTxContext.getHandler()) && isWebSocket) {
223225
httpTxContext.setWSRequest(true);
224226
convertToUpgradeRequest(httpTxContext);
225227
}
@@ -238,8 +240,10 @@ private boolean sendAsGrizzlyRequest(
238240
if (method == Method.CONNECT) {
239241
final int port = uri.getPort();
240242
requestPacket.setRequestURI(uri.getHost() + ':' + (port == -1 ? 443 : port));
241-
} else {
243+
} else if ((secure || isWebSocket) && config.isUseRelativeURIsWithConnectProxies()) {
242244
requestPacket.setRequestURI(getNonEmptyPath(uri));
245+
} else {
246+
requestPacket.setRequestURI(uri.toUrl());
243247
}
244248

245249
final BodyHandler bodyHandler = isPayloadAllowed(method) ?

providers/netty/src/main/java/org/asynchttpclient/providers/netty/handler/HttpProtocol.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ public void handle(final Channel channel, final NettyResponseFuture<?> future, f
405405
try {
406406
if (e instanceof HttpResponse) {
407407
HttpResponse response = (HttpResponse) e;
408-
// FIXME why do we buffer the response? I don't remember...
408+
// we buffer the response until we get the LastHttpContent
409409
future.setPendingResponse(response);
410410
return;
411411

providers/netty/src/main/java/org/asynchttpclient/providers/netty/handler/WebSocketProtocol.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ public void handle(Channel channel, NettyResponseFuture<?> future, Object e) thr
7272

7373
if (e instanceof HttpResponse) {
7474
HttpResponse response = (HttpResponse) e;
75+
// we buffer the response until we get the LastHttpContent
76+
future.setPendingResponse(response);
77+
78+
} else if (e instanceof LastHttpContent) {
79+
HttpResponse response = future.getPendingResponse();
80+
future.setPendingResponse(null);
7581
HttpResponseStatus status = new NettyResponseStatus(future.getURI(), config, response);
7682
HttpResponseHeaders responseHeaders = new NettyResponseHeaders(response.headers());
7783

@@ -151,8 +157,6 @@ public void handle(Channel channel, NettyResponseFuture<?> future, Object e) thr
151157
} else {
152158
logger.debug("UpgradeHandler returned a null NettyWebSocket ");
153159
}
154-
} else if (e instanceof LastHttpContent) {
155-
// FIXME what to do with this kind of messages?
156160
} else {
157161
logger.error("Invalid message {}", e);
158162
}

providers/netty/src/main/java/org/asynchttpclient/providers/netty/request/NettyRequestFactory.java

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
import static org.asynchttpclient.providers.netty.util.HttpUtils.isNTLM;
1717
import static org.asynchttpclient.providers.netty.util.HttpUtils.isSecure;
1818
import static org.asynchttpclient.providers.netty.util.HttpUtils.isWebSocket;
19+
import static org.asynchttpclient.providers.netty.util.HttpUtils.useProxyConnect;
1920
import static org.asynchttpclient.providers.netty.ws.WebSocketUtils.getKey;
2021
import static org.asynchttpclient.util.AsyncHttpProviderUtils.DEFAULT_CHARSET;
21-
import static org.asynchttpclient.util.AsyncHttpProviderUtils.constructUserAgent;
2222
import static org.asynchttpclient.util.AsyncHttpProviderUtils.getAuthority;
2323
import static org.asynchttpclient.util.AsyncHttpProviderUtils.getNonEmptyPath;
2424
import static org.asynchttpclient.util.AsyncHttpProviderUtils.keepAliveHeaderValue;
@@ -49,7 +49,6 @@
4949
import org.asynchttpclient.generators.InputStreamBodyGenerator;
5050
import org.asynchttpclient.ntlm.NTLMEngine;
5151
import org.asynchttpclient.ntlm.NTLMEngineException;
52-
import org.asynchttpclient.providers.netty.NettyAsyncHttpProvider;
5352
import org.asynchttpclient.providers.netty.NettyAsyncHttpProviderConfig;
5453
import org.asynchttpclient.providers.netty.request.body.NettyBody;
5554
import org.asynchttpclient.providers.netty.request.body.NettyBodyBody;
@@ -77,7 +76,7 @@ private String requestUri(UriComponents uri, ProxyServer proxyServer, HttpMethod
7776
if (method == HttpMethod.CONNECT)
7877
return getAuthority(uri);
7978

80-
else if (proxyServer != null && !(isSecure(uri) && config.isUseRelativeURIsWithSSLProxies()))
79+
else if (proxyServer != null && !(useProxyConnect(uri) && config.isUseRelativeURIsWithConnectProxies()))
8180
return uri.toString();
8281

8382
else {
@@ -322,10 +321,8 @@ public NettyRequest newNettyRequest(Request request, UriComponents uri, boolean
322321
httpRequest.headers().set(HttpHeaders.Names.ACCEPT, "*/*");
323322

324323
// Add default user agent
325-
if (!httpRequest.headers().contains(HttpHeaders.Names.USER_AGENT)) {
326-
String userAgent = config.getUserAgent() != null ? config.getUserAgent() : constructUserAgent(NettyAsyncHttpProvider.class, config);
327-
httpRequest.headers().set(HttpHeaders.Names.USER_AGENT, userAgent);
328-
}
324+
if (!httpRequest.headers().contains(HttpHeaders.Names.USER_AGENT) && config.getUserAgent() != null)
325+
httpRequest.headers().set(HttpHeaders.Names.USER_AGENT, config.getUserAgent());
329326

330327
return nettyRequest;
331328
}

providers/netty/src/main/java/org/asynchttpclient/providers/netty/request/NettyRequestSender.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import static org.asynchttpclient.providers.netty.util.HttpUtils.WEBSOCKET;
1717
import static org.asynchttpclient.providers.netty.util.HttpUtils.isSecure;
18+
import static org.asynchttpclient.providers.netty.util.HttpUtils.useProxyConnect;
1819
import static org.asynchttpclient.util.AsyncHttpProviderUtils.getDefaultPort;
1920
import static org.asynchttpclient.util.AsyncHttpProviderUtils.requestTimeout;
2021
import static org.asynchttpclient.util.ProxyUtils.avoidProxy;
@@ -100,7 +101,7 @@ public <T> ListenableFuture<T> sendRequest(final Request request,//
100101
boolean resultOfAConnect = future != null && future.getNettyRequest() != null && future.getNettyRequest().getHttpRequest().getMethod() == HttpMethod.CONNECT;
101102
boolean useProxy = proxyServer != null && !resultOfAConnect;
102103

103-
if (useProxy && isSecure(uri))
104+
if (useProxy && useProxyConnect(uri))
104105
// SSL proxy, have to handle CONNECT
105106
if (future != null && future.isConnectAllowed())
106107
// CONNECT forced

providers/netty/src/main/java/org/asynchttpclient/providers/netty/util/HttpUtils.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,8 @@ public static boolean isSecure(String scheme) {
4343
public static boolean isSecure(UriComponents uri) {
4444
return isSecure(uri.getScheme());
4545
}
46+
47+
public static boolean useProxyConnect(UriComponents uri) {
48+
return isSecure(uri) || isWebSocket(uri.getScheme());
49+
}
4650
}

0 commit comments

Comments
 (0)