Merged branch 'jetty-10.0.x' into 'jetty-11.0.x'.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

jetty-client/src/main/java/org/eclipse/jetty/client/HttpRedirector.java

@@ -28,7 +28,9 @@ import org.eclipse.jetty.client.api.Request;
 import org.eclipse.jetty.client.api.Response;
 import org.eclipse.jetty.client.api.Result;
 import org.eclipse.jetty.client.util.BufferingResponseListener;
+import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpMethod;
+import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.util.NanoTime;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -38,7 +40,7 @@ import org.slf4j.LoggerFactory;
  * <p>
  * Applications can disable redirection via {@link Request#followRedirects(boolean)}
  * and then rely on this class to perform the redirect in a simpler way, for example:
- * <pre>
+ * <pre>{@code
  * HttpRedirector redirector = new HttpRedirector(httpClient);
  *
  * Request request = httpClient.newRequest("http://host/path").followRedirects(false);
@@ -53,7 +55,7 @@ import org.slf4j.LoggerFactory;
  *     request = result.getRequest();
  *     response = result.getResponse();
  * }
- * </pre>
+ * }</pre>
  */
 public class HttpRedirector
 {
@@ -85,11 +87,11 @@ public class HttpRedirector
     {
         switch (response.getStatus())
         {
-            case 301:
+            case HttpStatus.MOVED_PERMANENTLY_301:
-            case 302:
+            case HttpStatus.MOVED_TEMPORARILY_302:
-            case 303:
+            case HttpStatus.SEE_OTHER_303:
-            case 307:
+            case HttpStatus.TEMPORARY_REDIRECT_307:
-            case 308:
+            case HttpStatus.PERMANENT_REDIRECT_308:
                 return true;
             default:
                 return false;
@@ -191,7 +193,7 @@ public class HttpRedirector
         int status = response.getStatus();
         switch (status)
         {
-            case 301:
+            case HttpStatus.MOVED_PERMANENTLY_301:
             {
                 String method = request.getMethod();
                 if (HttpMethod.GET.is(method) || HttpMethod.HEAD.is(method) || HttpMethod.PUT.is(method))
@@ -201,7 +203,7 @@ public class HttpRedirector
                 fail(request, response, new HttpResponseException("HTTP protocol violation: received 301 for non GET/HEAD/POST/PUT request", response));
                 return null;
             }
-            case 302:
+            case HttpStatus.MOVED_TEMPORARILY_302:
             {
                 String method = request.getMethod();
                 if (HttpMethod.HEAD.is(method) || HttpMethod.PUT.is(method))
@@ -209,7 +211,7 @@ public class HttpRedirector
                 else
                     return redirect(request, response, listener, newURI, HttpMethod.GET.asString());
             }
-            case 303:
+            case HttpStatus.SEE_OTHER_303:
             {
                 String method = request.getMethod();
                 if (HttpMethod.HEAD.is(method))
@@ -217,8 +219,8 @@ public class HttpRedirector
                 else
                     return redirect(request, response, listener, newURI, HttpMethod.GET.asString());
             }
-            case 307:
+            case HttpStatus.TEMPORARY_REDIRECT_307:
-            case 308:
+            case HttpStatus.PERMANENT_REDIRECT_308:
             {
                 // Keep same method
                 return redirect(request, response, listener, newURI, request.getMethod());
@@ -310,6 +312,30 @@ public class HttpRedirector
         {
             Request redirect = client.copyRequest(httpRequest, location);
 
+            // Use the given method.
+            redirect.method(method);
+
+            if (HttpMethod.GET.is(method))
+            {
+                redirect.body(null);
+                redirect.headers(headers ->
+                {
+                    headers.remove(HttpHeader.CONTENT_LENGTH);
+                    headers.remove(HttpHeader.CONTENT_TYPE);
+                });
+            }
+
+            Request.Content body = redirect.getBody();
+            if (body != null && !body.isReproducible())
+            {
+                if (LOG.isDebugEnabled())
+                    LOG.debug("Could not redirect to {}, request body is not reproducible", location);
+                HttpConversation conversation = httpRequest.getConversation();
+                conversation.updateResponseListeners(null);
+                notifier.forwardSuccessComplete(conversation.getResponseListeners(), httpRequest, response);
+                return null;
+            }
+
             // Adjust the timeout of the new request, taking into account the
             // timeout of the previous request and the time already elapsed.
             long timeoutNanoTime = httpRequest.getTimeoutNanoTime();
@@ -328,9 +354,6 @@ public class HttpRedirector
                 }
             }
 
-            // Use given method
-            redirect.method(method);
-
             redirect.onRequestBegin(request ->
             {
                 Throwable cause = httpRequest.getAbortCause();

jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientRedirectTest.java

@@ -33,6 +33,7 @@ import jakarta.servlet.http.HttpServletResponse;
 import org.eclipse.jetty.client.api.ContentResponse;
 import org.eclipse.jetty.client.api.Response;
 import org.eclipse.jetty.client.api.Result;
+import org.eclipse.jetty.client.util.AsyncRequestContent;
 import org.eclipse.jetty.client.util.ByteBufferRequestContent;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpMethod;
@@ -163,6 +164,49 @@ public class HttpClientRedirectTest extends AbstractHttpClientServerTest
         assertArrayEquals(data, response.getContent());
     }
 
+    @ParameterizedTest
+    @ArgumentsSource(ScenarioProvider.class)
+    public void test303WithRequestContentNotReproducible(Scenario scenario) throws Exception
+    {
+        start(scenario, new RedirectHandler());
+
+        byte[] data = new byte[]{0, 1, 2, 3, 4, 5, 6, 7};
+        AsyncRequestContent body = new AsyncRequestContent(ByteBuffer.wrap(data));
+        body.close();
+        ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
+            .scheme(scenario.getScheme())
+            .method(HttpMethod.POST)
+            .path("/303/localhost/done")
+            .body(body)
+            .timeout(5, TimeUnit.SECONDS)
+            .send();
+        assertNotNull(response);
+        assertEquals(200, response.getStatus());
+        assertFalse(response.getHeaders().contains(HttpHeader.LOCATION));
+        assertEquals(0, response.getContent().length);
+    }
+
+    @ParameterizedTest
+    @ArgumentsSource(ScenarioProvider.class)
+    public void test307WithRequestContentNotReproducible(Scenario scenario) throws Exception
+    {
+        start(scenario, new RedirectHandler());
+
+        byte[] data = new byte[]{0, 1, 2, 3, 4, 5, 6, 7};
+        AsyncRequestContent body = new AsyncRequestContent(ByteBuffer.wrap(data));
+        body.close();
+        ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
+            .scheme(scenario.getScheme())
+            .method(HttpMethod.POST)
+            .path("/307/localhost/done")
+            .body(body)
+            .timeout(5, TimeUnit.SECONDS)
+            .send();
+        assertNotNull(response);
+        assertEquals(307, response.getStatus());
+        assertTrue(response.getHeaders().contains(HttpHeader.LOCATION));
+    }
+
     @ParameterizedTest
     @ArgumentsSource(ScenarioProvider.class)
     public void testMaxRedirections(Scenario scenario) throws Exception
@@ -704,7 +748,7 @@ public class HttpClientRedirectTest extends AbstractHttpClientServerTest
                 location += "/" + path;
 
                 if (Boolean.parseBoolean(request.getParameter("decode")))
-                    location = URLDecoder.decode(location, "UTF-8");
+                    location = URLDecoder.decode(location, StandardCharsets.UTF_8);
 
                 response.setHeader("Location", location);
 

jetty-deploy/src/test/java/org/eclipse/jetty/deploy/bindings/GlobalWebappConfigBindingTest.java

@@ -23,6 +23,7 @@ import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.eclipse.jetty.toolchain.test.PathAssert;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
+import org.eclipse.jetty.util.component.LifeCycle;
 import org.eclipse.jetty.webapp.WebAppContext;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -61,7 +62,7 @@ public class GlobalWebappConfigBindingTest
     public void teardownEnvironment() throws Exception
     {
         // Stop jetty.
-        jetty.stop();
+        LifeCycle.stop(jetty);
     }
 
     @Test

jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/ConnectTimeoutTest.java

@@ -24,6 +24,7 @@ import org.eclipse.jetty.http2.api.Session;
 import org.eclipse.jetty.http2.api.server.ServerSessionListener;
 import org.eclipse.jetty.util.Promise;
 import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -33,10 +34,12 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
 public class ConnectTimeoutTest extends AbstractTest
 {
     @Test
+    @Tag("external")
     public void testConnectTimeout() throws Exception
     {
-        final String host = "10.255.255.1";
+        // Using IANA hosted example.com:81 to reliably produce a Connect Timeout.
-        final int port = 80;
+        final String host = "example.com";
+        final int port = 81;
         int connectTimeout = 1000;
         assumeConnectTimeout(host, port, connectTimeout);
 

tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientConnectTimeoutTest.java

@@ -47,11 +47,13 @@ public class HttpClientConnectTimeoutTest extends AbstractTest<TransportScenario
 
     @ParameterizedTest
     @ArgumentsSource(TransportProvider.class)
+    @Tag("external")
     public void testConnectTimeout(Transport transport) throws Exception
     {
         init(transport);
-        final String host = "10.255.255.1";
+        // Using IANA hosted example.com:81 to reliably produce a Connect Timeout.
-        final int port = 80;
+        final String host = "example.com";
+        final int port = 81;
         int connectTimeout = 1000;
         assumeConnectTimeout(host, port, connectTimeout);
 
@@ -74,11 +76,13 @@ public class HttpClientConnectTimeoutTest extends AbstractTest<TransportScenario
 
     @ParameterizedTest
     @ArgumentsSource(TransportProvider.class)
+    @Tag("external")
     public void testConnectTimeoutIsCancelledByShorterRequestTimeout(Transport transport) throws Exception
     {
         init(transport);
-        String host = "10.255.255.1";
+        // Using IANA hosted example.com:81 to reliably produce a Connect Timeout.
-        int port = 80;
+        final String host = "example.com";
+        final int port = 81;
         int connectTimeout = 2000;
         assumeConnectTimeout(host, port, connectTimeout);
 
@@ -104,11 +108,13 @@ public class HttpClientConnectTimeoutTest extends AbstractTest<TransportScenario
 
     @ParameterizedTest
     @ArgumentsSource(TransportProvider.class)
+    @Tag("external")
     public void retryAfterConnectTimeout(Transport transport) throws Exception
     {
         init(transport);
-        final String host = "10.255.255.1";
+        // Using IANA hosted example.com:81 to reliably produce a Connect Timeout.
-        final int port = 80;
+        final String host = "example.com";
+        final int port = 81;
         int connectTimeout = 1000;
         assumeConnectTimeout(host, port, connectTimeout);
 

tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java

@@ -238,9 +238,9 @@ public class HttpClientContinueTest extends AbstractTest<TransportScenario>
                 }
                 else
                 {
-                    // Send 100-Continue and consume the content
+                    // Send 100-Continue and consume the content.
-                    IO.copy(request.getInputStream(), new ByteArrayOutputStream());
+                    IO.copy(request.getInputStream(), OutputStream.nullOutputStream());
-                    // Send a redirect
+                    // Send a redirect.
                     response.sendRedirect("/done");
                 }
             }

tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTimeoutTest.java

@@ -51,6 +51,7 @@ import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.hamcrest.Matchers;
 import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ArgumentsSource;
 import org.opentest4j.TestAbortedException;
@@ -282,6 +283,7 @@ public class HttpClientTimeoutTest extends AbstractTest<TransportScenario>
 
     @ParameterizedTest
     @ArgumentsSource(TransportProvider.class)
+    @Tag("external")
     public void testBlockingConnectTimeoutFailsRequest(Transport transport) throws Exception
     {
         // Failure to connect is based on InetSocket address failure, which Unix-Domain does not use.
@@ -292,6 +294,7 @@ public class HttpClientTimeoutTest extends AbstractTest<TransportScenario>
 
     @ParameterizedTest
     @ArgumentsSource(TransportProvider.class)
+    @Tag("external")
     public void testNonBlockingConnectTimeoutFailsRequest(Transport transport) throws Exception
     {
         // Failure to connect is based on InetSocket address failure, which Unix-Domain does not use.
@@ -302,8 +305,9 @@ public class HttpClientTimeoutTest extends AbstractTest<TransportScenario>
 
     private void testConnectTimeoutFailsRequest(boolean blocking) throws Exception
     {
-        String host = "10.255.255.1";
+        // Using IANA hosted example.com:81 to reliably produce a Connect Timeout.
-        int port = 80;
+        final String host = "example.com";
+        final int port = 81;
         int connectTimeout = 1000;
         assumeConnectTimeout(host, port, connectTimeout);
 
@@ -329,6 +333,7 @@ public class HttpClientTimeoutTest extends AbstractTest<TransportScenario>
 
     @ParameterizedTest
     @ArgumentsSource(TransportProvider.class)
+    @Tag("external")
     public void testConnectTimeoutIsCancelledByShorterRequestTimeout(Transport transport) throws Exception
     {
         // Failure to connect is based on InetSocket address failure, which Unix-Domain does not use.
@@ -336,8 +341,9 @@ public class HttpClientTimeoutTest extends AbstractTest<TransportScenario>
 
         init(transport);
 
-        String host = "10.255.255.1";
+        // Using IANA hosted example.com:81 to reliably produce a Connect Timeout.
-        int port = 80;
+        final String host = "example.com";
+        final int port = 81;
         int connectTimeout = 2000;
         assumeConnectTimeout(host, port, connectTimeout);
 
@@ -365,6 +371,7 @@ public class HttpClientTimeoutTest extends AbstractTest<TransportScenario>
 
     @ParameterizedTest
     @ArgumentsSource(TransportProvider.class)
+    @Tag("external")
     public void testRetryAfterConnectTimeout(Transport transport) throws Exception
     {
         // Failure to connect is based on InetSocket address failure, which Unix-Domain does not use.
@@ -372,8 +379,9 @@ public class HttpClientTimeoutTest extends AbstractTest<TransportScenario>
 
         init(transport);
 
-        final String host = "10.255.255.1";
+        // Using IANA hosted example.com:81 to reliably produce a Connect Timeout.
-        final int port = 80;
+        final String host = "example.com";
+        final int port = 81;
         int connectTimeout = 1000;
         assumeConnectTimeout(host, port, connectTimeout);