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 c45940b9c1..6f4dd99963 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 @@ -69,6 +69,7 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { private final boolean stripDelimiter; private boolean discardingTooLongFrame; private int tooLongFrameLength; + private boolean failImmediatelyOnTooLongFrame = false; /** * Creates a new instance. @@ -169,11 +170,11 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { discardingTooLongFrame = false; buffer.skipBytes(minFrameLength + minDelimLength); - // TODO Let user choose when the exception should be raised - early or late? - // If early, fail() should be called when discardingTooLongFrame is set to true. int tooLongFrameLength = this.tooLongFrameLength; this.tooLongFrameLength = 0; - fail(ctx, tooLongFrameLength); + if (!failImmediatelyOnTooLongFrame) { + fail(ctx, tooLongFrameLength); + } return null; } @@ -199,6 +200,9 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { tooLongFrameLength = buffer.readableBytes(); buffer.skipBytes(buffer.readableBytes()); discardingTooLongFrame = true; + if (failImmediatelyOnTooLongFrame) { + fail(ctx, tooLongFrameLength); + } } } else { // Still discarding the buffer since a delimiter is not found. @@ -209,6 +213,23 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder { } } + /** + * 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 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. + */ + public DelimiterBasedFrameDecoder setFailImmediatelyOnTooLongFrame( + boolean failImmediatelyOnTooLongFrame) + { + this.failImmediatelyOnTooLongFrame = failImmediatelyOnTooLongFrame; + return this; + } + private void fail(ChannelHandlerContext ctx, long frameLength) { if (frameLength > 0) { Channels.fireExceptionCaught( 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 292f24a68e..ec67571642 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 @@ -198,6 +198,7 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder { private boolean discardingTooLongFrame; private long tooLongFrameLength; private long bytesToDiscard; + private boolean failImmediatelyOnTooLongFrame = false; /** * Creates a new instance. @@ -290,7 +291,7 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder { buffer.skipBytes(localBytesToDiscard); bytesToDiscard -= localBytesToDiscard; this.bytesToDiscard = bytesToDiscard; - failIfNecessary(ctx); + failIfNecessary(ctx, false); return null; } @@ -340,7 +341,7 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder { tooLongFrameLength = frameLength; bytesToDiscard = frameLength - buffer.readableBytes(); buffer.skipBytes(buffer.readableBytes()); - failIfNecessary(ctx); + failIfNecessary(ctx, true); return null; } @@ -366,19 +367,25 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder { return frame; } - private void failIfNecessary(ChannelHandlerContext ctx) { + private void failIfNecessary(ChannelHandlerContext ctx, boolean firstDetectionOfTooLongFrame) { if (bytesToDiscard == 0) { // Reset to the initial state and tell the handlers that // the frame was too large. - // TODO Let user choose when the exception should be raised - early or late? - // If early, fail() should be called when discardingTooLongFrame is set to true. long tooLongFrameLength = this.tooLongFrameLength; this.tooLongFrameLength = 0; discardingTooLongFrame = false; - fail(ctx, tooLongFrameLength); + if (!failImmediatelyOnTooLongFrame) + { + fail(ctx, tooLongFrameLength); + } } else { // Keep discarding. } + + if (firstDetectionOfTooLongFrame && failImmediatelyOnTooLongFrame) + { + fail(ctx, tooLongFrameLength); + } } /** @@ -402,6 +409,23 @@ 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 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. + */ + 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 ccd1bfe463..6d67acb0e4 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 @@ -46,4 +46,25 @@ public class DelimiterBasedFrameDecoderTest { Assert.assertEquals("A", buf.toString(CharsetUtil.ISO_8859_1)); } } + + @Test + public void testFailImmediatelyTooLongFrameRecovery() throws Exception { + DecoderEmbedder embedder = new DecoderEmbedder( + new DelimiterBasedFrameDecoder(1, Delimiters.nulDelimiter()). + setFailImmediatelyOnTooLongFrame(true)); + + for (int i = 0; i < 2; i ++) { + try { + embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 1, 2 })); + Assert.fail(CodecEmbedderException.class.getSimpleName() + " must be raised."); + } catch (CodecEmbedderException e) { + Assert.assertTrue(e.getCause() instanceof TooLongFrameException); + // Expected + } + + embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 0, 'A', 0 })); + ChannelBuffer buf = embedder.poll(); + Assert.assertEquals("A", buf.toString(CharsetUtil.ISO_8859_1)); + } + } } 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 777c4dc501..b1402f3c8a 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 @@ -46,4 +46,25 @@ public class LengthFieldBasedFrameDecoderTest { Assert.assertEquals("A", buf.toString(CharsetUtil.ISO_8859_1)); } } + + @Test + public void testFailImmediatelyTooLongFrameRecovery() throws Exception { + DecoderEmbedder embedder = new DecoderEmbedder( + new LengthFieldBasedFrameDecoder(5, 0, 4, 0, 4). + setFailImmediatelyOnTooLongFrame(true)); + + for (int i = 0; i < 2; i ++) { + try { + embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 0, 0, 0, 2 })); + Assert.fail(CodecEmbedderException.class.getSimpleName() + " must be raised."); + } catch (CodecEmbedderException e) { + Assert.assertTrue(e.getCause() instanceof TooLongFrameException); + // Expected + } + + embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 0, 0, 0, 0, 0, 1, 'A' })); + ChannelBuffer buf = embedder.poll(); + Assert.assertEquals("A", buf.toString(CharsetUtil.ISO_8859_1)); + } + } }