Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fixes #4203 and #4204 - Transfer-Encoding + Content-Length behaviors (#4205) 

* Issue #4203 - Updating test to verify report on Transfer-Encoding

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

* Fixes #4203 - Transfer-Encoding + Content-Length is 400 Bad Request

+ Fixing validation to not be header order dependent.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

* Issue #4203 - Fixing hasTransferEncoding reset and testcase assumption

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

* Issue #4204 - Transfer-Encoding RFC7230 behaviors

+ More test cases and implementation.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

* Issue #4204 - Transfer-Encoding RFC7230 behaviors

+ Adjusting HttpParser to handle the case where we have multiple
  Transfer-Encoding headers and none declare the 'chunked' token.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

* Issue #4204 - Transfer-Encoding RFC7230 behaviors

+ Making changes from PR review

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

* Issue #4203 Transfer Encoding

request with TE and no chunking is a Bad Request

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Joakim Erdfelt 2019-10-18 14:05:15 -07:00 committed by Greg Wilkins
parent 3d19f61122
commit 890c0b26cb
5 changed files with 208 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
View File

@ -169,6 +169,7 @@ public class HttpParser
private Utf8StringBuilder _uri = new Utf8StringBuilder(INITIAL_URI_LENGTH); // Tune?
private EndOfContent _endOfContent;
private boolean _hasContentLength;
private boolean _hasTransferEncoding;
private long _contentLength = -1;
private long _contentPosition;
private int _chunkLength;
@ -955,6 +956,9 @@ public class HttpParser
switch (_header)
{
case CONTENT_LENGTH:
if (_hasTransferEncoding && complianceViolation(TRANSFER_ENCODING_WITH_CONTENT_LENGTH))
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Transfer-Encoding and Content-Length");
if (_hasContentLength)
{
if (complianceViolation(MULTIPLE_CONTENT_LENGTHS))
@ -964,9 +968,6 @@ public class HttpParser
}
_hasContentLength = true;
if (_endOfContent == EndOfContent.CHUNKED_CONTENT && complianceViolation(TRANSFER_ENCODING_WITH_CONTENT_LENGTH))
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Bad Content-Length");
if (_endOfContent != EndOfContent.CHUNKED_CONTENT)
{
_contentLength = convertContentLength(_valueString);
@ -978,9 +979,15 @@ public class HttpParser
break;
case TRANSFER_ENCODING:
_hasTransferEncoding = true;
if (_hasContentLength && complianceViolation(TRANSFER_ENCODING_WITH_CONTENT_LENGTH))
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Transfer-Encoding and Content-Length");
// we encountered another Transfer-Encoding header, but chunked was already set
if (_endOfContent == EndOfContent.CHUNKED_CONTENT)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Bad Transfer-Encoding, chunked not last");
if (HttpHeaderValue.CHUNKED.is(_valueString))
{
_endOfContent = EndOfContent.CHUNKED_CONTENT;
@ -989,15 +996,26 @@ public class HttpParser
else
{
List<String> values = new QuotedCSV(_valueString).getValues();
if (!values.isEmpty() && HttpHeaderValue.CHUNKED.is(values.get(values.size() - 1)))
int chunked = -1;
int len = values.size();
for (int i = 0; i < len; i++)
{
_endOfContent = EndOfContent.CHUNKED_CONTENT;
_contentLength = -1;
if (HttpHeaderValue.CHUNKED.is(values.get(i)))
{
if (chunked != -1)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Bad Transfer-Encoding, multiple chunked tokens");
chunked = i;
// declared chunked
_endOfContent = EndOfContent.CHUNKED_CONTENT;
_contentLength = -1;
}
// we have a non-chunked token after a declared chunked token
else if (_endOfContent == EndOfContent.CHUNKED_CONTENT)
{
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Bad Transfer-Encoding, chunked not last");
}
}
else if (values.stream().anyMatch(HttpHeaderValue.CHUNKED::is))
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Bad chunking");
}
break;
case HOST:
@ -1139,6 +1157,17 @@ public class HttpParser
return _handler.messageComplete();
}
// We found Transfer-Encoding headers, but none declared the 'chunked' token
if (_hasTransferEncoding && _endOfContent != EndOfContent.CHUNKED_CONTENT)
{
if (_responseHandler == null || _endOfContent != EndOfContent.EOF_CONTENT)
{
// Transfer-Encoding chunked not specified
// https://tools.ietf.org/html/rfc7230#section-3.3.1
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Bad Transfer-Encoding, chunked not last");
}
}
// Was there a required host header?
if (!_host && _version == HttpVersion.HTTP_1_1 && _requestHandler != null)
{
@ -1818,6 +1847,7 @@ public class HttpParser
_endOfContent = EndOfContent.UNKNOWN_CONTENT;
_contentLength = -1;
_hasContentLength = false;
_hasTransferEncoding = false;
_contentPosition = 0;
_responseStatus = 0;
_contentChunk = null;

2
jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
View File

@ -1050,7 +1050,7 @@ public class HttpParserTest
assertEquals("GET", _methodOrVersion);
assertEquals("/chunk", _uriOrStatus);
assertEquals("HTTP/1.0", _versionOrReason);
assertThat(_bad, containsString("Bad chunking"));
assertThat(_bad, containsString("Bad Transfer-Encoding"));
}
@Test

190
jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
View File

@ -30,10 +30,13 @@ import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintWriter;
import java.io.StringReader;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@ -54,6 +57,9 @@ import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
@ -263,43 +269,172 @@ public class HttpConnectionTest
}
}
static final int CHUNKED = -1;
static final int DQUOTED_CHUNKED = -2;
static final int BAD_CHUNKED = -3;
static final int UNKNOWN_TE = -4;
public static Stream<Arguments> http11ContentLengthAndChunkedData()
{
return Stream.of(
Arguments.of(new int[]{CHUNKED, 8}),
Arguments.of(new int[]{8, CHUNKED}),
Arguments.of(new int[]{8, CHUNKED, 8}),
Arguments.of(new int[]{DQUOTED_CHUNKED, 8}),
Arguments.of(new int[]{8, DQUOTED_CHUNKED}),
Arguments.of(new int[]{8, DQUOTED_CHUNKED, 8}),
Arguments.of(new int[]{BAD_CHUNKED, 8}),
Arguments.of(new int[]{8, BAD_CHUNKED}),
Arguments.of(new int[]{8, BAD_CHUNKED, 8}),
Arguments.of(new int[]{UNKNOWN_TE, 8}),
Arguments.of(new int[]{8, UNKNOWN_TE}),
Arguments.of(new int[]{8, UNKNOWN_TE, 8}),
Arguments.of(new int[]{8, UNKNOWN_TE, CHUNKED, DQUOTED_CHUNKED, BAD_CHUNKED, 8})
);
}
/**
* More then 1 Content-Length is a bad requests per HTTP rfcs.
*/
@Test
public void testHttp11ContentLengthAndChunk() throws Exception
@ParameterizedTest
@MethodSource("http11ContentLengthAndChunkedData")
public void testHttp11ContentLengthAndChunk(int[] contentLengths) throws Exception
{
HttpParser.LOG.info("badMessage: 400 Bad messages EXPECTED...");
int[][] contentLengths = {
{-1, 8},
{8, -1},
{8, -1, 8},
};
for (int x = 0; x < contentLengths.length; x++)
StringBuilder request = new StringBuilder();
request.append("POST / HTTP/1.1\r\n");
request.append("Host: local\r\n");
for (int n = 0; n < contentLengths.length; n++)
{
StringBuilder request = new StringBuilder();
request.append("POST /?id=").append(Integer.toString(x)).append(" HTTP/1.1\r\n");
request.append("Host: local\r\n");
int[] clen = contentLengths[x];
for (int n = 0; n < clen.length; n++)
switch (contentLengths[n])
{
if (clen[n] == -1)
case CHUNKED:
request.append("Transfer-Encoding: chunked\r\n");
else
request.append("Content-Length: ").append(Integer.toString(clen[n])).append("\r\n");
break;
case DQUOTED_CHUNKED:
request.append("Transfer-Encoding: \"chunked\"\r\n");
break;
case BAD_CHUNKED:
request.append("Transfer-Encoding: 'chunked'\r\n");
break;
case UNKNOWN_TE:
request.append("Transfer-Encoding: bogus\r\n");
break;
default:
request.append("Content-Length: ").append(contentLengths[n]).append("\r\n");
break;
}
request.append("Content-Type: text/plain\r\n");
request.append("Connection: close\r\n");
request.append("\r\n");
request.append("8;\r\n"); // chunk header
request.append("abcdefgh"); // actual content of 8 bytes
request.append("\r\n0;\r\n"); // last chunk
String rawResponse = connector.getResponse(request.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat("Response.status", response.getStatus(), is(HttpServletResponse.SC_BAD_REQUEST));
}
request.append("Content-Type: text/plain\r\n");
request.append("\r\n");
request.append("8;\r\n"); // chunk header
request.append("abcdefgh"); // actual content of 8 bytes
request.append("\r\n0;\r\n\r\n"); // last chunk
String rawResponse = connector.getResponse(request.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat("Response.status", response.getStatus(), is(HttpServletResponse.SC_BAD_REQUEST));
}
/**
* Examples of valid Chunked behaviors.
*/
public static Stream<Arguments> http11TransferEncodingChunked()
{
return Stream.of(
Arguments.of(Arrays.asList("chunked, ")), // results in 1 entry
Arguments.of(Arrays.asList(", chunked")),
// invalid tokens with chunked as last
// no conflicts, chunked token is specified and is last, will result in chunked
Arguments.of(Arrays.asList("bogus, chunked")),
Arguments.of(Arrays.asList("'chunked', chunked")), // apostrophe characters with and without
Arguments.of(Arrays.asList("identity, chunked")), // identity was removed in RFC2616 errata and has been dropped in RFC7230
// multiple headers
Arguments.of(Arrays.asList("identity", "chunked")), // 2 separate headers
Arguments.of(Arrays.asList("", "chunked")) // 2 separate headers
);
}
/**
* Test Chunked Transfer-Encoding behavior indicated by
* https://tools.ietf.org/html/rfc7230#section-3.3.1
*/
@ParameterizedTest
@MethodSource("http11TransferEncodingChunked")
public void testHttp11TransferEncodingChunked(List<String> tokens) throws Exception
{
StringBuilder request = new StringBuilder();
request.append("POST / HTTP/1.1\r\n");
request.append("Host: local\r\n");
tokens.forEach((token) -> request.append("Transfer-Encoding: ").append(token).append("\r\n"));
request.append("Content-Type: text/plain\r\n");
request.append("\r\n");
request.append("8;\r\n"); // chunk header
request.append("abcdefgh"); // actual content of 8 bytes
request.append("\r\n0;\r\n\r\n"); // last chunk
System.out.println(request.toString());
String rawResponse = connector.getResponse(request.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat("Response.status (" + response.getReason() + ")", response.getStatus(), is(HttpServletResponse.SC_OK));
}
public static Stream<Arguments> http11TransferEncodingInvalidChunked()
{
return Stream.of(
// == Results in 400 Bad Request
Arguments.of(Arrays.asList("bogus", "identity")), // 2 separate headers
Arguments.of(Arrays.asList("bad")),
Arguments.of(Arrays.asList("identity")), // identity was removed in RFC2616 errata and has been dropped in RFC7230
Arguments.of(Arrays.asList("'chunked'")), // apostrophe characters
Arguments.of(Arrays.asList("`chunked`")), // backtick "quote" characters
Arguments.of(Arrays.asList("[chunked]")), // bracketed (seen as mistake in several REST libraries)
Arguments.of(Arrays.asList("{chunked}")), // json'd (seen as mistake in several REST libraries)
Arguments.of(Arrays.asList("\u201Cchunked\u201D")), // opening and closing (fancy) double quotes characters
// invalid tokens with chunked not as last
Arguments.of(Arrays.asList("chunked, bogus")),
Arguments.of(Arrays.asList("chunked, 'chunked'")),
Arguments.of(Arrays.asList("chunked, identity")),
Arguments.of(Arrays.asList("chunked, identity, chunked")), // duplicate chunked
Arguments.of(Arrays.asList("chunked", "identity")), // 2 separate header lines
// multiple chunked tokens present
Arguments.of(Arrays.asList("chunked", "identity", "chunked")), // 3 separate header lines
Arguments.of(Arrays.asList("chunked", "chunked")), // 2 separate header lines
Arguments.of(Arrays.asList("chunked, chunked")) // on same line
);
}
/**
* Test bad Transfer-Encoding behavior as indicated by
* https://tools.ietf.org/html/rfc7230#section-3.3.1
*/
@ParameterizedTest
@MethodSource("http11TransferEncodingInvalidChunked")
public void testHttp11TransferEncodingInvalidChunked(List<String> tokens) throws Exception
{
HttpParser.LOG.info("badMessage: 400 Bad messages EXPECTED...");
StringBuilder request = new StringBuilder();
request.append("POST / HTTP/1.1\r\n");
request.append("Host: local\r\n");
tokens.forEach((token) -> request.append("Transfer-Encoding: ").append(token).append("\r\n"));
request.append("Content-Type: text/plain\r\n");
request.append("\r\n");
request.append("8;\r\n"); // chunk header
request.append("abcdefgh"); // actual content of 8 bytes
request.append("\r\n0;\r\n\r\n"); // last chunk
System.out.println(request.toString());
String rawResponse = connector.getResponse(request.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat("Response.status", response.getStatus(), is(HttpServletResponse.SC_BAD_REQUEST));
}
@Test
@ -541,11 +676,10 @@ public class HttpConnectionTest
"Host: localhost\r\n" +
"Transfer-Encoding: chunked\r\n" +
"Content-Type: text/plain\r\n" +
"Connection: close\r\n" +
"\r\n" +
"A\r\n" +
"0123456789\r\n" +
"0\r\n");
"0\r\n\r\n");
int offset = 0;
offset = checkContains(response, offset, "HTTP/1.1 200");

9
jetty-server/src/test/java/org/eclipse/jetty/server/PartialRFC2616Test.java
View File

@ -35,6 +35,7 @@ import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ -348,12 +349,10 @@ public class PartialRFC2616Test
"\n");
offset = 0;
response = endp.getResponse();
offset = checkContains(response, offset, "HTTP/1.1 200 OK", "2. identity") + 10;
offset = checkContains(response, offset, "/R1", "2. identity") + 3;
offset = checkContains(response, offset, "HTTP/1.1 400 ", "2. identity") + 10;
offset = 0;
response = endp.getResponse();
offset = checkContains(response, offset, "HTTP/1.1 200 OK", "2. identity") + 10;
offset = checkContains(response, offset, "/R2", "2. identity") + 3;
assertThat("There should be no next response as first one closed connection", response, is(nullValue()));
}
@Test
@ -385,7 +384,7 @@ public class PartialRFC2616Test
"\n" +
"abcdef");
response = endp.getResponse();
offset = checkContains(response, offset, "HTTP/1.1 400 Bad", "3. ignore c-l") + 1;
offset = checkContains(response, offset, "HTTP/1.1 400 ", "3. ignore c-l") + 1;
checkNotContained(response, offset, "/R2", "3. _content-length");
}

13
tests/test-integration/src/test/java/org/eclipse/jetty/test/rfcs/RFC2616BaseTest.java
View File

@ -368,20 +368,11 @@ public abstract class RFC2616BaseTest
req1.append("\n");
req1.append("123\r\n");
req1.append("GET /echo/R2 HTTP/1.1\n");
req1.append("Host: localhost\n");
req1.append("Connection: close\n");
req1.append("\n");
List<HttpTester.Response> responses = http.requests(req1);
assertEquals(2, responses.size(), "Response Count");
assertEquals(1, responses.size(), "Response Count");
HttpTester.Response response = responses.get(0);
assertThat("4.4.2 Message Length / Response Code", response.getStatus(), is(HttpStatus.OK_200));
assertThat("4.4.2 Message Length / Body", response.getContent(), Matchers.containsString("123\n"));
response = responses.get(1);
assertThat("4.4.2 Message Length / Response Code", response.getStatus(), is(HttpStatus.OK_200));
assertEquals("", response.getContent(), "4.4.2 Message Length / No Body");
assertThat("4.4.2 Message Length / Response Code", response.getStatus(), is(HttpStatus.BAD_REQUEST_400));
// 4.4.3 -
// Client - do not send 'Content-Length' if entity-length