From 5f8513e67ec4dd28fdbd80a4e1b61194473b3421 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 8 Apr 2014 17:36:23 +0200 Subject: [PATCH] 432270 - Slow requests with response content delimited by EOF fail. Better handling of the idle case, and closing the connection only if the response can be failed. --- .../client/http/HttpReceiverOverHTTP.java | 12 ++++--- .../eclipse/jetty/client/HttpClientTest.java | 32 +++++++++++++++++-- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java index 505d264747f..1b688e16e50 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java @@ -213,7 +213,11 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res @Override public void earlyEOF() { - failAndClose(new EOFException()); + HttpExchange exchange = getHttpExchange(); + if (exchange == null) + getHttpConnection().close(); + else + failAndClose(new EOFException()); } @Override @@ -244,10 +248,8 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res private void failAndClose(Throwable failure) { - // 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); + if (responseFailure(failure)) + getHttpConnection().close(failure); } @Override diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java index c6789e20cdb..bedd3482776 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java @@ -1201,7 +1201,30 @@ public class HttpClientTest extends AbstractHttpClientServerTest } @Test - public void testContentDelimitedByEOFWithSlowRequest() throws Exception + public void testSmallContentDelimitedByEOFWithSlowRequestHTTP10() throws Exception + { + testContentDelimitedByEOFWithSlowRequest(HttpVersion.HTTP_1_0, 1024); + } + + @Test + public void testBigContentDelimitedByEOFWithSlowRequestHTTP10() throws Exception + { + testContentDelimitedByEOFWithSlowRequest(HttpVersion.HTTP_1_0, 128 * 1024); + } + + @Test + public void testSmallContentDelimitedByEOFWithSlowRequestHTTP11() throws Exception + { + testContentDelimitedByEOFWithSlowRequest(HttpVersion.HTTP_1_1, 1024); + } + + @Test + public void testBigContentDelimitedByEOFWithSlowRequestHTTP11() throws Exception + { + testContentDelimitedByEOFWithSlowRequest(HttpVersion.HTTP_1_1, 128 * 1024); + } + + private void testContentDelimitedByEOFWithSlowRequest(final HttpVersion version, int length) 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 @@ -1210,7 +1233,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest // 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]; + final byte[] data = new byte[length]; new Random().nextBytes(data); start(new AbstractHandler() { @@ -1218,7 +1241,9 @@ public class HttpClientTest extends AbstractHttpClientServerTest 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"); + // Send Connection: close to avoid that the server chunks the content with HTTP 1.1. + if (version.compareTo(HttpVersion.HTTP_1_0) > 0) + response.setHeader("Connection", "close"); response.getOutputStream().write(data); } }); @@ -1226,6 +1251,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest DeferredContentProvider content = new DeferredContentProvider(ByteBuffer.wrap(new byte[]{0})); Request request = client.newRequest("localhost", connector.getLocalPort()) .scheme(scheme) + .version(version) .content(content); FutureResponseListener listener = new FutureResponseListener(request); request.send(listener);