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
This commit is contained in:
Trustin Lee 2011-08-19 11:05:24 +09:00
parent 83ad5fb07e
commit f4ebcf7aed
2 changed files with 66 additions and 11 deletions

View File

@ -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

View File

@ -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));
}
}
}