diff --git a/src/main/java/org/jboss/netty/handler/codec/frame/LineBasedFrameDecoder.java b/src/main/java/org/jboss/netty/handler/codec/frame/LineBasedFrameDecoder.java index d4068f65a4..27f25f6de8 100644 --- a/src/main/java/org/jboss/netty/handler/codec/frame/LineBasedFrameDecoder.java +++ b/src/main/java/org/jboss/netty/handler/codec/frame/LineBasedFrameDecoder.java @@ -17,8 +17,8 @@ package org.jboss.netty.handler.codec.frame; import org.jboss.netty.buffer.ChannelBuffer; import org.jboss.netty.channel.Channel; -import org.jboss.netty.channel.Channels; import org.jboss.netty.channel.ChannelHandlerContext; +import org.jboss.netty.channel.Channels; /** * A decoder that splits the received {@link ChannelBuffer}s on line endings. @@ -36,6 +36,7 @@ public class LineBasedFrameDecoder extends FrameDecoder { /** True if we're discarding input because we're already over maxLength. */ private boolean discarding; + private int discardedBytes; /** * Creates a new decoder. @@ -73,51 +74,67 @@ public class LineBasedFrameDecoder extends FrameDecoder { final Channel channel, final ChannelBuffer buffer) throws Exception { final int eol = findEndOfLine(buffer); - if (eol != -1) { - final ChannelBuffer frame; - final int length = eol - buffer.readerIndex(); - assert length >= 0: "Invalid length=" + length; - if (discarding) { - frame = null; - buffer.skipBytes(length); + if (!discarding) { + if (eol >= 0) { + final ChannelBuffer frame; + final int length = eol - buffer.readerIndex(); + final int delimLength = buffer.getByte(eol) == '\r'? 2 : 1; + + if (length > maxLength) { + buffer.readerIndex(eol + delimLength); + fail(ctx, length); + return null; + } + + try { + if (stripDelimiter) { + frame = extractFrame(buffer, buffer.readerIndex(), length); + } else { + frame = extractFrame(buffer, buffer.readerIndex(), length + delimLength); + } + } finally { + buffer.skipBytes(length + delimLength); + } + return frame; + } else { + final int length = buffer.readableBytes(); + if (length > maxLength) { + discardedBytes = length; + buffer.readerIndex(buffer.writerIndex()); + discarding = true; + if (failFast) { + fail(ctx, "over " + discardedBytes); + } + } + return null; + } + } else { + if (eol >= 0) { + final int length = discardedBytes + eol - buffer.readerIndex(); + final int delimLength = buffer.getByte(eol) == '\r'? 2 : 1; + buffer.readerIndex(eol + delimLength); + discardedBytes = 0; + discarding = false; if (!failFast) { - fail(ctx, "over " + (maxLength + length) + " bytes"); + fail(ctx, length); } } else { - int delimLength; - final byte delim = buffer.getByte(buffer.readerIndex() + length); - if (delim == '\r') { - delimLength = 2; // Skip the \r\n. - } else { - delimLength = 1; - } - if (stripDelimiter) { - frame = extractFrame(buffer, buffer.readerIndex(), length); - } else { - frame = extractFrame(buffer, buffer.readerIndex(), length + delimLength); - } - buffer.skipBytes(length + delimLength); + discardedBytes = buffer.readableBytes(); + buffer.readerIndex(buffer.writerIndex()); } - return frame; + return null; } - - final int buffered = buffer.readableBytes(); - if (!discarding && buffered > maxLength) { - discarding = true; - if (failFast) { - fail(ctx, buffered + " bytes buffered already"); - } - } - if (discarding) { - buffer.skipBytes(buffer.readableBytes()); - } - return null; } - private void fail(final ChannelHandlerContext ctx, final String msg) { - Channels.fireExceptionCaught(ctx.getChannel(), - new TooLongFrameException("Frame length exceeds " + maxLength + " (" - + msg + ')')); + private void fail(final ChannelHandlerContext ctx, int length) { + fail(ctx, String.valueOf(length)); + } + + private void fail(final ChannelHandlerContext ctx, String length) { + Channels.fireExceptionCaught( + ctx.getChannel(), + new TooLongFrameException( + "frame length (" + length + ") exceeds the allowed maximum (" + maxLength + ')')); } /** @@ -136,5 +153,4 @@ public class LineBasedFrameDecoder extends FrameDecoder { } return -1; // Not found. } - } diff --git a/src/test/java/org/jboss/netty/handler/codec/frame/LineBasedFrameDecoderTest.java b/src/test/java/org/jboss/netty/handler/codec/frame/LineBasedFrameDecoderTest.java index 6e433e0505..09e3fe057b 100644 --- a/src/test/java/org/jboss/netty/handler/codec/frame/LineBasedFrameDecoderTest.java +++ b/src/test/java/org/jboss/netty/handler/codec/frame/LineBasedFrameDecoderTest.java @@ -16,36 +16,103 @@ package org.jboss.netty.handler.codec.frame; import org.jboss.netty.buffer.ChannelBuffer; -import org.jboss.netty.buffer.ChannelBuffers; +import org.jboss.netty.handler.codec.embedder.CodecEmbedderException; import org.jboss.netty.handler.codec.embedder.DecoderEmbedder; import org.jboss.netty.util.CharsetUtil; -import org.junit.Assert; import org.junit.Test; +import static org.hamcrest.CoreMatchers.*; +import static org.jboss.netty.buffer.ChannelBuffers.*; +import static org.junit.Assert.*; + public class LineBasedFrameDecoderTest { @Test public void testDecodeWithStrip() throws Exception { DecoderEmbedder embedder = new DecoderEmbedder( new LineBasedFrameDecoder(8192, true, false)); - Assert.assertTrue(embedder.offer(ChannelBuffers.copiedBuffer("first\r\nsecond\nthird", CharsetUtil.US_ASCII))); - Assert.assertTrue(embedder.finish()); - Assert.assertEquals("first", embedder.poll().toString(CharsetUtil.US_ASCII)); - Assert.assertEquals("second", embedder.poll().toString(CharsetUtil.US_ASCII)); - Assert.assertNull(embedder.poll()); + assertTrue(embedder.offer(copiedBuffer("first\r\nsecond\nthird", CharsetUtil.US_ASCII))); + assertTrue(embedder.finish()); + assertEquals("first", embedder.poll().toString(CharsetUtil.US_ASCII)); + assertEquals("second", embedder.poll().toString(CharsetUtil.US_ASCII)); + assertNull(embedder.poll()); } + @Test public void testDecodeWithoutStrip() throws Exception { DecoderEmbedder embedder = new DecoderEmbedder( new LineBasedFrameDecoder(8192, false, false)); - Assert.assertTrue(embedder.offer(ChannelBuffers.copiedBuffer("first\r\nsecond\nthird", CharsetUtil.US_ASCII))); - Assert.assertTrue(embedder.finish()); - Assert.assertEquals("first\r\n", embedder.poll().toString(CharsetUtil.US_ASCII)); - Assert.assertEquals("second\n", embedder.poll().toString(CharsetUtil.US_ASCII)); - Assert.assertNull(embedder.poll()); + assertTrue(embedder.offer(copiedBuffer("first\r\nsecond\nthird", CharsetUtil.US_ASCII))); + assertTrue(embedder.finish()); + assertEquals("first\r\n", embedder.poll().toString(CharsetUtil.US_ASCII)); + assertEquals("second\n", embedder.poll().toString(CharsetUtil.US_ASCII)); + assertNull(embedder.poll()); } + @Test + public void testTooLongLine1() throws Exception { + DecoderEmbedder embedder = new DecoderEmbedder( + new LineBasedFrameDecoder(16, false, false)); + + try { + embedder.offer(copiedBuffer("12345678901234567890\r\nfirst\nsecond", CharsetUtil.US_ASCII)); + fail(); + } catch (CodecEmbedderException e) { + assertThat(e.getCause(), is(instanceOf(TooLongFrameException.class))); + } + + embedder.offer(wrappedBuffer(new byte[1])); // A workaround that triggers decode() once again. + + assertThat(embedder.size(), is(1)); + assertTrue(embedder.finish()); + + assertThat(embedder.size(), is(1)); + assertThat(embedder.poll().toString(CharsetUtil.US_ASCII), is("first\n")); + } + + @Test + public void testTooLongLine2() throws Exception { + DecoderEmbedder embedder = new DecoderEmbedder( + new LineBasedFrameDecoder(16, false, false)); + + assertFalse(embedder.offer(copiedBuffer("12345678901234567", CharsetUtil.US_ASCII))); + try { + embedder.offer(copiedBuffer("890\r\nfirst\r\n", CharsetUtil.US_ASCII)); + fail(); + } catch (CodecEmbedderException e) { + assertThat(e.getCause(), is(instanceOf(TooLongFrameException.class))); + } + + embedder.offer(wrappedBuffer(new byte[1])); // A workaround that triggers decode() once again. + + assertThat(embedder.size(), is(1)); + assertTrue(embedder.finish()); + + assertThat(embedder.size(), is(1)); + assertEquals("first\r\n", embedder.poll().toString(CharsetUtil.US_ASCII)); + } + + @Test + public void testTooLongLineWithFailFast() throws Exception { + DecoderEmbedder embedder = new DecoderEmbedder( + new LineBasedFrameDecoder(16, false, true)); + + try { + embedder.offer(copiedBuffer("12345678901234567", CharsetUtil.US_ASCII)); + fail(); + } catch (CodecEmbedderException e) { + assertThat(e.getCause(), is(instanceOf(TooLongFrameException.class))); + } + + assertThat(embedder.offer(copiedBuffer("890", CharsetUtil.US_ASCII)), is(false)); + assertThat(embedder.offer(copiedBuffer("123\r\nfirst\r\n", CharsetUtil.US_ASCII)), is(true)); + assertThat(embedder.size(), is(1)); + assertTrue(embedder.finish()); + + assertThat(embedder.size(), is(1)); + assertEquals("first\r\n", embedder.poll().toString(CharsetUtil.US_ASCII)); + } }