Use original method in redirects for HEAD / OPTIONS requests (#1736)

Originally, when setFollowRedirect is set to true (like in Head302Test), the expected flow is HEAD --> GET (based on the behavior of Redirect30xInterceptor - see 3c25a42 for the reasoning).

Following a change to the interceptor (to fix #1728), Head302Test started going on an infinite loop caused by the way the handler in the tent was set up (to account for the original HEAD --> GET flow and not the new HEAD --> HEAD --> HEAD.... flow).

This commit does 3 things:

* Changes Redirect30xInterceptor to set all redirects of a HEAD request to be HEADs as well
* Change Head302Test to account for the new flow
* Notes a flaky test in AsyncStreamHandlerTest that was found during work on the PR

Author: TomGranot

client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java

@@ -38,6 +38,8 @@
 
 import static io.netty.handler.codec.http.HttpHeaderNames.*;
 import static org.asynchttpclient.util.HttpConstants.Methods.GET;
+import static org.asynchttpclient.util.HttpConstants.Methods.HEAD;
+import static org.asynchttpclient.util.HttpConstants.Methods.OPTIONS;
 import static org.asynchttpclient.util.HttpConstants.ResponseStatusCodes.*;
 import static org.asynchttpclient.util.HttpUtils.followRedirect;
 import static org.asynchttpclient.util.MiscUtils.isNonEmpty;
@@ -87,7 +89,7 @@ public boolean exitAfterHandlingRedirect(Channel channel,
 
         String originalMethod = request.getMethod();
         boolean switchToGet = !originalMethod.equals(GET)
-                && (statusCode == MOVED_PERMANENTLY_301 || statusCode == SEE_OTHER_303 || (statusCode == FOUND_302 && !config.isStrict302Handling()));
+                && !originalMethod.equals(OPTIONS) && !originalMethod.equals(HEAD) && (statusCode == MOVED_PERMANENTLY_301 || statusCode == SEE_OTHER_303 || (statusCode == FOUND_302 && !config.isStrict302Handling()));
         boolean keepBody = statusCode == TEMPORARY_REDIRECT_307 || statusCode == PERMANENT_REDIRECT_308 || (statusCode == FOUND_302 && config.isStrict302Handling());
 
         final RequestBuilder requestBuilder = new RequestBuilder(switchToGet ? GET : originalMethod)
@@ -126,7 +128,6 @@ else if (isNonEmpty(request.getBodyParts())) {
         HttpHeaders responseHeaders = response.headers();
         String location = responseHeaders.get(LOCATION);
         Uri newUri = Uri.create(future.getUri(), location);
-
         LOGGER.debug("Redirecting to {}", newUri);
 
         CookieStore cookieStore = config.getCookieStore();

client/src/test/java/org/asynchttpclient/AsyncStreamHandlerTest.java

@@ -428,6 +428,8 @@ public Integer onCompleted() {
       }));
   }
 
+  // This test is flaky - see https://github.com/AsyncHttpClient/async-http-client/issues/1728#issuecomment-699962325
+  // For now, just run again if fails
   @Test(groups = "online")
   public void asyncOptionsTest() throws Throwable {
 

client/src/test/java/org/asynchttpclient/Head302Test.java

@@ -70,9 +70,17 @@ public Response onCompleted(Response response) throws Exception {
   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())) {
-        response.setStatus(HttpServletResponse.SC_FOUND); // 302
-        response.setHeader("Location", request.getPathInfo() + "_moved");
-      } else if ("GET".equalsIgnoreCase(request.getMethod())) {
+        // See https://github.com/AsyncHttpClient/async-http-client/issues/1728#issuecomment-700007980
+        // When setFollowRedirect == TRUE, a follow-up request to a HEAD request will also be a HEAD.
+        // This will cause an infinite loop, which will error out once the maximum amount of redirects is hit (default 5).
+        // Instead, we (arbitrarily) choose to allow for 3 redirects and then return a 200.
+        if(request.getRequestURI().endsWith("_moved_moved_moved")) {
+          response.setStatus(HttpServletResponse.SC_OK);
+        } else {
+          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);