[#441] Provide a better way to handle decoder failures

* Update toString() of all HttpObject implementations
* HttpMessageDecoder does not raise an exception but sets decoderResult property of the decoded message.
* HttpMessageDecoder discards inbound traffic once decoding fails, by adding a new state called BAD_MESSAGE.
* Add a test case that tests this behavior.
This commit is contained in:
Trustin Lee 2012-09-28 15:16:29 +09:00
parent b923d0c51f
commit 41e0ef2e9a
10 changed files with 243 additions and 7 deletions

View File

@ -51,4 +51,24 @@ public class DefaultHttpChunk extends DefaultHttpObject implements HttpChunk {
public boolean isLast() {
return last;
}
@Override
public String toString() {
StringBuilder buf = new StringBuilder();
buf.append(getClass().getSimpleName());
final boolean last = isLast();
buf.append("(last: ");
buf.append(last);
if (!last) {
buf.append(", size: ");
buf.append(getContent().readableBytes());
}
buf.append(", decodeResult: ");
buf.append(getDecodeResult());
buf.append(')');
return buf.toString();
}
}

View File

@ -17,6 +17,7 @@ package io.netty.handler.codec.http;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.util.internal.StringUtil;
import java.util.List;
import java.util.Map;
@ -104,4 +105,37 @@ public class DefaultHttpChunkTrailer extends DefaultHttpObject implements HttpCh
public void setContent(ByteBuf content) {
throw new IllegalStateException("read-only");
}
@Override
public String toString() {
StringBuilder buf = new StringBuilder();
buf.append(getClass().getSimpleName());
final boolean last = isLast();
buf.append("(last: ");
buf.append(last);
if (!last) {
buf.append(", size: ");
buf.append(getContent().readableBytes());
}
buf.append(", decodeResult: ");
buf.append(getDecodeResult());
buf.append(')');
buf.append(StringUtil.NEWLINE);
appendHeaders(buf);
// Remove the last newline.
buf.setLength(buf.length() - StringUtil.NEWLINE.length());
return buf.toString();
}
private void appendHeaders(StringBuilder buf) {
for (Map.Entry<String, String> e: getHeaders()) {
buf.append(e.getKey());
buf.append(": ");
buf.append(e.getValue());
buf.append(StringUtil.NEWLINE);
}
}
}

View File

@ -70,6 +70,8 @@ public class DefaultHttpRequest extends DefaultHttpMessage implements HttpReques
buf.append(getClass().getSimpleName());
buf.append("(transferEncoding: ");
buf.append(getTransferEncoding());
buf.append(", decodeResult: ");
buf.append(getDecodeResult());
buf.append(')');
buf.append(StringUtil.NEWLINE);
buf.append(getMethod().toString());

View File

@ -54,6 +54,8 @@ public class DefaultHttpResponse extends DefaultHttpMessage implements HttpRespo
buf.append(getClass().getSimpleName());
buf.append("(transferEncoding: ");
buf.append(getTransferEncoding());
buf.append(", decodeResult: ");
buf.append(getDecodeResult());
buf.append(')');
buf.append(StringUtil.NEWLINE);
buf.append(getProtocolVersion().getText());

View File

