Issue #4208 - Content-Length in 304 (#4211)

* Issue #4208 Content-Length in 304

Added tests for RFC7230 section 3.3.2 for 304 and HEAD responses with content length.
Fixed HttpGenerator to set content-length in 304 response
Fixed insufficient content written check for 304

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4208 Content-Length in 304

Use contentLengthField

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2019-10-17 14:26:19 +11:00 committed by GitHub
parent 869184c827
commit 894fc9b115
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 77 additions and 10 deletions

View File

@ -704,17 +704,24 @@ public class HttpGenerator
_endOfContent = EndOfContent.NO_CONTENT;
// But it is an error if there actually is content
if (_contentPrepared > 0 || contentLength > 0)
if (_contentPrepared > 0)
throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Content for no content response");
if (contentLengthField)
{
if (_contentPrepared == 0 && last)
if (response != null && response.getStatus() == HttpStatus.NOT_MODIFIED_304)
putContentLength(header, contentLength);
else if (contentLength > 0)
{
// TODO discard content for backward compatibility with 9.3 releases
// TODO review if it is still needed in 9.4 or can we just throw.
content.clear();
contentLength = 0;
if (_contentPrepared == 0 && last)
{
// TODO discard content for backward compatibility with 9.3 releases
// TODO review if it is still needed in 9.4 or can we just throw.
content.clear();
}
else
throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Content for no content response");
}
else
throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Content for no content response");
}
}
// Else if we are HTTP/1.1 and the content length is unknown and we are either persistent

View File

@ -507,7 +507,9 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
}
// RFC 7230, section 3.3.
if (!_request.isHead() && !_response.isContentComplete(_response.getHttpOutput().getWritten()))
if (!_request.isHead() &&
_response.getStatus() != HttpStatus.NOT_MODIFIED_304 &&
!_response.isContentComplete(_response.getHttpOutput().getWritten()))
{
if (isCommitted())
abort(new IOException("insufficient content written"));

View File

@ -113,7 +113,7 @@ public class DumpHandler extends AbstractHandler.ErrorDispatchHandler
writer.write("<pre>\nlocal=" + request.getLocalAddr() + ":" + request.getLocalPort() + "\n</pre>\n");
writer.write("<pre>\nremote=" + request.getRemoteAddr() + ":" + request.getRemotePort() + "\n</pre>\n");
writer.write("<h3>Header:</h3><pre>");
writer.write(request.getMethod() + " " + request.getRequestURI() + " " + request.getProtocol() + "\n");
writer.write(String.format("%4s %s %s\n", request.getMethod(), request.getRequestURI(), request.getProtocol()));
Enumeration<String> headers = request.getHeaderNames();
while (headers.hasMoreElements())
{

View File

@ -24,6 +24,7 @@ import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.server.handler.AbstractHandler;
@ -432,6 +433,21 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest
}
}
@ParameterizedTest
@MethodSource("httpVersions")
public void testSetContentLengthAnd304Status(HttpVersion httpVersion) throws Exception
{
server.setHandler(new SetContentLength304Handler());
server.start();
HttpTester.Response response = executeRequest(httpVersion);
assertThat("response code", response.getStatus(), is(304));
assertThat(response, containsHeaderValue("content-length", "32768"));
byte[] content = response.getContentBytes();
assertThat(content.length, is(0));
assertFalse(response.isEarlyEOF());
}
@ParameterizedTest
@MethodSource("httpVersions")
public void testSetContentLengthFlushAndWriteInsufficientBytes(HttpVersion httpVersion) throws Exception
@ -519,6 +535,21 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest
}
}
private class SetContentLength304Handler extends AbstractHandler
{
private SetContentLength304Handler()
{
}
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
baseRequest.setHandled(true);
response.setContentLength(32768);
response.setStatus(HttpStatus.NOT_MODIFIED_304);
}
}
private class SetContentLengthAndWriteThatAmountOfBytesHandler extends ThrowExceptionOnDemandHandler
{
private SetContentLengthAndWriteThatAmountOfBytesHandler(boolean throwException)

View File

@ -107,6 +107,33 @@ public class PartialRFC2616Test
}
}
@Test
public void test3_3_2()
{
try
{
String get = connector.getResponse("GET /R1 HTTP/1.0\n" + "Host: localhost\n" + "\n");
checkContains(get, 0, "HTTP/1.1 200", "GET");
checkContains(get, 0, "Content-Type: text/html", "GET _content");
checkContains(get, 0, "<html>", "GET body");
int cli = get.indexOf("Content-Length");
String contentLength = get.substring(cli,get.indexOf("\r",cli));
String head = connector.getResponse("HEAD /R1 HTTP/1.0\n" + "Host: localhost\n" + "\n");
checkContains(head, 0, "HTTP/1.1 200", "HEAD");
checkContains(head, 0, "Content-Type: text/html", "HEAD _content");
assertEquals(-1, head.indexOf("<html>"), "HEAD no body");
checkContains(head, 0, contentLength, "3.3.2 HEAD");
}
catch (Exception e)
{
e.printStackTrace();
assertTrue(false);
}
}
@Test
public void test3_6_a() throws Exception
{