Skip to content

Commit

Permalink
HttpObjectAggregator doesn't check content-length header
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Scottmitch committed Aug 17, 2015
1 parent e796c99 commit a7135e8
Show file tree
Hide file tree
Showing 9 changed files with 328 additions and 182 deletions.
Original file line number Diff line number Diff line change
@@ -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() { }
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,16 @@ public static long getContentLength(HttpMessage message, long defaultValue) {
return defaultValue;
}

/**
* Get an {@code int} representation of {@link #getContentLength(HttpMessage, long)}.
* @return the content length or {@code defaultValue} if this message does
* not have the {@code "Content-Length"} header or its value is not
* a number. Not to exceed the boundaries of integer.
*/
public static int getContentLength(HttpMessage message, int defaultValue) {
return (int) Math.min(Integer.MAX_VALUE, HttpHeaderUtil.getContentLength(message, (long) defaultValue));
}

/**
* Returns the content length of the specified web socket message. If the
* specified message is not a web socket message, {@code -1} is returned.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;

import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_LENGTH;
import static io.netty.handler.codec.http.HttpHeaderUtil.getContentLength;

/**
* A {@link ChannelHandler} that aggregates an {@link HttpMessage}
* and its following {@link HttpContent}s into a single {@link FullHttpRequest}
Expand All @@ -50,28 +53,43 @@
*/
public class HttpObjectAggregator
extends MessageAggregator<HttpObject, HttpMessage, HttpContent, FullHttpMessage> {

private static final InternalLogger logger = InternalLoggerFactory.getInstance(HttpObjectAggregator.class);
private static final FullHttpResponse CONTINUE = new DefaultFullHttpResponse(
HttpVersion.HTTP_1_1, HttpResponseStatus.CONTINUE, Unpooled.EMPTY_BUFFER);
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);
private static final FullHttpResponse TOO_LARGE = new DefaultFullHttpResponse(
HttpVersion.HTTP_1_1, HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE, Unpooled.EMPTY_BUFFER);

static {
TOO_LARGE.headers().set(HttpHeaderNames.CONTENT_LENGTH, 0);
EXPECTATION_FAILED.headers().set(CONTENT_LENGTH, 0);
TOO_LARGE.headers().set(CONTENT_LENGTH, 0);
}

private final boolean closeOnExpectationFailed;

/**
* 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,
* {@link #handleOversizedMessage(ChannelHandlerContext, HttpMessage)}
* will be called.
* @param maxContentLength the maximum length of the aggregated content in bytes.
* If the length of the aggregated content exceeds this value,
* {@link #handleOversizedMessage(ChannelHandlerContext, HttpMessage)} will be called.
*/
public HttpObjectAggregator(int 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,
* {@link #handleOversizedMessage(ChannelHandlerContext, HttpMessage)} will be called.
* @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) {
super(maxContentLength);
this.closeOnExpectationFailed = closeOnExpectationFailed;
}

@Override
Expand All @@ -95,22 +113,32 @@ protected boolean isAggregated(HttpObject msg) throws Exception {
}

@Override
protected boolean hasContentLength(HttpMessage start) throws Exception {
return HttpHeaderUtil.isContentLengthSet(start);
protected boolean isContentLengthInvalid(HttpMessage start, int maxContentLength) {
return getContentLength(start, -1) > maxContentLength;
}

@Override
protected Object newContinueResponse(HttpMessage start, int maxContentLength, ChannelPipeline pipeline) {
if (HttpHeaderUtil.is100ContinueExpected(start)) {
if (getContentLength(start, -1) <= maxContentLength) {
return CONTINUE.duplicate().retain();
}

pipeline.fireUserEventTriggered(HttpExpectationFailedEvent.INSTANCE);
return EXPECTATION_FAILED.duplicate().retain();
}
return null;
}

@Override
protected long contentLength(HttpMessage start) throws Exception {
return HttpHeaderUtil.getContentLength(start);
protected boolean closeAfterContinueResponse(Object msg) {
return closeOnExpectationFailed && ignoreContentAfterContinueResponse(msg);
}

@Override
protected Object newContinueResponse(HttpMessage start) throws Exception {
if (HttpHeaderUtil.is100ContinueExpected(start)) {
return CONTINUE;
} else {
return null;
}
protected boolean ignoreContentAfterContinueResponse(Object msg) {
return msg instanceof HttpResponse &&
((HttpResponse) msg).status().code() == HttpResponseStatus.EXPECTATION_FAILED.code();
}

@Override
Expand Down Expand Up @@ -157,7 +185,8 @@ protected void finishAggregation(FullHttpMessage aggregated) throws Exception {
protected void handleOversizedMessage(final ChannelHandlerContext ctx, HttpMessage oversized) throws Exception {
if (oversized instanceof HttpRequest) {
// send back a 413 and close the connection
ChannelFuture future = ctx.writeAndFlush(TOO_LARGE).addListener(new ChannelFutureListener() {
ChannelFuture future = ctx.writeAndFlush(TOO_LARGE.duplicate().retain()).addListener(
new ChannelFutureListener() {
@Override
public void operationComplete(ChannelFuture future) throws Exception {
if (!future.isSuccess()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,16 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc
// 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package io.netty.handler.codec.http.websocketx;

import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelPipeline;
import io.netty.handler.codec.MessageAggregator;
import io.netty.handler.codec.TooLongFrameException;

Expand Down Expand Up @@ -63,18 +64,23 @@ protected boolean isAggregated(WebSocketFrame msg) throws Exception {
}

@Override
protected boolean hasContentLength(WebSocketFrame start) throws Exception {
protected boolean isContentLengthInvalid(WebSocketFrame start, int maxContentLength) {
return false;
}

@Override
protected long contentLength(WebSocketFrame start) throws Exception {
protected Object newContinueResponse(WebSocketFrame start, int maxContentLength, ChannelPipeline pipeline) {
return null;
}

@Override
protected boolean closeAfterContinueResponse(Object msg) throws Exception {
throw new UnsupportedOperationException();
}

@Override
protected Object newContinueResponse(WebSocketFrame start) throws Exception {
return null;
protected boolean ignoreContentAfterContinueResponse(Object msg) throws Exception {
throw new UnsupportedOperationException();
}

@Override
Expand Down
Loading

0 comments on commit a7135e8

Please sign in to comment.