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

HttpObjectAggregator doesn't check content-length header

Browse Source 
Motivation:
The HttpObjectAggregator always responds with a 100-continue response. It should check the Content-Length header to see if the content length is OK, and if not responds with a 417.

Modifications:
- HttpObjectAggregator checks the Content-Length header in the case of a 100-continue.

Result:
HttpObjectAggregator responds with 417 if content is known to be too big.
This commit is contained in:
Scott Mitchell 2015-08-03 15:09:44 -07:00
parent 366b2a82f6
commit 0255a0ae73
4 changed files with 224 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
codec-http/src/main/java/io/netty/handler/codec/http/HttpExpectationFailedEvent.java Normal file
View File

@ -0,0 +1,25 @@
/*
* Copyright 2015 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty.handler.codec.http;
/**
* A user event designed to communicate that a expectation has failed and there should be no expectation that a
* body will follow.
*/
public final class HttpExpectationFailedEvent {
public static final HttpExpectationFailedEvent INSTANCE = new HttpExpectationFailedEvent();
private HttpExpectationFailedEvent() { }
}

60
codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java
View File

@ -18,7 +18,6 @@ package io.netty.handler.codec.http;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufHolder;
import io.netty.buffer.CompositeByteBuf;
import io.netty.buffer.DefaultByteBufHolder;
import io.netty.buffer.Unpooled;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelFutureListener;
@ -28,10 +27,13 @@ import io.netty.channel.ChannelPipeline;
import io.netty.handler.codec.DecoderResult;
import io.netty.handler.codec.MessageToMessageDecoder;
import io.netty.handler.codec.TooLongFrameException;
import io.netty.handler.codec.http.HttpHeaders.Names;
import java.util.List;
import static io.netty.handler.codec.http.HttpHeaders.*;
import static io.netty.handler.codec.http.HttpHeaders.is100ContinueExpected;
import static io.netty.handler.codec.http.HttpHeaders.isContentLengthSet;
import static io.netty.handler.codec.http.HttpHeaders.removeTransferEncodingChunked;
/**
* A {@link ChannelHandler} that aggregates an {@link HttpMessage}
@ -56,10 +58,17 @@ public class HttpObjectAggregator extends MessageToMessageDecoder<HttpObject> {
public static final int DEFAULT_MAX_COMPOSITEBUFFER_COMPONENTS = 1024;
private static final FullHttpResponse CONTINUE =
new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.CONTINUE, Unpooled.EMPTY_BUFFER);
private static final FullHttpResponse EXPECTATION_FAILED = new DefaultFullHttpResponse(
HttpVersion.HTTP_1_1, HttpResponseStatus.EXPECTATION_FAILED, Unpooled.EMPTY_BUFFER);
static {
HttpHeaders.setContentLength(EXPECTATION_FAILED, 0);
}
private final int maxContentLength;
private AggregatedFullHttpMessage currentMessage;
private boolean tooLongFrameFound;
private final boolean closeOnExpectationFailed;
private int maxCumulationBufferComponents = DEFAULT_MAX_COMPOSITEBUFFER_COMPONENTS;
private ChannelHandlerContext ctx;
@ -73,14 +82,26 @@ public class HttpObjectAggregator extends MessageToMessageDecoder<HttpObject> {
* a {@link TooLongFrameException} will be raised.
*/
public HttpObjectAggregator(int maxContentLength) {
if (maxContentLength <= 0) {
throw new IllegalArgumentException(
"maxContentLength must be a positive integer: " +
maxContentLength);
}
this.maxContentLength = maxContentLength;
this(maxContentLength, false);
}
/**
* Creates a new instance.
* @param maxContentLength
* the maximum length of the aggregated content in bytes.
* If the length of the aggregated content exceeds this value,
* a {@link TooLongFrameException} will be raised.
* @param closeOnExpectationFailed If a 100-continue response is detected but the content length is too large
* then {@code true} means close the connection. otherwise the connection will remain open and data will be
* consumed and discarded until the next request is received.
*/
public HttpObjectAggregator(int maxContentLength, boolean closeOnExpectationFailed) {
if (maxContentLength <= 0) {
throw new IllegalArgumentException("maxContentLength must be a positive integer: " + maxContentLength);
}
this.maxContentLength = maxContentLength;
this.closeOnExpectationFailed = closeOnExpectationFailed;
}
/**
* Returns the maximum number of components in the cumulation buffer. If the number of
* the components in the cumulation buffer exceeds this value, the components of the
@ -124,12 +145,25 @@ public class HttpObjectAggregator extends MessageToMessageDecoder<HttpObject> {
HttpMessage m = (HttpMessage) msg;
// Handle the 'Expect: 100-continue' header if necessary.
// TODO: Respond with 413 Request Entity Too Large
// and discard the traffic or close the connection.
// No need to notify the upstream handlers - just log.
// If decoding a response, just throw an exception.
if (is100ContinueExpected(m)) {
ctx.writeAndFlush(CONTINUE).addListener(new ChannelFutureListener() {
if (HttpHeaders.getContentLength(m, 0) > maxContentLength) {
tooLongFrameFound = true;
final ChannelFuture future = ctx.writeAndFlush(EXPECTATION_FAILED.duplicate().retain());
future.addListener(new ChannelFutureListener() {
@Override
public void operationComplete(ChannelFuture future) throws Exception {
if (!future.isSuccess()) {
ctx.fireExceptionCaught(future.cause());
}
}
});
if (closeOnExpectationFailed) {
future.addListener(ChannelFutureListener.CLOSE);
}
ctx.pipeline().fireUserEventTriggered(HttpExpectationFailedEvent.INSTANCE);
return;
}
ctx.writeAndFlush(CONTINUE.duplicate().retain()).addListener(new ChannelFutureListener() {
@Override
public void operationComplete(ChannelFuture future) throws Exception {
if (!future.isSuccess()) {

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

@ -433,6 +433,16 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
// This method is responsible for ending requests in some situations and must be called
// when the input has been shutdown.
super.channelInactive(ctx);
} else if (evt instanceof HttpExpectationFailedEvent) {
switch (currentState) {
case READ_FIXED_LENGTH_CONTENT:
case READ_VARIABLE_LENGTH_CONTENT:
case READ_CHUNK_SIZE:
reset();
break;
default:
break;
}
}
super.userEventTriggered(ctx, evt);
}

142
codec-http/src/test/java/io/netty/handler/codec/http/HttpObjectAggregatorTest.java
View File

@ -209,4 +209,146 @@ public class HttpObjectAggregatorTest {
assertNull(ch.readInbound());
ch.finish();
}
@Test
public void testOversizedRequestWith100Continue() {
EmbeddedChannel embedder = new EmbeddedChannel(new HttpObjectAggregator(8));
// send an oversized request with 100 continue
HttpRequest message = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "http://localhost");
HttpHeaders.set100ContinueExpected(message, true);
HttpHeaders.setContentLength(message, 16);
HttpContent chunk1 = releaseLater(new DefaultHttpContent(Unpooled.copiedBuffer("some", CharsetUtil.US_ASCII)));
HttpContent chunk2 = releaseLater(new DefaultHttpContent(Unpooled.copiedBuffer("test", CharsetUtil.US_ASCII)));
HttpContent chunk3 = LastHttpContent.EMPTY_LAST_CONTENT;
// Send a request with 100-continue + large Content-Length header value.
assertFalse(embedder.writeInbound(message));
// The aggregator should respond with '417.'
FullHttpResponse response = (FullHttpResponse) embedder.readOutbound();
assertEquals(HttpResponseStatus.EXPECTATION_FAILED, response.getStatus());
assertEquals("0", response.headers().get(HttpHeaders.Names.CONTENT_LENGTH));
// An ill-behaving client could continue to send data without a respect, and such data should be discarded.
assertFalse(embedder.writeInbound(chunk1));
// The aggregator should not close the connection because keep-alive is on.
assertTrue(embedder.isOpen());
// Now send a valid request.
HttpRequest message2 = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "http://localhost");
assertFalse(embedder.writeInbound(message2));
assertFalse(embedder.writeInbound(chunk2));
assertTrue(embedder.writeInbound(chunk3));
FullHttpRequest fullMsg = (FullHttpRequest) embedder.readInbound();
assertNotNull(fullMsg);
assertEquals(
chunk2.content().readableBytes() + chunk3.content().readableBytes(),
HttpHeaders.getContentLength(fullMsg));
assertEquals(HttpHeaders.getContentLength(fullMsg), fullMsg.content().readableBytes());
fullMsg.release();
assertFalse(embedder.finish());
}
@Test
public void testOversizedRequestWith100ContinueAndDecoder() {
EmbeddedChannel embedder = new EmbeddedChannel(new HttpRequestDecoder(), new HttpObjectAggregator(4));
embedder.writeInbound(Unpooled.copiedBuffer(
"PUT /upload HTTP/1.1\r\n" +
"Expect: 100-continue\r\n" +
"Content-Length: 100\r\n\r\n", CharsetUtil.US_ASCII));
assertNull(embedder.readInbound());
FullHttpResponse response = (FullHttpResponse) embedder.readOutbound();
assertEquals(HttpResponseStatus.EXPECTATION_FAILED, response.getStatus());
assertEquals("0", response.headers().get(HttpHeaders.Names.CONTENT_LENGTH));
// Keep-alive is on by default in HTTP/1.1, so the connection should be still alive.
assertTrue(embedder.isOpen());
// The decoder should be reset by the aggregator at this point and be able to decode the next request.
embedder.writeInbound(Unpooled.copiedBuffer("GET /max-upload-size HTTP/1.1\r\n\r\n", CharsetUtil.US_ASCII));
FullHttpRequest request = (FullHttpRequest) embedder.readInbound();
assertThat(request.getMethod(), is(HttpMethod.GET));
assertThat(request.getUri(), is("/max-upload-size"));
assertThat(request.content().readableBytes(), is(0));
request.release();
assertFalse(embedder.finish());
}
@Test
public void testOversizedRequestWith100ContinueAndDecoderCloseConnection() {
EmbeddedChannel embedder = new EmbeddedChannel(new HttpRequestDecoder(), new HttpObjectAggregator(4, true));
embedder.writeInbound(Unpooled.copiedBuffer(
"PUT /upload HTTP/1.1\r\n" +
"Expect: 100-continue\r\n" +
"Content-Length: 100\r\n\r\n", CharsetUtil.US_ASCII));
assertNull(embedder.readInbound());
FullHttpResponse response = (FullHttpResponse) embedder.readOutbound();
assertEquals(HttpResponseStatus.EXPECTATION_FAILED, response.getStatus());
assertEquals("0", response.headers().get(HttpHeaders.Names.CONTENT_LENGTH));
// We are forcing the connection closed if an expectation is exceeded.
assertFalse(embedder.isOpen());
assertFalse(embedder.finish());
}
@Test
public void testRequestAfterOversized100ContinueAndDecoder() {
EmbeddedChannel embedder = new EmbeddedChannel(new HttpRequestDecoder(), new HttpObjectAggregator(15));
// Write first request with Expect: 100-continue
HttpRequest message = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "http://localhost");
HttpHeaders.set100ContinueExpected(message, true);
HttpHeaders.setContentLength(message, 16);
HttpContent chunk1 = releaseLater(new DefaultHttpContent(Unpooled.copiedBuffer("some", CharsetUtil.US_ASCII)));
HttpContent chunk2 = releaseLater(new DefaultHttpContent(Unpooled.copiedBuffer("test", CharsetUtil.US_ASCII)));
HttpContent chunk3 = LastHttpContent.EMPTY_LAST_CONTENT;
// Send a request with 100-continue + large Content-Length header value.
assertFalse(embedder.writeInbound(message));
// The aggregator should respond with '417'
FullHttpResponse response = (FullHttpResponse) embedder.readOutbound();
assertEquals(HttpResponseStatus.EXPECTATION_FAILED, response.getStatus());
assertEquals("0", response.headers().get(HttpHeaders.Names.CONTENT_LENGTH));
// An ill-behaving client could continue to send data without a respect, and such data should be discarded.
assertFalse(embedder.writeInbound(chunk1));
// The aggregator should not close the connection because keep-alive is on.
assertTrue(embedder.isOpen());
// Now send a valid request.
HttpRequest message2 = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.PUT, "http://localhost");
assertFalse(embedder.writeInbound(message2));
assertFalse(embedder.writeInbound(chunk2));
assertTrue(embedder.writeInbound(chunk3));
FullHttpRequest fullMsg = (FullHttpRequest) embedder.readInbound();
assertNotNull(fullMsg);
assertEquals(
chunk2.content().readableBytes() + chunk3.content().readableBytes(),
HttpHeaders.getContentLength(fullMsg));
assertEquals(HttpHeaders.getContentLength(fullMsg), fullMsg.content().readableBytes());
fullMsg.release();
assertFalse(embedder.finish());
}
}