From 7dd4c9f7b9157535fbdf8d490f56c3b0c62ccd7b Mon Sep 17 00:00:00 2001 From: Steven Soloff Date: Fri, 9 Jun 2017 13:03:01 -0400 Subject: [PATCH 1/2] 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. --- .../src/test/java/org/asynchttpclient/Head302Test.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/client/src/test/java/org/asynchttpclient/Head302Test.java b/client/src/test/java/org/asynchttpclient/Head302Test.java index 0686e10619..ac2b0eab8e 100644 --- a/client/src/test/java/org/asynchttpclient/Head302Test.java +++ b/client/src/test/java/org/asynchttpclient/Head302Test.java @@ -16,6 +16,7 @@ package org.asynchttpclient; import static org.asynchttpclient.Dsl.*; +import static org.testng.Assert.assertEquals; import static org.testng.Assert.fail; import java.io.IOException; @@ -54,6 +55,8 @@ public void handle(String s, org.eclipse.jetty.server.Request r, HttpServletRequ } else { // this handler is to handle HEAD request response.setStatus(HttpServletResponse.SC_FORBIDDEN); } + + r.setHandled(true); } } @@ -64,11 +67,12 @@ public AbstractHandler configureHandler() throws Exception { @Test(groups = "standalone") public void testHEAD302() throws IOException, BrokenBarrierException, InterruptedException, ExecutionException, TimeoutException { - try (AsyncHttpClient client = asyncHttpClient()) { + AsyncHttpClientConfig clientConfig = new DefaultAsyncHttpClientConfig.Builder().setFollowRedirect(true).build(); + try (AsyncHttpClient client = asyncHttpClient(clientConfig)) { final CountDownLatch l = new CountDownLatch(1); Request request = head("http://localhost:" + port1 + "/Test").build(); - client.executeRequest(request, new AsyncCompletionHandlerBase() { + Response response = client.executeRequest(request, new AsyncCompletionHandlerBase() { @Override public Response onCompleted(Response response) throws Exception { l.countDown(); @@ -79,6 +83,8 @@ public Response onCompleted(Response response) throws Exception { if (!l.await(TIMEOUT, TimeUnit.SECONDS)) { fail("Timeout out"); } + + assertEquals(response.getStatusCode(), HttpServletResponse.SC_OK); } } } From 608a4dd5ed5c8bb814b6e09e3131908bc360001b Mon Sep 17 00:00:00 2001 From: Steven Soloff Date: Fri, 9 Jun 2017 13:52:58 -0400 Subject: [PATCH 2/2] Fix broken Head302Test The test now verifies that a HEAD redirected via 302 is switched to GET per [1]. [1] https://github.com/AsyncHttpClient/async-http-client/issues/989 --- .../java/org/asynchttpclient/Head302Test.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/client/src/test/java/org/asynchttpclient/Head302Test.java b/client/src/test/java/org/asynchttpclient/Head302Test.java index ac2b0eab8e..2512b9cb11 100644 --- a/client/src/test/java/org/asynchttpclient/Head302Test.java +++ b/client/src/test/java/org/asynchttpclient/Head302Test.java @@ -17,6 +17,7 @@ import static org.asynchttpclient.Dsl.*; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; import java.io.IOException; @@ -46,13 +47,11 @@ public class Head302Test extends AbstractBasicTest { private static class Head302handler extends AbstractHandler { public void handle(String s, org.eclipse.jetty.server.Request r, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { if ("HEAD".equalsIgnoreCase(request.getMethod())) { - if (request.getPathInfo().endsWith("_moved")) { - response.setStatus(HttpServletResponse.SC_OK); - } else { - response.setStatus(HttpServletResponse.SC_FOUND); // 302 - response.setHeader("Location", request.getPathInfo() + "_moved"); - } - } else { // this handler is to handle HEAD request + response.setStatus(HttpServletResponse.SC_FOUND); // 302 + response.setHeader("Location", request.getPathInfo() + "_moved"); + } else if ("GET".equalsIgnoreCase(request.getMethod())) { + response.setStatus(HttpServletResponse.SC_OK); + } else { response.setStatus(HttpServletResponse.SC_FORBIDDEN); } @@ -80,11 +79,12 @@ public Response onCompleted(Response response) throws Exception { } }).get(3, TimeUnit.SECONDS); - if (!l.await(TIMEOUT, TimeUnit.SECONDS)) { + if (l.await(TIMEOUT, TimeUnit.SECONDS)) { + assertEquals(response.getStatusCode(), HttpServletResponse.SC_OK); + assertTrue(response.getUri().getPath().endsWith("_moved")); + } else { fail("Timeout out"); } - - assertEquals(response.getStatusCode(), HttpServletResponse.SC_OK); } } }