andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Consistent fail-fast property

Browse Source 
* DelimiterBasedFrameDecoder and LengthFieldBasedFrameDecoder must
expose the fail-fast option consistently
* Renamed failImmediatelyOnTooLongFrame to failFast
This commit is contained in:
Trustin Lee 2011-11-22 18:37:36 +09:00
parent 1ac6c75d39
commit 0850449b09
4 changed files with 96 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
src/main/java/org/jboss/netty/handler/codec/frame/DelimiterBasedFrameDecoder.java
View File

@ -67,9 +67,9 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder {
private final ChannelBuffer[] delimiters; private final ChannelBuffer[] delimiters;
private final int maxFrameLength; private final int maxFrameLength;
private final boolean stripDelimiter; private final boolean stripDelimiter;
private final boolean failFast;
private boolean discardingTooLongFrame; private boolean discardingTooLongFrame;
private int tooLongFrameLength; private int tooLongFrameLength;
private final boolean failImmediatelyOnTooLongFrame;
/** /**
* Creates a new instance. * Creates a new instance.
@ -95,6 +95,29 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder {
*/ */
public DelimiterBasedFrameDecoder( public DelimiterBasedFrameDecoder(
int maxFrameLength, boolean stripDelimiter, ChannelBuffer delimiter) { 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 <tt>true</tt>, a {@link TooLongFrameException} is
* thrown as soon as the decoder notices the length of the
* frame will exceed <tt>maxFrameLength</tt> regardless of
* whether the entire frame has been read.
* If <tt>false</tt>, a {@link TooLongFrameException} is
* thrown after the entire frame that exceeds
* <tt>maxFrameLength</tt> has been read.
* @param delimiter the delimiter
*/
public DelimiterBasedFrameDecoder(
int maxFrameLength, boolean stripDelimiter, boolean failFast,
ChannelBuffer delimiter) {
validateMaxFrameLength(maxFrameLength); validateMaxFrameLength(maxFrameLength);
validateDelimiter(delimiter); validateDelimiter(delimiter);
delimiters = new ChannelBuffer[] { delimiters = new ChannelBuffer[] {
@ -103,7 +126,7 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder {
}; };
this.maxFrameLength = maxFrameLength; this.maxFrameLength = maxFrameLength;
this.stripDelimiter = stripDelimiter; 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 <code>false</code> * 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( public DelimiterBasedFrameDecoder(
int maxFrameLength, boolean stripDelimiter, ChannelBuffer... delimiters) { 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. * the length of the frame exceeds this value.
* @param stripDelimiter whether the decoded frame should strip out the * @param stripDelimiter whether the decoded frame should strip out the
* delimiter or not * delimiter or not
* @param failImmediatelyOnTooLongFrame If false (the default) a {@link TooLongFrameException} * @param failFast If <tt>true</tt>, a {@link TooLongFrameException} is
* is thrown if the length of the frame exceeds maxFrameLength, * thrown as soon as the decoder notices the length of the
* after the delimiter has been read. * frame will exceed <tt>maxFrameLength</tt> regardless of
* If true a {@link TooLongFrameException} is thrown immediately * whether the entire frame has been read.
* when the length of the frame exceeds maxFrameLength, * If <tt>false</tt>, a {@link TooLongFrameException} is
* regardless of whether a delimiter has been found yet. * thrown after the entire frame that exceeds
* <tt>maxFrameLength</tt> has been read.
* @param delimiters the delimiters * @param delimiters the delimiters
*/ */
public DelimiterBasedFrameDecoder( public DelimiterBasedFrameDecoder(
int maxFrameLength, boolean stripDelimiter, boolean failImmediatelyOnTooLongFrame, ChannelBuffer... delimiters) { int maxFrameLength, boolean stripDelimiter, boolean failFast, ChannelBuffer... delimiters) {
validateMaxFrameLength(maxFrameLength); validateMaxFrameLength(maxFrameLength);
if (delimiters == null) { if (delimiters == null) {
throw new NullPointerException("delimiters"); throw new NullPointerException("delimiters");
@ -159,7 +190,7 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder {
} }
this.maxFrameLength = maxFrameLength; this.maxFrameLength = maxFrameLength;
this.stripDelimiter = stripDelimiter; this.stripDelimiter = stripDelimiter;
this.failImmediatelyOnTooLongFrame = failImmediatelyOnTooLongFrame; this.failFast = failFast;
} }
@Override @Override
@ -188,7 +219,7 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder {
int tooLongFrameLength = this.tooLongFrameLength; int tooLongFrameLength = this.tooLongFrameLength;
this.tooLongFrameLength = 0; this.tooLongFrameLength = 0;
if (!failImmediatelyOnTooLongFrame) { if (!failFast) {
fail(ctx, tooLongFrameLength); fail(ctx, tooLongFrameLength);
} }
return null; return null;
@ -216,7 +247,7 @@ public class DelimiterBasedFrameDecoder extends FrameDecoder {
tooLongFrameLength = buffer.readableBytes(); tooLongFrameLength = buffer.readableBytes();
buffer.skipBytes(buffer.readableBytes()); buffer.skipBytes(buffer.readableBytes());
discardingTooLongFrame = true; discardingTooLongFrame = true;
if (failImmediatelyOnTooLongFrame) { if (failFast) {
fail(ctx, tooLongFrameLength); fail(ctx, tooLongFrameLength);
} }
} }

60
src/main/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoder.java
View File

@ -195,10 +195,10 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder {
private final int lengthFieldEndOffset; private final int lengthFieldEndOffset;
private final int lengthAdjustment; private final int lengthAdjustment;
private final int initialBytesToStrip; private final int initialBytesToStrip;
private final boolean failFast;
private boolean discardingTooLongFrame; private boolean discardingTooLongFrame;
private long tooLongFrameLength; private long tooLongFrameLength;
private long bytesToDiscard; private long bytesToDiscard;
private boolean failImmediatelyOnTooLongFrame = false;
/** /**
* Creates a new instance. * Creates a new instance.
@ -219,7 +219,6 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder {
this(maxFrameLength, lengthFieldOffset, lengthFieldLength, 0, 0); this(maxFrameLength, lengthFieldOffset, lengthFieldLength, 0, 0);
} }
/** /**
* Creates a new instance. * Creates a new instance.
* *
@ -240,6 +239,39 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder {
int maxFrameLength, int maxFrameLength,
int lengthFieldOffset, int lengthFieldLength, int lengthFieldOffset, int lengthFieldLength,
int lengthAdjustment, int initialBytesToStrip) { 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 <tt>true</tt>, a {@link TooLongFrameException} is thrown as
* soon as the decoder notices the length of the frame will exceed
* <tt>maxFrameLength</tt> regardless of whether the entire frame
* has been read. If <tt>false</tt>, a {@link TooLongFrameException}
* is thrown after the entire frame that exceeds <tt>maxFrameLength</tt>
* has been read.
*/
public LengthFieldBasedFrameDecoder(
int maxFrameLength,
int lengthFieldOffset, int lengthFieldLength,
int lengthAdjustment, int initialBytesToStrip, boolean failFast) {
if (maxFrameLength <= 0) { if (maxFrameLength <= 0) {
throw new IllegalArgumentException( throw new IllegalArgumentException(
"maxFrameLength must be a positive integer: " + "maxFrameLength must be a positive integer: " +
@ -280,6 +312,7 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder {
this.lengthAdjustment = lengthAdjustment; this.lengthAdjustment = lengthAdjustment;
lengthFieldEndOffset = lengthFieldOffset + lengthFieldLength; lengthFieldEndOffset = lengthFieldOffset + lengthFieldLength;
this.initialBytesToStrip = initialBytesToStrip; this.initialBytesToStrip = initialBytesToStrip;
this.failFast = failFast;
} }
@Override @Override
@ -376,14 +409,14 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder {
long tooLongFrameLength = this.tooLongFrameLength; long tooLongFrameLength = this.tooLongFrameLength;
this.tooLongFrameLength = 0; this.tooLongFrameLength = 0;
discardingTooLongFrame = false; discardingTooLongFrame = false;
if ((!failImmediatelyOnTooLongFrame) || if ((!failFast) ||
(failImmediatelyOnTooLongFrame && firstDetectionOfTooLongFrame)) (failFast && firstDetectionOfTooLongFrame))
{ {
fail(ctx, tooLongFrameLength); fail(ctx, tooLongFrameLength);
} }
} else { } else {
// Keep discarding and notify handlers if necessary. // Keep discarding and notify handlers if necessary.
if (failImmediatelyOnTooLongFrame && firstDetectionOfTooLongFrame) if (failFast && firstDetectionOfTooLongFrame)
{ {
fail(ctx, this.tooLongFrameLength); fail(ctx, this.tooLongFrameLength);
} }
@ -412,23 +445,6 @@ public class LengthFieldBasedFrameDecoder extends FrameDecoder {
return frame; 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) { private void fail(ChannelHandlerContext ctx, long frameLength) {
if (frameLength > 0) { if (frameLength > 0) {
Channels.fireExceptionCaught( Channels.fireExceptionCaught(

11
src/test/java/org/jboss/netty/handler/codec/frame/DelimiterBasedFrameDecoderTest.java
View File

@ -28,13 +28,14 @@ import org.junit.Test;
*/ */
public class DelimiterBasedFrameDecoderTest { public class DelimiterBasedFrameDecoderTest {
@Test @Test
public void testTooLongFrameRecovery() throws Exception { public void testFailSlowTooLongFrameRecovery() throws Exception {
DecoderEmbedder<ChannelBuffer> embedder = new DecoderEmbedder<ChannelBuffer>( DecoderEmbedder<ChannelBuffer> embedder = new DecoderEmbedder<ChannelBuffer>(
new DelimiterBasedFrameDecoder(1, Delimiters.nulDelimiter())); new DelimiterBasedFrameDecoder(1, true, false, Delimiters.nulDelimiter()));
for (int i = 0; i < 2; i ++) { for (int i = 0; i < 2; i ++) {
embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 1, 2 }));
try { 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."); Assert.fail(CodecEmbedderException.class.getSimpleName() + " must be raised.");
} catch (CodecEmbedderException e) { } catch (CodecEmbedderException e) {
Assert.assertTrue(e.getCause() instanceof TooLongFrameException); Assert.assertTrue(e.getCause() instanceof TooLongFrameException);
@ -48,9 +49,9 @@ public class DelimiterBasedFrameDecoderTest {
} }
@Test @Test
public void testFailImmediatelyTooLongFrameRecovery() throws Exception { public void testFailFastTooLongFrameRecovery() throws Exception {
DecoderEmbedder<ChannelBuffer> embedder = new DecoderEmbedder<ChannelBuffer>( DecoderEmbedder<ChannelBuffer> embedder = new DecoderEmbedder<ChannelBuffer>(
new DelimiterBasedFrameDecoder(1, true, true, Delimiters.nulDelimiter())); new DelimiterBasedFrameDecoder(1, Delimiters.nulDelimiter()));
for (int i = 0; i < 2; i ++) { for (int i = 0; i < 2; i ++) {
try { try {

12
src/test/java/org/jboss/netty/handler/codec/frame/LengthFieldBasedFrameDecoderTest.java
View File

@ -28,13 +28,14 @@ import org.junit.Test;
*/ */
public class LengthFieldBasedFrameDecoderTest { public class LengthFieldBasedFrameDecoderTest {
@Test @Test
public void testTooLongFrameRecovery() throws Exception { public void testFailSlowTooLongFrameRecovery() throws Exception {
DecoderEmbedder<ChannelBuffer> embedder = new DecoderEmbedder<ChannelBuffer>( DecoderEmbedder<ChannelBuffer> embedder = new DecoderEmbedder<ChannelBuffer>(
new LengthFieldBasedFrameDecoder(5, 0, 4, 0, 4)); new LengthFieldBasedFrameDecoder(5, 0, 4, 0, 4, false));
for (int i = 0; i < 2; i ++) { for (int i = 0; i < 2; i ++) {
embedder.offer(ChannelBuffers.wrappedBuffer(new byte[] { 0, 0, 0, 2 }));
try { 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."); Assert.fail(CodecEmbedderException.class.getSimpleName() + " must be raised.");
} catch (CodecEmbedderException e) { } catch (CodecEmbedderException e) {
Assert.assertTrue(e.getCause() instanceof TooLongFrameException); Assert.assertTrue(e.getCause() instanceof TooLongFrameException);
@ -48,10 +49,9 @@ public class LengthFieldBasedFrameDecoderTest {
} }
@Test @Test
public void testFailImmediatelyTooLongFrameRecovery() throws Exception { public void testFailFastTooLongFrameRecovery() throws Exception {
DecoderEmbedder<ChannelBuffer> embedder = new DecoderEmbedder<ChannelBuffer>( DecoderEmbedder<ChannelBuffer> embedder = new DecoderEmbedder<ChannelBuffer>(
new LengthFieldBasedFrameDecoder(5, 0, 4, 0, 4). new LengthFieldBasedFrameDecoder(5, 0, 4, 0, 4));
setFailImmediatelyOnTooLongFrame(true));
for (int i = 0; i < 2; i ++) { for (int i = 0; i < 2; i ++) {
try { try {