andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Verify we do not receive multiple content-length headers or a content-length and transfer-encoding: chunked header when using HTTP/1.1 (#9865)

Browse Source 
Motivation:

RFC7230 states that we should not accept multiple content-length headers and also should not accept a content-length header in combination with transfer-encoding: chunked

Modifications:

- Check for multiple content-length headers and if found mark message as invalid
- Check if we found a content-length header and also a transfer-encoding: chunked and if so mark the message as invalid
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9861
This commit is contained in:
Norman Maurer 2019-12-13 08:53:19 +01:00 committed by GitHub
parent 9a0ccf24f3
commit 8494b046ec
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 99 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
View File

@ -600,23 +600,61 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
if (name != null) {
headers.add(name, value);
}
// reset name and value fields
name = null;
value = null;
State nextState;
List<String> values = headers.getAll(HttpHeaderNames.CONTENT_LENGTH);
int contentLengthValuesCount = values.size();
if (contentLengthValuesCount > 0) {
// Guard against multiple Content-Length headers as stated in
// https://tools.ietf.org/html/rfc7230#section-3.3.2:
//
// If a message is received that has multiple Content-Length header
// fields with field-values consisting of the same decimal value, or a
// single Content-Length header field with a field value containing a
// list of identical decimal values (e.g., "Content-Length: 42, 42"),
// indicating that duplicate Content-Length header fields have been
// generated or combined by an upstream message processor, then the
// recipient MUST either reject the message as invalid or replace the
// duplicated field-values with a single valid Content-Length field
// containing that decimal value prior to determining the message body
// length or forwarding the message.
if (contentLengthValuesCount > 1 && message.protocolVersion() == HttpVersion.HTTP_1_1) {
throw new IllegalArgumentException("Multiple Content-Length headers found");
}
contentLength = Long.parseLong(values.get(0));
}
if (isContentAlwaysEmpty(message)) {
HttpUtil.setTransferEncodingChunked(message, false);
nextState = State.SKIP_CONTROL_CHARS;
return State.SKIP_CONTROL_CHARS;
} else if (HttpUtil.isTransferEncodingChunked(message)) {
nextState = State.READ_CHUNK_SIZE;
// See https://tools.ietf.org/html/rfc7230#section-3.3.3
//
// If a message is received with both a Transfer-Encoding and a
// Content-Length header field, the Transfer-Encoding overrides the
// Content-Length. Such a message might indicate an attempt to
// perform request smuggling (Section 9.5) or response splitting
// (Section 9.4) and ought to be handled as an error. A sender MUST
// remove the received Content-Length field prior to forwarding such
// a message downstream.
//
// This is also what http_parser does:
// https://github.com/nodejs/http-parser/blob/v2.9.2/http_parser.c#L1769
if (contentLengthValuesCount > 0 && message.protocolVersion() == HttpVersion.HTTP_1_1) {
throw new IllegalArgumentException(
"Both 'Content-Length: " + contentLength + "' and 'Transfer-Encoding: chunked' found");
}
return State.READ_CHUNK_SIZE;
} else if (contentLength() >= 0) {
nextState = State.READ_FIXED_LENGTH_CONTENT;
return State.READ_FIXED_LENGTH_CONTENT;
} else {
nextState = State.READ_VARIABLE_LENGTH_CONTENT;
return State.READ_VARIABLE_LENGTH_CONTENT;
}
return nextState;
}
private long contentLength() {

64
codec-http/src/test/java/io/netty/handler/codec/http/HttpRequestDecoderTest.java
View File

@ -323,29 +323,75 @@ public class HttpRequestDecoderTest {
@Test
public void testWhitespace() {
EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestDecoder());
String requestStr = "GET /some/path HTTP/1.1\r\n" +
"Transfer-Encoding : chunked\r\n" +
"Host: netty.io\n\r\n";
assertTrue(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)));
HttpRequest request = channel.readInbound();
assertTrue(request.decoderResult().isFailure());
assertTrue(request.decoderResult().cause() instanceof IllegalArgumentException);
assertFalse(channel.finish());
testInvalidHeaders0(requestStr);
}
@Test
public void testHeaderWithNoValueAndMissingColon() {
EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestDecoder());
String requestStr = "GET /some/path HTTP/1.1\r\n" +
"Content-Length: 0\r\n" +
"Host:\r\n" +
"netty.io\r\n\r\n";
testInvalidHeaders0(requestStr);
}
@Test
public void testMultipleContentLengthHeaders() {
String requestStr = "GET /some/path HTTP/1.1\r\n" +
"Content-Length: 1\r\n" +
"Content-Length: 0\r\n\r\n" +
"b";
testInvalidHeaders0(requestStr);
}
@Test
public void testMultipleContentLengthHeaders2() {
String requestStr = "GET /some/path HTTP/1.1\r\n" +
"Content-Length: 1\r\n" +
"Connection: close\r\n" +
"Content-Length: 0\r\n\r\n" +
"b";
testInvalidHeaders0(requestStr);
}
@Test
public void testContentLengthHeaderWithCommaValue() {
String requestStr = "GET /some/path HTTP/1.1\r\n" +
"Content-Length: 1,1\r\n\r\n" +
"b";
testInvalidHeaders0(requestStr);
}
@Test
public void testMultipleContentLengthHeadersWithFolding() {
String requestStr = "POST / HTTP/1.1\r\n" +
"Host: example.com\r\n" +
"Connection: close\r\n" +
"Content-Length: 5\r\n" +
"Content-Length:\r\n" +
"\t6\r\n\r\n" +
"123456";
testInvalidHeaders0(requestStr);
}
@Test
public void testContentLengthHeaderAndChunked() {
String requestStr = "POST / HTTP/1.1\r\n" +
"Host: example.com\r\n" +
"Connection: close\r\n" +
"Content-Length: 5\r\n" +
"Transfer-Encoding: chunked\r\n\r\n" +
"0\r\n\r\n";
testInvalidHeaders0(requestStr);
}
private static void testInvalidHeaders0(String requestStr) {
EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestDecoder());
assertTrue(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)));
HttpRequest request = channel.readInbound();
System.err.println(request.headers().names().toString());
assertTrue(request.decoderResult().isFailure());
assertTrue(request.decoderResult().cause() instanceof IllegalArgumentException);
assertFalse(channel.finish());