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

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

@@ -123,10 +123,16 @@ public abstract class HttpConnection implements IConnection
 
     protected void normalizeRequest(Request request)
     {
-        final HttpVersion version = request.getVersion();
-        final HttpFields headers = request.getHeaders();
-        final ContentProvider content = request.getContent();
-        final ProxyConfiguration.Proxy proxy = destination.getProxy();
+        boolean normalized = ((HttpRequest)request).normalized();
+        if (LOG.isDebugEnabled())
+            LOG.debug("Normalizing {} {}", !normalized, request);
+        if (normalized)
+            return;
+
+        HttpVersion version = request.getVersion();
+        HttpFields headers = request.getHeaders();
+        ContentProvider content = request.getContent();
+        ProxyConfiguration.Proxy proxy = destination.getProxy();
 
         // Make sure the path is there
         String path = request.getPath();

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

@@ -311,10 +311,10 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
         }
     }
 
-    public boolean process(final Connection connection)
+    public boolean process(Connection connection)
     {
         HttpClient client = getHttpClient();
-        final HttpExchange exchange = getHttpExchanges().poll();
+        HttpExchange exchange = getHttpExchanges().poll();
         if (LOG.isDebugEnabled())
             LOG.debug("Processing exchange {} on {} of {}", exchange, connection, this);
         if (exchange == null)
@@ -331,7 +331,7 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
         }
         else
         {
-            final Request request = exchange.getRequest();
+            Request request = exchange.getRequest();
             Throwable cause = request.getAbortCause();
             if (cause != null)
             {
@@ -347,21 +347,30 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
             }
             else
             {
-                SendFailure result = ((IConnection)connection).send(exchange);
+                SendFailure result = send((IConnection)connection, exchange);
                 if (result != null)
                 {
                     if (LOG.isDebugEnabled())
                         LOG.debug("Send failed {} for {}", result, exchange);
                     if (result.retry)
+                    {
+                        // Resend this exchange, likely on another connection,
+                        // and return false to avoid to re-enter this method.
                         send(exchange);
-                    else
-                        request.abort(result.failure);
+                        return false;
+                    }
+                    request.abort(result.failure);
                 }
             }
             return getHttpExchanges().peek() != null;
         }
     }
 
