426870 - HTTP 1.0 Request with Connection: keep-alive and response

content hangs.

Fixed HttpGenerator to stay in the EOF_CONTENT mode if such case is
detected (while before it was moving to NO_CONTENT mode).
By staying in EOF_CONTENT mode the generator is made non-persistent
and eventually the connection is closed, signaling the end-of-content
to the client.
This commit is contained in:
Simone Bordet 2014-01-29 11:19:45 +01:00
parent 5ed1a9dfb4
commit cbdfd87d78
4 changed files with 93 additions and 10 deletions

View File

@ -76,7 +76,7 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements Connec
fillInterested();
}
protected boolean isClosed()
public boolean isClosed()
{
return closed.get();
}

View File

@ -48,6 +48,7 @@ import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.client.api.Connection;
import org.eclipse.jetty.client.api.ContentProvider;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Destination;
@ -58,12 +59,16 @@ 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.FutureResponseListener;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpHeaderValue;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.toolchain.test.TestingDir;
import org.eclipse.jetty.toolchain.test.annotation.Slow;
import org.eclipse.jetty.util.FuturePromise;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.Assert;
@ -1073,4 +1078,82 @@ public class HttpClientTest extends AbstractHttpClientServerTest
Assert.assertEquals(200, response.getStatus());
}
@Test
public void testHTTP10WithKeepAliveAndContentLength() throws Exception
{
start(new AbstractHandler()
{
@Override
public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
// Send the headers at this point, then write the content
byte[] content = "TEST".getBytes("UTF-8");
response.setContentLength(content.length);
response.flushBuffer();
response.getOutputStream().write(content);
}
});
ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
.scheme(scheme)
.version(HttpVersion.HTTP_1_0)
.header(HttpHeader.CONNECTION, HttpHeaderValue.KEEP_ALIVE.asString())
.timeout(5, TimeUnit.SECONDS)
.send();
Assert.assertEquals(200, response.getStatus());
Assert.assertTrue(response.getHeaders().contains(HttpHeader.CONNECTION, HttpHeaderValue.KEEP_ALIVE.asString()));
}
@Test
public void testHTTP10WithKeepAliveAndNoContentLength() throws Exception
{
start(new AbstractHandler()
{
@Override
public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
// Send the headers at this point, then write the content
response.flushBuffer();
response.getOutputStream().print("TEST");
}
});
FuturePromise<Connection> promise = new FuturePromise<>();
Destination destination = client.getDestination(scheme, "localhost", connector.getLocalPort());
destination.newConnection(promise);
try (Connection connection = promise.get(5, TimeUnit.SECONDS))
{
long timeout = 5000;
Request request = client.newRequest(destination.getHost(), destination.getPort())
.scheme(destination.getScheme())
.version(HttpVersion.HTTP_1_0)
.header(HttpHeader.CONNECTION, HttpHeaderValue.KEEP_ALIVE.asString())
.timeout(timeout, TimeUnit.MILLISECONDS);
FutureResponseListener listener = new FutureResponseListener(request);
connection.send(request, listener);
ContentResponse response = listener.get(2 * timeout, TimeUnit.MILLISECONDS);
Assert.assertEquals(200, response.getStatus());
Assert.assertTrue(((HttpConnectionOverHTTP)connection).isClosed());
}
}
@Test
public void testHTTP10WithKeepAliveAndNoContent() throws Exception
{
start(new EmptyServerHandler());
ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
.scheme(scheme)
.version(HttpVersion.HTTP_1_0)
.header(HttpHeader.CONNECTION, HttpHeaderValue.KEEP_ALIVE.asString())
.timeout(5, TimeUnit.SECONDS)
.send();
Assert.assertEquals(200, response.getStatus());
Assert.assertTrue(response.getHeaders().contains(HttpHeader.CONNECTION, HttpHeaderValue.KEEP_ALIVE.asString()));
}
}

View File

@ -105,7 +105,6 @@ public class HttpGenerator
_persistent = null;
_contentPrepared = 0;
_needCRLF = false;
_noContent=false;
}
/* ------------------------------------------------------------ */
@ -744,13 +743,14 @@ public class HttpGenerator
}
else
{
// No idea, so we must assume that a body is coming
_endOfContent = (!isPersistent() || _info.getHttpVersion().ordinal() < HttpVersion.HTTP_1_1.ordinal() ) ? EndOfContent.EOF_CONTENT : EndOfContent.CHUNKED_CONTENT;
if (response!=null && _endOfContent==EndOfContent.EOF_CONTENT)
{
_endOfContent=EndOfContent.NO_CONTENT;
_noContent=true;
}
// No idea, so we must assume that a body is coming.
_endOfContent = EndOfContent.CHUNKED_CONTENT;
// HTTP 1.0 does not understand chunked content, so we must use EOF content.
// For a request with HTTP 1.0 & Connection: keep-alive
// we *must* close the connection, otherwise the client
// has no way to detect the end of the content.
if (!isPersistent() || _info.getHttpVersion().ordinal() < HttpVersion.HTTP_1_1.ordinal())
_endOfContent = EndOfContent.EOF_CONTENT;
}
break;

View File

@ -294,7 +294,7 @@ public class HttpGeneratorServerTest
assertEquals(t, tr[r]._body, this._content);
if (v == 10)
assertTrue(t, gen.isPersistent() || tr[r]._contentLength >= 0 || c == 2 || c == 0);
assertTrue(t, gen.isPersistent() || tr[r]._contentLength >= 0 || c == 2 || c == 1 || c == 0);
else
assertTrue(t, gen.isPersistent() || c == 2 || c == 3);