432270 - Slow requests with response content delimited by EOF fail.

Fixed by using a flag to determine the need to close the connection,
and by closing the connection only at exchange termination.

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

@@ -77,9 +77,10 @@ public class HttpChannelOverHTTP extends HttpChannel
     public void exchangeTerminated(Result result)
     {
         super.exchangeTerminated(result);
-        boolean close = result.isFailed();
         HttpFields responseHeaders = result.getResponse().getHeaders();
-        close |= responseHeaders.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString());
+        boolean close = result.isFailed() ||
+                responseHeaders.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString()) ||
+                receiver.isShutdown();
         if (close)
             connection.close();
         else

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

@@ -38,6 +38,7 @@ import org.eclipse.jetty.util.BufferUtil;
 public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.ResponseHandler<ByteBuffer>
 {
     private final HttpParser parser = new HttpParser(this);
+    private boolean shutdown;
 
     public HttpReceiverOverHTTP(HttpChannelOverHTTP channel)
     {
@@ -124,10 +125,23 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
 
     private void shutdown()
     {
-        // Shutting down the parser may invoke messageComplete() or earlyEOF()
+        // Mark this receiver as shutdown, so that we can
+        // close the connection when the exchange terminates.
+        // We cannot close the connection from here because
+        // the request may still be in process.
+        shutdown = true;
+
+        // Shutting down the parser may invoke messageComplete() or earlyEOF().
+        // In case of content delimited by EOF, without a Connection: close
+        // header, the connection will be closed at exchange termination
+        // thanks to the flag we have set above.
         parser.atEOF();
         parser.parseNext(BufferUtil.EMPTY_BUFFER);
-        getHttpConnection().close(new EOFException());
+    }
+
+    protected boolean isShutdown()
+    {
+        return shutdown;
     }
 
     @Override
@@ -230,8 +244,10 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
 
     private void failAndClose(Throwable failure)
     {
-        if (responseFailure(failure))
-            getHttpChannel().getHttpConnection().close();
+        // Close the connection anyway, even if responseFailure() returns false.
+        // This may happen for idle closes (there is no exchange to fail).
+        responseFailure(failure);
+        getHttpConnection().close(failure);
     }
 
     @Override

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

@@ -59,6 +59,7 @@ import org.eclipse.jetty.client.http.HttpConnectionOverHTTP;
 import org.eclipse.jetty.client.http.HttpDestinationOverHTTP;
 import org.eclipse.jetty.client.util.BufferingResponseListener;
 import org.eclipse.jetty.client.util.BytesContentProvider;
+import org.eclipse.jetty.client.util.DeferredContentProvider;
 import org.eclipse.jetty.client.util.FutureResponseListener;
 import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpHeader;
@@ -72,6 +73,7 @@ import org.eclipse.jetty.util.FuturePromise;
 import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.junit.Assert;
+import org.junit.Assume;
 import org.junit.Rule;
 import org.junit.Test;
 
@@ -1197,4 +1199,43 @@ public class HttpClientTest extends AbstractHttpClientServerTest
 
         Assert.assertTrue(completeLatch.await(5, TimeUnit.SECONDS));
     }
+
+    @Test
+    public void testContentDelimitedByEOFWithSlowRequest() throws Exception
+    {
+        // This test is crafted in a way that the response completes before the request is fully written.
+        // With SSL, the response coming down will close the SSLEngine so it would not be possible to
+        // write the last chunk of the request content, and the request will be failed, failing also the
+        // test, which is not what we want.
+        // This is a limit of Java's SSL implementation that does not allow half closes.
+        Assume.assumeTrue(sslContextFactory == null);
+
+        final byte[] data = new byte[8 * 1024];
+        new Random().nextBytes(data);
+        start(new AbstractHandler()
+        {
+            @Override
+            public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
+            {
+                baseRequest.setHandled(true);
+                response.setHeader("Connection", "close");
+                response.getOutputStream().write(data);
+            }
+        });
+
+        DeferredContentProvider content = new DeferredContentProvider(ByteBuffer.wrap(new byte[]{0}));
+        Request request = client.newRequest("localhost", connector.getLocalPort())
+                .scheme(scheme)
+                .content(content);
+        FutureResponseListener listener = new FutureResponseListener(request);
+        request.send(listener);
+        // Wait some time to simulate a slow request.
+        Thread.sleep(1000);
+        content.close();
+
+        ContentResponse response = listener.get(5, TimeUnit.SECONDS);
+
+        Assert.assertEquals(200, response.getStatus());
+        Assert.assertArrayEquals(data, response.getContent());
+    }
 }