+    protected SendFailure send(IConnection connection, HttpExchange exchange)
+    {
+        return connection.send(exchange);
+    }
+
     @Override
     public void newConnection(Promise<Connection> promise)
     {

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

@@ -90,6 +90,7 @@ public class HttpRequest implements Request
     private Supplier<HttpFields> trailers;
     private String upgradeProtocol;
     private Object tag;
+    private boolean normalized;
 
     protected HttpRequest(HttpClient client, HttpConversation conversation, URI uri)
     {
@@ -822,6 +823,23 @@ public class HttpRequest implements Request
         return aborted.get();
     }
 
+    /**
+     * <p>Marks this request as <em>normalized</em>.</p>
+     * <p>A request is normalized by setting things that applications give
+     * for granted such as defaulting the method to {@code GET}, adding the
+     * {@code Host} header, adding the cookies, adding {@code Authorization}
+     * headers, etc.</p>
+     *
+     * @return whether this request was already normalized
+     * @see HttpConnection#normalizeRequest(Request)
+     */
+    boolean normalized()
+    {
+        boolean result = normalized;
+        normalized = true;
+        return result;
+    }
+
     private String buildQuery()
     {
         StringBuilder result = new StringBuilder();

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

@@ -32,6 +32,10 @@ public class SendFailure
     @Override
     public String toString()
     {
-        return String.format("%s[failure=%s,retry=%b]", super.toString(), failure, retry);
+        return String.format("%s@%x[failure=%s,retry=%b]",
+            getClass().getSimpleName(),
+            hashCode(),
+            failure,
+            retry);
     }
 }

jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java

@@ -146,12 +146,17 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements IConne
     public boolean onIdleExpired()
     {
         long idleTimeout = getEndPoint().getIdleTimeout();
-        boolean close = delegate.onIdleTimeout(idleTimeout);
+        boolean close = onIdleTimeout(idleTimeout);
         if (close)
             close(new TimeoutException("Idle timeout " + idleTimeout + " ms"));
         return false;
     }
 
+    protected boolean onIdleTimeout(long idleTimeout)
+    {
+        return delegate.onIdleTimeout(idleTimeout);
+    }
+
     @Override
     public void onFillable()
     {

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

@@ -0,0 +1,171 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.client;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.jetty.client.api.Connection;
+import org.eclipse.jetty.client.api.ContentResponse;
+import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP;
+import org.eclipse.jetty.client.http.HttpConnectionOverHTTP;
+import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.io.EndPoint;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.util.Promise;
+import org.eclipse.jetty.util.thread.QueuedThreadPool;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class HttpClientIdleTimeoutTest
+{
+    private Server server;
+    private ServerConnector connector;
+    private HttpClient client;
+
+    private void start(Handler handler) throws Exception
+    {
+        QueuedThreadPool serverThreads = new QueuedThreadPool();
+        serverThreads.setName("server");
+        server = new Server(serverThreads);
+        connector = new ServerConnector(server, 1, 1);
+        server.addConnector(connector);
+        server.setHandler(handler);
+        server.start();
+    }
+
+    @AfterEach
+    public void dispose() throws Exception
+    {
+        if (server != null)
+            server.stop();
+        if (client != null)
+            client.stop();
+    }
+
+    @Test
+    public void testRequestIsRetriedWhenSentDuringIdleTimeout() throws Exception
+    {
+        start(new EmptyServerHandler()
+        {
+            @Override
+            protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
+            {
+                Cookie[] cookies = request.getCookies();
+                if (cookies == null || cookies.length == 0)
+                {
+                    // Send a cookie in the first response.
+                    response.addCookie(new Cookie("name", "value"));
+                }
+                else
+                {
+                    // Verify that there is only one cookie, i.e.
+                    // that the request has not been normalized twice.
+                    assertEquals(1, cookies.length);
+                }
+            }
+        });
+
+        CountDownLatch idleTimeoutLatch = new CountDownLatch(1);
+        CountDownLatch requestLatch = new CountDownLatch(1);
+        CountDownLatch retryLatch = new CountDownLatch(1);
+        QueuedThreadPool clientThreads = new QueuedThreadPool();
+        clientThreads.setName("client");
+        client = new HttpClient(new HttpClientTransportOverHTTP(1)
+        {
+            @Override
+            public HttpDestination newHttpDestination(Origin origin)
+            {
+                return new DuplexHttpDestination(getHttpClient(), origin)
+                {
+                    @Override
+                    protected SendFailure send(IConnection connection, HttpExchange exchange)
+                    {
+                        SendFailure result = super.send(connection, exchange);
+                        if (result != null && result.retry)
+                            retryLatch.countDown();
+                        return result;
+                    }
+                };
+            }
+
+            @Override
+            protected HttpConnectionOverHTTP newHttpConnection(EndPoint endPoint, HttpDestination destination, Promise<Connection> promise)
+            {
+                return new HttpConnectionOverHTTP(endPoint, destination, promise)
+                {
+                    @Override
+                    protected boolean onIdleTimeout(long idleTimeout)
+                    {
+                        boolean result = super.onIdleTimeout(idleTimeout);
+                        if (result)
+                            idleTimeoutLatch.countDown();
+                        assertTrue(await(requestLatch));
+                        return result;
+                    }
+                };
+            }
+        });
+        client.setExecutor(clientThreads);
+        client.start();
+
+        long idleTimeout = 1000;
+        client.setIdleTimeout(idleTimeout);
+
+        // Create one connection.
+        ContentResponse response = client.newRequest("localhost", connector.getLocalPort()).send();
+        assertEquals(response.getStatus(), HttpStatus.OK_200);
+
+        assertTrue(idleTimeoutLatch.await(2 * idleTimeout, TimeUnit.MILLISECONDS));
+
+        // Send a request exactly while the connection is idle timing out.
+        CountDownLatch responseLatch = new CountDownLatch(1);
+        client.newRequest("localhost", connector.getLocalPort()).send(result ->
+        {
+            assertTrue(result.isSucceeded());
+            assertEquals(HttpStatus.OK_200, result.getResponse().getStatus());
+            responseLatch.countDown();
+        });
+        assertTrue(retryLatch.await(5, TimeUnit.SECONDS));
+        requestLatch.countDown();
+
+        assertTrue(responseLatch.await(5, TimeUnit.SECONDS));
+    }
+
+    private boolean await(CountDownLatch latch)
+    {
+        try
+        {
+            return latch.await(15, TimeUnit.SECONDS);
+        }
+        catch (InterruptedException x)
+        {
+            throw new RuntimeException(x);
+        }
+    }
+}