diff --git a/src/main/java/org/jboss/netty/handler/codec/frame/DelimiterBasedFrameDecoder.java b/src/main/java/org/jboss/netty/handler/codec/frame/DelimiterBasedFrameDecoder.java index c7ea5d3a8c..03ce644b61 100644 --- a/src/main/java/org/jboss/netty/handler/codec/frame/DelimiterBasedFrameDecoder.java +++ b/src/main/java/org/jboss/netty/handler/codec/frame/DelimiterBasedFrameDecoder.java @@ -67,9 +67,9 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { private final ChannelBuffer[] delimiters; private final int maxFrameLength; private final boolean stripDelimiter; + private final boolean failFast; private boolean discardingTooLongFrame; private int tooLongFrameLength; - private final boolean failImmediatelyOnTooLongFrame; /** * Creates a new instance. @@ -95,6 +95,29 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { */ public DelimiterBasedFrameDecoder( int maxFrameLength, boolean stripDelimiter, ChannelBuffer delimiter) { + this(maxFrameLength, stripDelimiter, true, delimiter); + } + + /** + * Creates a new instance. + * + * @param maxFrameLength the maximum length of the decoded frame. + * A {@link TooLongFrameException} is thrown if + * the length of the frame exceeds this value. + * @param stripDelimiter whether the decoded frame should strip out the + * delimiter or not + * @param failFast If true, a {@link TooLongFrameException} is + * thrown as soon as the decoder notices the length of the + * frame will exceed maxFrameLength regardless of + * whether the entire frame has been read. + * If false, a {@link TooLongFrameException} is + * thrown after the entire frame that exceeds + * maxFrameLength has been read. + * @param delimiter the delimiter + */ + public DelimiterBasedFrameDecoder( + int maxFrameLength, boolean stripDelimiter, boolean failFast, + ChannelBuffer delimiter) { validateMaxFrameLength(maxFrameLength); validateDelimiter(delimiter); delimiters = new ChannelBuffer[] { @@ -103,7 +126,7 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { }; this.maxFrameLength = maxFrameLength; this.stripDelimiter = stripDelimiter; - this.failImmediatelyOnTooLongFrame = false; + this.failFast = failFast; } /** @@ -119,11 +142,18 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { } /** - * Calls {@link #DelimiterBasedFrameDecoder(int, boolean, boolean, ChannelBuffer...)} with failImmediatelyOnTooLongFrame set to false + * Creates a new instance. + * + * @param maxFrameLength the maximum length of the decoded frame. + * A {@link TooLongFrameException} is thrown if + * the length of the frame exceeds this value. + * @param stripDelimiter whether the decoded frame should strip out the + * delimiter or not + * @param delimiters the delimiters */ public DelimiterBasedFrameDecoder( int maxFrameLength, boolean stripDelimiter, ChannelBuffer... delimiters) { - this(maxFrameLength, stripDelimiter, false, delimiters); + this(maxFrameLength, stripDelimiter, true, delimiters); } /** @@ -134,16 +164,17 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { * the length of the frame exceeds this value. * @param stripDelimiter whether the decoded frame should strip out the * delimiter or not - * @param failImmediatelyOnTooLongFrame If false (the default) a {@link TooLongFrameException} - * is thrown if the length of the frame exceeds maxFrameLength, - * after the delimiter has been read. - * If true a {@link TooLongFrameException} is thrown immediately - * when the length of the frame exceeds maxFrameLength, - * regardless of whether a delimiter has been found yet. + * @param failFast If true, a {@link TooLongFrameException} is + * thrown as soon as the decoder notices the length of the + * frame will exceed maxFrameLength regardless of + * whether the entire frame has been read. + * If false, a {@link TooLongFrameException} is + * thrown after the entire frame that exceeds + * maxFrameLength has been read. * @param delimiters the delimiters */ public DelimiterBasedFrameDecoder( - int maxFrameLength, boolean stripDelimiter, boolean failImmediatelyOnTooLongFrame, ChannelBuffer... delimiters) { + int maxFrameLength, boolean stripDelimiter, boolean failFast, ChannelBuffer... delimiters) { validateMaxFrameLength(maxFrameLength); if (delimiters == null) { throw new NullPointerException("delimiters"); @@ -159,9 +190,9 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { } this.maxFrameLength = maxFrameLength; this.stripDelimiter = stripDelimiter; - this.failImmediatelyOnTooLongFrame = failImmediatelyOnTooLongFrame; + this.failFast = failFast; } - + @Override protected Object decode( ChannelHandlerContext ctx, Channel channel, ChannelBuffer buffer) throws Exception { @@ -188,7 +219,7 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { int tooLongFrameLength = this.tooLongFrameLength; this.tooLongFrameLength = 0; - if (!failImmediatelyOnTooLongFrame) { + if (!failFast) { fail(ctx, tooLongFrameLength); } return null; @@ -216,7 +247,7 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { tooLongFrameLength = buffer.readableBytes(); buffer.skipBytes(buffer.readableBytes()); discardingTooLongFrame = true; - if (failImmediatelyOnTooLongFrame) { + if (failFast) { fail(ctx, tooLongFrameLength); } } diff --git a/src/main/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoder.java b/src/main/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoder.java index 99cb528837..4a6c3b6fca 100644 --- a/src/main/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoder.java +++ b/src/main/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoder.java @@ -195,10 +195,10 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder { private final int lengthFieldEndOffset; private final int lengthAdjustment; private final int initialBytesToStrip; + private final boolean failFast; private boolean discardingTooLongFrame; private long tooLongFrameLength; private long bytesToDiscard; - private boolean failImmediatelyOnTooLongFrame = false; /** * Creates a new instance. @@ -219,7 +219,6 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder { this(maxFrameLength, lengthFieldOffset, lengthFieldLength, 0, 0); } - /** * Creates a new instance. * @@ -240,6 +239,39 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder { int maxFrameLength, int lengthFieldOffset, int lengthFieldLength, int lengthAdjustment, int initialBytesToStrip) { + this( + maxFrameLength, + lengthFieldOffset, lengthFieldLength, lengthAdjustment, + initialBytesToStrip, true); + } + + /** + * Creates a new instance. + * + * @param maxFrameLength + * the maximum length of the frame. If the length of the frame is + * greater than this value, {@link TooLongFrameException} will be + * thrown. + * @param lengthFieldOffset + * the offset of the length field + * @param lengthFieldLength + * the length of the length field + * @param lengthAdjustment + * the compensation value to add to the value of the length field + * @param initialBytesToStrip + * the number of first bytes to strip out from the decoded frame + * @param failFast + * If true, a {@link TooLongFrameException} is thrown as + * soon as the decoder notices the length of the frame will exceed + * maxFrameLength regardless of whether the entire frame + * has been read. If false, a {@link TooLongFrameException} + * is thrown after the entire frame that exceeds maxFrameLength + * has been read. + */ + public LengthFieldBasedFrameDecoder( + int maxFrameLength, + int lengthFieldOffset, int lengthFieldLength, + int lengthAdjustment, int initialBytesToStrip, boolean failFast) { if (maxFrameLength <= 0) { throw new IllegalArgumentException( "maxFrameLength must be a positive integer: " + @@ -280,6 +312,7 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder { this.lengthAdjustment = lengthAdjustment; lengthFieldEndOffset = lengthFieldOffset + lengthFieldLength; this.initialBytesToStrip = initialBytesToStrip; + this.failFast = failFast; } @Override @@ -376,14 +409,14 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder { long tooLongFrameLength = this.tooLongFrameLength; this.tooLongFrameLength = 0; discardingTooLongFrame = false; - if ((!failImmediatelyOnTooLongFrame) || - (failImmediatelyOnTooLongFrame && firstDetectionOfTooLongFrame)) + if ((!failFast) || + (failFast && firstDetectionOfTooLongFrame)) { fail(ctx, tooLongFrameLength); } } else { // Keep discarding and notify handlers if necessary. - if (failImmediatelyOnTooLongFrame && firstDetectionOfTooLongFrame) + if (failFast && firstDetectionOfTooLongFrame) { fail(ctx, this.tooLongFrameLength); } @@ -412,23 +445,6 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder { return frame; } - /** - * Set the behavior when a frame longer than maxFrameLength is encountered. - * - * @param failImmediatelyOnTooLongFrame If false (the default) a {@link TooLongFrameException} - * is thrown if the length of the frame exceeds maxFrameLength, - * after the entire frame has been read. - * If true a {@link TooLongFrameException} is thrown immediately - * when the length of the frame exceeds maxFrameLength, - * regardless of whether the entire frame has been read. - */ - public LengthFieldBasedFrameDecoder setFailImmediatelyOnTooLongFrame( - boolean failImmediatelyOnTooLongFrame) - { - this.failImmediatelyOnTooLongFrame = failImmediatelyOnTooLongFrame; - return this; - } - private void fail(ChannelHandlerContext ctx, long frameLength) { if (frameLength > 0) { Channels.fireExceptionCaught( diff --git a/src/test/java/org/jboss/netty/handler/codec/frame/DelimiterBasedFrameDecoderTest.java b/src/test/java/org/jboss/netty/handler/codec/frame/DelimiterBasedFrameDecoderTest.java index 13c6506867..0d03407b61 100644 --- a/src/test/java/org/jboss/netty/handler/codec/frame/DelimiterBasedFrameDecoderTest.java +++ b/src/test/java/org/jboss/netty/handler/codec/frame/DelimiterBasedFrameDecoderTest.java @@ -28,13 +28,14 @@ import org.junit.Test; */ public class DelimiterBasedFrameDecoderTest { @Test - public void testTooLongFrameRecovery() throws Exception { + public void testFailSlowTooLongFrameRecovery() throws Exception { DecoderEmbedder embedder = new DecoderEmbedder( - new DelimiterBasedFrameDecoder(1, Delimiters.nulDelimiter())); + new DelimiterBasedFrameDecoder(1, true, false, Delimiters.nulDelimiter())); for (int i = 0; i < 2; i ++) { + embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 1, 2 })); try { - embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 1, 2, 0 })); + embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 0 })); Assert.fail(CodecEmbedderException.class.getSimpleName() + " must be raised."); } catch (CodecEmbedderException e) { Assert.assertTrue(e.getCause() instanceof TooLongFrameException); @@ -48,9 +49,9 @@ public class DelimiterBasedFrameDecoderTest { } @Test - public void testFailImmediatelyTooLongFrameRecovery() throws Exception { + public void testFailFastTooLongFrameRecovery() throws Exception { DecoderEmbedder embedder = new DecoderEmbedder( - new DelimiterBasedFrameDecoder(1, true, true, Delimiters.nulDelimiter())); + new DelimiterBasedFrameDecoder(1, Delimiters.nulDelimiter())); for (int i = 0; i < 2; i ++) { try { diff --git a/src/test/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoderTest.java b/src/test/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoderTest.java index b1402f3c8a..54236f5424 100644 --- a/src/test/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoderTest.java +++ b/src/test/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoderTest.java @@ -28,13 +28,14 @@ import org.junit.Test; */ public class LengthFieldBasedFrameDecoderTest { @Test - public void testTooLongFrameRecovery() throws Exception { + public void testFailSlowTooLongFrameRecovery() throws Exception { DecoderEmbedder embedder = new DecoderEmbedder( - new LengthFieldBasedFrameDecoder(5, 0, 4, 0, 4)); + new LengthFieldBasedFrameDecoder(5, 0, 4, 0, 4, false)); for (int i = 0; i < 2; i ++) { + embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 0, 0, 0, 2 })); try { - embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 0, 0, 0, 2, 0, 0 })); + embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 0, 0 })); Assert.fail(CodecEmbedderException.class.getSimpleName() + " must be raised."); } catch (CodecEmbedderException e) { Assert.assertTrue(e.getCause() instanceof TooLongFrameException); @@ -48,10 +49,9 @@ public class LengthFieldBasedFrameDecoderTest { } @Test - public void testFailImmediatelyTooLongFrameRecovery() throws Exception { + public void testFailFastTooLongFrameRecovery() throws Exception { DecoderEmbedder embedder = new DecoderEmbedder( - new LengthFieldBasedFrameDecoder(5, 0, 4, 0, 4). - setFailImmediatelyOnTooLongFrame(true)); + new LengthFieldBasedFrameDecoder(5, 0, 4, 0, 4)); for (int i = 0; i < 2; i ++) { try {