From f4ebcf7aed4ffff825beec67b47d3a8d99712936 Mon Sep 17 00:00:00 2001
From: Trustin Lee <trustin@gmail.com>
Date: Fri, 19 Aug 2011 11:05:24 +0900
Subject: [PATCH] NETTY-435 LengthFieldBasedFrameDecoder fails to recover from
 TooLongFrameException

* Fixed a bug where TooLongFrameException is not raised immediately when the large frame was fully decoded at the first attempt
* Fixed a bug where LengthFieldBasedFrameDecoder does not reset its state completely after raising TooLongFrameException
---
 .../frame/LengthFieldBasedFrameDecoder.java   | 28 ++++++-----
 .../LengthFieldBasedFrameDecoderTest.java     | 49 +++++++++++++++++++
 2 files changed, 66 insertions(+), 11 deletions(-)
 create mode 100644 src/test/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoderTest.java

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 363b44ceec..292f24a68e 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
@@ -290,17 +290,7 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder {
             buffer.skipBytes(localBytesToDiscard);
             bytesToDiscard -= localBytesToDiscard;
             this.bytesToDiscard = bytesToDiscard;
-            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;
-                fail(ctx, tooLongFrameLength);
-            } else {
-                // Keep discarding.
-            }
+            failIfNecessary(ctx);
             return null;
         }
 
@@ -350,6 +340,7 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder {
             tooLongFrameLength = frameLength;
             bytesToDiscard = frameLength - buffer.readableBytes();
             buffer.skipBytes(buffer.readableBytes());
+            failIfNecessary(ctx);
             return null;
         }
 
@@ -375,6 +366,21 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder {
         return frame;
     }
 
+    private void failIfNecessary(ChannelHandlerContext ctx) {
+        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);
+        } else {
+            // Keep discarding.
+        }
+    }
+
     /**
      * Extract the sub-region of the specified buffer. This method is called by
      * {@link #decode(ChannelHandlerContext, Channel, ChannelBuffer)} for each
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
new file mode 100644
index 0000000000..777c4dc501
--- /dev/null
+++ b/src/test/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoderTest.java
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2009 Red Hat, Inc.
+ *
+ * Red Hat 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 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;
+
+/**
+ * @author <a href="http://gleamynode.net/">Trustin Lee</a>
+ */
+public class LengthFieldBasedFrameDecoderTest {
+    @Test
+    public void testTooLongFrameRecovery() throws Exception {
+        DecoderEmbedder<ChannelBuffer> embedder = new DecoderEmbedder<ChannelBuffer>(
+                new LengthFieldBasedFrameDecoder(5, 0, 4, 0, 4));
+
+        for (int i = 0; i < 2; i ++) {
+            try {
+                embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 0, 0, 0, 2, 0, 0 }));
+                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, 1, 'A' }));
+            ChannelBuffer buf = embedder.poll();
+            Assert.assertEquals("A", buf.toString(CharsetUtil.ISO_8859_1));
+        }
+    }
+}