Skip to content

Commit 3c25a42

Browse files
ssoloffslandelle
authored andcommitted
Fix broken Head302Test (#1421)
* Update Head302Test to show it is actually failing This test had the following defects which caused it to falsely pass: * Jetty requires a handler to explicitly indicate it has handled the request by calling Request.setHandled(true). Otherwise the server will return 404. * The test did not configure the client to follow redirects. * The test itself wasn't asserting anything useful beyond that the request did not time out. To ensure the redirect was followed, it needs to assert the expected response status code from the redirected location is received. * Fix broken Head302Test The test now verifies that a HEAD redirected via 302 is switched to GET per [1]. [1] #989
1 parent af2e5af commit 3c25a42

File tree

1 file changed

+16
-10
lines changed

1 file changed

+16
-10
lines changed

Diff for: client/src/test/java/org/asynchttpclient/Head302Test.java

+16-10
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
package org.asynchttpclient;
1717

1818
import static org.asynchttpclient.Dsl.*;
19+
import static org.testng.Assert.assertEquals;
20+
import static org.testng.Assert.assertTrue;
1921
import static org.testng.Assert.fail;
2022

2123
import java.io.IOException;
@@ -45,15 +47,15 @@ public class Head302Test extends AbstractBasicTest {
4547
private static class Head302handler extends AbstractHandler {
4648
public void handle(String s, org.eclipse.jetty.server.Request r, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
4749
if ("HEAD".equalsIgnoreCase(request.getMethod())) {
48-
if (request.getPathInfo().endsWith("_moved")) {
49-
response.setStatus(HttpServletResponse.SC_OK);
50-
} else {
51-
response.setStatus(HttpServletResponse.SC_FOUND); // 302
52-
response.setHeader("Location", request.getPathInfo() + "_moved");
53-
}
54-
} else { // this handler is to handle HEAD request
50+
response.setStatus(HttpServletResponse.SC_FOUND); // 302
51+
response.setHeader("Location", request.getPathInfo() + "_moved");
52+
} else if ("GET".equalsIgnoreCase(request.getMethod())) {
53+
response.setStatus(HttpServletResponse.SC_OK);
54+
} else {
5555
response.setStatus(HttpServletResponse.SC_FORBIDDEN);
5656
}
57+
58+
r.setHandled(true);
5759
}
5860
}
5961

@@ -64,19 +66,23 @@ public AbstractHandler configureHandler() throws Exception {
6466

6567
@Test(groups = "standalone")
6668
public void testHEAD302() throws IOException, BrokenBarrierException, InterruptedException, ExecutionException, TimeoutException {
67-
try (AsyncHttpClient client = asyncHttpClient()) {
69+
AsyncHttpClientConfig clientConfig = new DefaultAsyncHttpClientConfig.Builder().setFollowRedirect(true).build();
70+
try (AsyncHttpClient client = asyncHttpClient(clientConfig)) {
6871
final CountDownLatch l = new CountDownLatch(1);
6972
Request request = head("http://localhost:" + port1 + "/Test").build();
7073

71-
client.executeRequest(request, new AsyncCompletionHandlerBase() {
74+
Response response = client.executeRequest(request, new AsyncCompletionHandlerBase() {
7275
@Override
7376
public Response onCompleted(Response response) throws Exception {
7477
l.countDown();
7578
return super.onCompleted(response);
7679
}
7780
}).get(3, TimeUnit.SECONDS);
7881

79-
if (!l.await(TIMEOUT, TimeUnit.SECONDS)) {
82+
if (l.await(TIMEOUT, TimeUnit.SECONDS)) {
83+
assertEquals(response.getStatusCode(), HttpServletResponse.SC_OK);
84+
assertTrue(response.getUri().getPath().endsWith("_moved"));
85+
} else {
8086
fail("Timeout out");
8187
}
8288
}

0 commit comments

Comments
 (0)