@ -19,6 +19,7 @@ import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPipeline;
import io.netty.handler.codec.DecodeResult;
import io.netty.handler.codec.ReplayingDecoder;
import io.netty.handler.codec.TooLongFrameException;
@ -125,7 +126,8 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
READ_CHUNKED_CONTENT,
READ_CHUNKED_CONTENT_AS_CHUNKS,
READ_CHUNK_DELIMITER,
READ_CHUNK_FOOTER
READ_CHUNK_FOOTER,
BAD_MESSAGE
}
/**
@ -176,7 +178,7 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
checkpoint();
}
}
case READ_INITIAL: {
case READ_INITIAL: try {
String[] initialLine = splitInitialLine(readLine(buffer, maxInitialLineLength));
if (initialLine.length < 3) {
// Invalid initial line - ignore.
@ -186,8 +188,10 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
message = createMessage(initialLine);
checkpoint(State.READ_HEADER);
} catch (Exception e) {
return invalidMessage(e);
}
case READ_HEADER: {
case READ_HEADER: try {
State nextState = readHeaders(buffer);
checkpoint(nextState);
if (nextState == State.READ_CHUNK_SIZE) {
@ -195,7 +199,7 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
return message;
} else if (nextState == State.SKIP_CONTROL_CHARS) {
// No content is expected.
return message;
return reset();
} else {
long contentLength = HttpHeaders.getContentLength(message, -1);
if (contentLength == 0 || contentLength == -1 && isDecodingRequest()) {
@ -229,6 +233,8 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
}
// We return null here, this forces decode to be called again where we will decode the content
return null;
} catch (Exception e) {
return invalidMessage(e);
}
case READ_VARIABLE_LENGTH_CONTENT: {
int toRead = actualReadableBytes();
@ -238,7 +244,7 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
if (message.getTransferEncoding() != HttpTransferEncoding.STREAMED) {
message.setTransferEncoding(HttpTransferEncoding.STREAMED);
return new Object[] {message, new DefaultHttpChunk(buffer.readBytes(toRead))};
return new Object[] { message, new DefaultHttpChunk(buffer.readBytes(toRead))};
} else {
return new DefaultHttpChunk(buffer.readBytes(toRead));
}
@ -308,7 +314,7 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
* everything else after this point takes care of reading chunked content. basically, read chunk size,
* read chunk, read and ignore the CRLF and repeat until 0
*/
case READ_CHUNK_SIZE: {
case READ_CHUNK_SIZE: try {
String line = readLine(buffer, maxInitialLineLength);
int chunkSize = getChunkSize(line);
this.chunkSize = chunkSize;
@ -321,6 +327,8 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
} else {
checkpoint(State.READ_CHUNKED_CONTENT);
}
} catch (Exception e) {
return invalidChunk(e);
}
case READ_CHUNKED_CONTENT: {
assert chunkSize <= Integer.MAX_VALUE;
@ -378,10 +386,12 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
} else if (next == HttpConstants.LF) {
checkpoint(State.READ_CHUNK_SIZE);
return null;
} else {
checkpoint();
}
}
}
case READ_CHUNK_FOOTER: {
case READ_CHUNK_FOOTER: try {
HttpChunkTrailer trailer = readTrailingHeaders(buffer);
if (maxChunkSize == 0) {
// Chunked encoding disabled.
@ -391,6 +401,13 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
// The last chunk, which is empty
return trailer;
}
} catch (Exception e) {
return invalidChunk(e);
}
case BAD_MESSAGE: {
// Keep discarding until disconnection.
buffer.skipBytes(actualReadableBytes());
return null;
}
default: {
throw new Error("Shouldn't reach here.");
@ -439,6 +456,24 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
return message;
}
private HttpMessage invalidMessage(Exception cause) {
checkpoint(State.BAD_MESSAGE);
if (message != null) {
message.setDecodeResult(DecodeResult.partialFailure(cause));
} else {
message = createInvalidMessage();
message.setDecodeResult(DecodeResult.failure(cause));
}
return message;
}
private HttpChunk invalidChunk(Exception cause) {
checkpoint(State.BAD_MESSAGE);
HttpChunk chunk = new DefaultHttpChunk(Unpooled.EMPTY_BUFFER);
chunk.setDecodeResult(DecodeResult.failure(cause));
return chunk;
}
private static void skipControlCharacters(ByteBuf buffer) {
for (;;) {
char c = (char) buffer.readUnsignedByte();
@ -621,6 +656,8 @@ public abstract class HttpMessageDecoder extends ReplayingDecoder<Object, HttpMe
protected abstract boolean isDecodingRequest();
protected abstract HttpMessage createMessage(String[] initialLine) throws Exception;
protected abstract HttpMessage createInvalidMessage();
private static int getChunkSize(String hex) {
hex = hex.trim();

View File

@ -76,6 +76,11 @@ public class HttpRequestDecoder extends HttpMessageDecoder {
HttpVersion.valueOf(initialLine[2]), HttpMethod.valueOf(initialLine[0]), initialLine[1]);
}
@Override
protected HttpMessage createInvalidMessage() {
return new DefaultHttpRequest(HttpVersion.HTTP_1_0, HttpMethod.GET, "/bad-request");
}
@Override
protected boolean isDecodingRequest() {
return true;

View File

@ -83,6 +83,8 @@ import io.netty.handler.codec.TooLongFrameException;
*/
public class HttpResponseDecoder extends HttpMessageDecoder {
private static final HttpResponseStatus UNKNOWN_STATUS = new HttpResponseStatus(999, "Unknown");
/**
* Creates a new instance with the default
* {@code maxInitialLineLength (4096}}, {@code maxHeaderSize (8192)}, and
@ -106,6 +108,11 @@ public class HttpResponseDecoder extends HttpMessageDecoder {
new HttpResponseStatus(Integer.valueOf(initialLine[1]), initialLine[2]));
}
@Override
protected HttpMessage createInvalidMessage() {
return new DefaultHttpResponse(HttpVersion.HTTP_1_0, UNKNOWN_STATUS);
}
@Override
protected boolean isDecodingRequest() {
return false;

View File

@ -71,6 +71,11 @@ public class RtspRequestDecoder extends RtspMessageDecoder {
RtspMethods.valueOf(initialLine[0]), initialLine[1]);
}
@Override
protected HttpMessage createInvalidMessage() {
return new DefaultHttpRequest(RtspVersions.RTSP_1_0, RtspMethods.OPTIONS, "/bad-request");
}
@Override
protected boolean isDecodingRequest() {
return true;

View File

@ -51,6 +51,8 @@ import io.netty.handler.codec.http.HttpResponseStatus;
*/
public class RtspResponseDecoder extends RtspMessageDecoder {
private static final HttpResponseStatus UNKNOWN_STATUS = new HttpResponseStatus(999, "Unknown");
/**
* Creates a new instance with the default
* {@code maxInitialLineLength (4096}}, {@code maxHeaderSize (8192)}, and
@ -74,6 +76,11 @@ public class RtspResponseDecoder extends RtspMessageDecoder {
new HttpResponseStatus(Integer.valueOf(initialLine[1]), initialLine[2]));
}
@Override
protected HttpMessage createInvalidMessage() {
return new DefaultHttpResponse(RtspVersions.RTSP_1_0, UNKNOWN_STATUS);
}
@Override
protected boolean isDecodingRequest() {
return false;

View File

@ -0,0 +1,117 @@
/*
* Copyright 2012 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;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.embedded.EmbeddedByteChannel;
import io.netty.handler.codec.DecodeResult;
import io.netty.util.CharsetUtil;
import java.util.Random;
import org.junit.Assert;
import org.junit.Test;
public class HttpInvalidMessageTest {
private final Random rnd = new Random();
@Test
public void testRequestWithBadInitialLine() throws Exception {
EmbeddedByteChannel ch = new EmbeddedByteChannel(new HttpRequestDecoder());
ch.writeInbound(Unpooled.copiedBuffer("GET / HTTP/1.0 with extra\r\n", CharsetUtil.UTF_8));
HttpRequest req = (HttpRequest) ch.readInbound();
DecodeResult dr = req.getDecodeResult();
Assert.assertFalse(dr.isSuccess());
Assert.assertFalse(dr.isPartial());
ensureInboundTrafficDiscarded(ch);
}
@Test
public void testRequestWithBadHeader() throws Exception {
EmbeddedByteChannel ch = new EmbeddedByteChannel(new HttpRequestDecoder());
ch.writeInbound(Unpooled.copiedBuffer("GET /maybe-something HTTP/1.0\r\n", CharsetUtil.UTF_8));
ch.writeInbound(Unpooled.copiedBuffer("Good_Name: Good Value\r\n", CharsetUtil.UTF_8));
ch.writeInbound(Unpooled.copiedBuffer("Bad=Name: Bad Value\r\n", CharsetUtil.UTF_8));
ch.writeInbound(Unpooled.copiedBuffer("\r\n", CharsetUtil.UTF_8));
HttpRequest req = (HttpRequest) ch.readInbound();
DecodeResult dr = req.getDecodeResult();
Assert.assertFalse(dr.isSuccess());
Assert.assertTrue(dr.isPartial());
Assert.assertEquals("Good Value", req.getHeader("Good_Name"));
Assert.assertEquals("/maybe-something", req.getUri());
ensureInboundTrafficDiscarded(ch);
}
@Test
public void testResponseWithBadInitialLine() throws Exception {
EmbeddedByteChannel ch = new EmbeddedByteChannel(new HttpResponseDecoder());
ch.writeInbound(Unpooled.copiedBuffer("HTTP/1.0 BAD_CODE Bad Server\r\n", CharsetUtil.UTF_8));
HttpResponse res = (HttpResponse) ch.readInbound();
DecodeResult dr = res.getDecodeResult();
Assert.assertFalse(dr.isSuccess());
Assert.assertFalse(dr.isPartial());
ensureInboundTrafficDiscarded(ch);
}
@Test
public void testResponseWithBadHeader() throws Exception {
EmbeddedByteChannel ch = new EmbeddedByteChannel(new HttpResponseDecoder());
ch.writeInbound(Unpooled.copiedBuffer("HTTP/1.0 200 Maybe OK\r\n", CharsetUtil.UTF_8));
ch.writeInbound(Unpooled.copiedBuffer("Good_Name: Good Value\r\n", CharsetUtil.UTF_8));
ch.writeInbound(Unpooled.copiedBuffer("Bad=Name: Bad Value\r\n", CharsetUtil.UTF_8));
ch.writeInbound(Unpooled.copiedBuffer("\r\n", CharsetUtil.UTF_8));
HttpResponse res = (HttpResponse) ch.readInbound();
DecodeResult dr = res.getDecodeResult();
Assert.assertFalse(dr.isSuccess());
Assert.assertTrue(dr.isPartial());
Assert.assertEquals("Maybe OK", res.getStatus().getReasonPhrase());
Assert.assertEquals("Good Value", res.getHeader("Good_Name"));
ensureInboundTrafficDiscarded(ch);
}
@Test
public void testBadChunk() throws Exception {
EmbeddedByteChannel ch = new EmbeddedByteChannel(new HttpRequestDecoder());
ch.writeInbound(Unpooled.copiedBuffer("GET / HTTP/1.0\r\n", CharsetUtil.UTF_8));
ch.writeInbound(Unpooled.copiedBuffer("Transfer-Encoding: chunked\r\n\r\n", CharsetUtil.UTF_8));
ch.writeInbound(Unpooled.copiedBuffer("BAD_LENGTH\r\n", CharsetUtil.UTF_8));
HttpRequest req = (HttpRequest) ch.readInbound();
Assert.assertTrue(req.getDecodeResult().isSuccess());
HttpChunk chunk = (HttpChunk) ch.readInbound();
DecodeResult dr = chunk.getDecodeResult();
Assert.assertFalse(dr.isSuccess());
Assert.assertFalse(dr.isPartial());
ensureInboundTrafficDiscarded(ch);
}
private void ensureInboundTrafficDiscarded(EmbeddedByteChannel ch) {
// Generate a lot of random traffic to ensure that it's discarded silently.
byte[] data = new byte[1048576];
rnd.nextBytes(data);
ByteBuf buf = Unpooled.wrappedBuffer(data);
for (int i = 0; i < 4096; i ++) {
buf.setIndex(0, data.length);
ch.writeInbound(buf);
ch.checkException();
Assert.assertNull(ch.readInbound());
}
}
}