Skip to content

Commit 452be89

Browse files
committed
("Follow Redirects requires Extra Connection") Make sure no IOException get thrown when we reclaim a connection for redirection
1 parent f41a122 commit 452be89

File tree

2 files changed

+224
-10
lines changed

2 files changed

+224
-10
lines changed

src/main/java/com/ning/http/client/providers/netty/NettyAsyncHttpProvider.java

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -723,14 +723,19 @@ public Response prepareResponse(final HttpResponseStatus status,
723723
/* @Override */
724724

725725
public <T> ListenableFuture<T> execute(Request request, final AsyncHandler<T> asyncHandler) throws IOException {
726-
return doConnect(request, asyncHandler, null, true, executeConnectAsync);
726+
return doConnect(request, asyncHandler, null, true, executeConnectAsync, false);
727727
}
728728

729729
private <T> void execute(final Request request, final NettyResponseFuture<T> f, boolean useCache, boolean asyncConnect) throws IOException {
730-
doConnect(request, f.getAsyncHandler(), f, useCache, asyncConnect);
730+
doConnect(request, f.getAsyncHandler(), f, useCache, asyncConnect, false);
731731
}
732732

733-
private <T> ListenableFuture<T> doConnect(final Request request, final AsyncHandler<T> asyncHandler, NettyResponseFuture<T> f, boolean useCache, boolean asyncConnect) throws IOException {
733+
private <T> void execute(final Request request, final NettyResponseFuture<T> f, boolean useCache, boolean asyncConnect, boolean reclaimCache) throws IOException {
734+
doConnect(request, f.getAsyncHandler(), f, useCache, asyncConnect, reclaimCache);
735+
}
736+
737+
private <T> ListenableFuture<T> doConnect(final Request request, final AsyncHandler<T> asyncHandler, NettyResponseFuture<T> f,
738+
boolean useCache, boolean asyncConnect, boolean reclaimCache) throws IOException {
734739

735740
if (isClose.get()) {
736741
throw new IOException("Closed");
@@ -783,8 +788,9 @@ private <T> ListenableFuture<T> doConnect(final Request request, final AsyncHand
783788
return f;
784789
}
785790

786-
if (!connectionsPool.canCacheConnection() ||
787-
(config.getMaxTotalConnections() > -1 && (maxConnections.get() + 1) > config.getMaxTotalConnections())) {
791+
// Do not throw an exception when we need an extra connection for a redirect.
792+
if (!reclaimCache && (!connectionsPool.canCacheConnection() ||
793+
(config.getMaxTotalConnections() > -1 && (maxConnections.get() + 1) > config.getMaxTotalConnections()))) {
788794
IOException ex = new IOException(String.format("Too many connections %s", config.getMaxTotalConnections()));
789795
try {
790796
asyncHandler.onThrowable(ex);
@@ -1094,7 +1100,8 @@ public Object call() throws Exception {
10941100

10951101
final RequestBuilder builder = new RequestBuilder(future.getRequest());
10961102
try {
1097-
upgradeProtocol(ctx.getChannel().getPipeline(), request.getUrl(), proxyServer);
1103+
log.debug("Connecting to proxy {} for scheme {}", proxyServer, request.getUrl());
1104+
upgradeProtocol(ctx.getChannel().getPipeline(), request.getUrl());
10981105
} catch (Throwable ex) {
10991106
abort(future, ex);
11001107
}
@@ -1254,7 +1261,7 @@ private void nextRequest(final Request request, final NettyResponseFuture<?> fut
12541261
}
12551262

12561263
private void nextRequest(final Request request, final NettyResponseFuture<?> future, final boolean useCache) throws IOException {
1257-
execute(request, future, useCache, true);
1264+
execute(request, future, useCache, true, true);
12581265
}
12591266

12601267
private void abort(NettyResponseFuture<?> future, Throwable t) {
@@ -1269,13 +1276,11 @@ private void abort(NettyResponseFuture<?> future, Throwable t) {
12691276
future.abort(t);
12701277
}
12711278

1272-
private void upgradeProtocol(ChannelPipeline p, String scheme, ProxyServer proxyServer) throws IOException, GeneralSecurityException {
1279+
private void upgradeProtocol(ChannelPipeline p, String scheme) throws IOException, GeneralSecurityException {
12731280
if (p.get(HTTP_HANDLER) != null) {
12741281
p.remove(HTTP_HANDLER);
12751282
}
12761283

1277-
log.debug("Connecting to proxy {} for scheme {}", proxyServer, scheme);
1278-
12791284
if (scheme.startsWith(HTTPS)) {
12801285
if (p.get(SSL_HANDLER) == null) {
12811286
p.addFirst(HTTP_HANDLER, new HttpClientCodec());
Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
/*
2+
* Copyright 2010 Ning, Inc.
3+
*
4+
* Ning licenses this file to you under the Apache License, version 2.0
5+
* (the "License"); you may not use this file except in compliance with the
6+
* License. You may obtain a copy of the License at:
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
package com.ning.http.client.async;
17+
18+
import com.ning.http.client.AsyncHttpClient;
19+
import com.ning.http.client.AsyncHttpClientConfig;
20+
import com.ning.http.client.ListenableFuture;
21+
import com.ning.http.client.RequestBuilder;
22+
import com.ning.http.client.Response;
23+
import com.ning.http.client.providers.netty.NettyAsyncHttpProviderConfig;
24+
import org.eclipse.jetty.server.Connector;
25+
import org.eclipse.jetty.server.Server;
26+
import org.eclipse.jetty.server.nio.SelectChannelConnector;
27+
import org.eclipse.jetty.servlet.ServletContextHandler;
28+
import org.eclipse.jetty.servlet.ServletHolder;
29+
import org.testng.annotations.AfterClass;
30+
import org.testng.annotations.BeforeClass;
31+
import org.testng.annotations.Test;
32+
33+
import javax.servlet.ServletException;
34+
import javax.servlet.http.HttpServlet;
35+
import javax.servlet.http.HttpServletRequest;
36+
import javax.servlet.http.HttpServletResponse;
37+
import java.io.IOException;
38+
import java.io.OutputStream;
39+
import java.net.ServerSocket;
40+
import java.util.Date;
41+
42+
import static org.testng.Assert.assertEquals;
43+
import static org.testng.Assert.assertNotNull;
44+
import static org.testng.FileAssert.fail;
45+
46+
/**
47+
* Test for multithreaded url fetcher calls that use two separate
48+
* sets of ssl certificates. This then tests that the certificate
49+
* settings do not clash (override each other), resulting in the
50+
* peer not authenticated exception
51+
*
52+
* @author dominict
53+
*/
54+
public class RedirectConnectionUsageTest {
55+
private String BASE_URL;
56+
57+
private String servletEndpointRedirectUrl;
58+
59+
private Server server;
60+
61+
private int port1;
62+
63+
public static int findFreePort() throws IOException {
64+
ServerSocket socket = null;
65+
66+
try {
67+
// 0 is open a socket on any free port
68+
socket = new ServerSocket(0);
69+
return socket.getLocalPort();
70+
} finally {
71+
if (socket != null) {
72+
socket.close();
73+
}
74+
}
75+
}
76+
77+
78+
@BeforeClass
79+
public void setUp() throws Exception {
80+
server = new Server();
81+
82+
port1 = findFreePort();
83+
84+
Connector listener = new SelectChannelConnector();
85+
listener.setHost("localhost");
86+
listener.setPort(port1);
87+
88+
server.addConnector(listener);
89+
90+
91+
ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS);
92+
93+
context.setContextPath("/");
94+
server.setHandler(context);
95+
96+
context.addServlet(new ServletHolder(new MockRedirectHttpServlet()), "/redirect/*");
97+
context.addServlet(new ServletHolder(new MockFullResponseHttpServlet()), "/*");
98+
99+
server.start();
100+
101+
BASE_URL = "http://localhost" + ":" + port1;
102+
servletEndpointRedirectUrl = BASE_URL + "/redirect";
103+
104+
}
105+
106+
@AfterClass
107+
public void tearDown() {
108+
try {
109+
if (server != null) {
110+
server.stop();
111+
}
112+
113+
} catch (Exception e) {
114+
System.err.print("Error stopping servlet tester");
115+
e.printStackTrace();
116+
}
117+
}
118+
119+
/**
120+
* Tests that after a redirect the final url in the response
121+
* reflect the redirect
122+
*/
123+
@Test
124+
public void testGetRedirectFinalUrl() {
125+
126+
AsyncHttpClient c = null;
127+
try {
128+
AsyncHttpClientConfig.Builder bc =
129+
new AsyncHttpClientConfig.Builder();
130+
131+
bc.setAllowPoolingConnection(true);
132+
bc.setMaximumConnectionsPerHost(1);
133+
bc.setMaximumConnectionsTotal(1);
134+
bc.setConnectionTimeoutInMs(1000);
135+
bc.setRequestTimeoutInMs(1000);
136+
bc.setFollowRedirects(true);
137+
138+
NettyAsyncHttpProviderConfig config = new NettyAsyncHttpProviderConfig();
139+
if (System.getProperty("blockingio") != null)
140+
config.addProperty(NettyAsyncHttpProviderConfig.USE_BLOCKING_IO, "true");
141+
//config.addProperty(NettyAsyncHttpProviderConfig.REUSE_ADDRESS, "true");
142+
bc.setAsyncHttpClientProviderConfig(config);
143+
c = new AsyncHttpClient(bc.build());
144+
145+
RequestBuilder builder = new RequestBuilder("GET");
146+
builder.setUrl(servletEndpointRedirectUrl);
147+
148+
com.ning.http.client.Request r = builder.build();
149+
150+
try {
151+
ListenableFuture<Response> response = c.executeRequest(r);
152+
Response res = null;
153+
res = response.get();
154+
assertNotNull(res.getResponseBody());
155+
assertEquals(BASE_URL + "/overthere", BASE_URL + "/overthere", res.getUri().toString());
156+
157+
} catch (Exception e) {
158+
System.err.print("============");
159+
e.printStackTrace();
160+
System.err.print("============");
161+
System.err.flush();
162+
fail("Should not get here, The request threw an exception");
163+
}
164+
165+
166+
}
167+
finally {
168+
// can hang here
169+
if (c != null) c.close();
170+
}
171+
172+
173+
}
174+
175+
176+
class MockRedirectHttpServlet extends HttpServlet {
177+
public void service(HttpServletRequest req, HttpServletResponse res)
178+
throws ServletException, IOException {
179+
res.sendRedirect("/overthere");
180+
}
181+
182+
}
183+
184+
class MockFullResponseHttpServlet extends HttpServlet {
185+
186+
private static final String contentType = "text/xml";
187+
private static final String xml = "<?xml version=\"1.0\"?><hello date=\"%s\"></hello>";
188+
189+
public void service(HttpServletRequest req, HttpServletResponse res) throws ServletException, IOException {
190+
String xmlToReturn = String.format(xml, new Object[]{new Date().toString()});
191+
192+
res.setStatus(200, "Complete, XML Being Returned");
193+
res.addHeader("Content-Type", contentType);
194+
res.addHeader("X-Method", req.getMethod());
195+
res.addHeader("MultiValue", "1");
196+
res.addHeader("MultiValue", "2");
197+
res.addHeader("MultiValue", "3");
198+
199+
OutputStream os = res.getOutputStream();
200+
201+
byte[] retVal = xmlToReturn.getBytes();
202+
res.setContentLength(retVal.length);
203+
os.write(retVal);
204+
os.close();
205+
}
206+
}
207+
208+
209+
}

0 commit comments

Comments
 (0)