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

RedisDecoder infinite loop

Browse Source 
Motivation:
RedisDecoder can get into an infinite loop while decoding bulk strings if the final \r and \n to indicate the end of content are split on ByteBuf boundaries.

Modifications:
- We should break out of the decode loop if remainingBulkLength is 0 and we don't have enough data to read EOL

Result:
No more infinite loop in RedisDecoder#decodeBulkStringContent.
This commit is contained in:
Scott Mitchell 2017-05-26 18:24:42 -07:00
parent 0b0309624a
commit 742ee76424
2 changed files with 20 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
codec-redis/src/main/java/io/netty/handler/codec/redis/RedisDecoder.java
View File

@ -201,7 +201,7 @@ public final class RedisDecoder extends ByteToMessageDecoder {
// ${expectedBulkLength}\r\n <here> {data...}\r\n // ${expectedBulkLength}\r\n <here> {data...}\r\n
private boolean decodeBulkStringContent(ByteBuf in, List<Object> out) throws Exception { private boolean decodeBulkStringContent(ByteBuf in, List<Object> out) throws Exception {
final int readableBytes = in.readableBytes(); final int readableBytes = in.readableBytes();
if (readableBytes == 0) { if (readableBytes == 0 || remainingBulkLength == 0 && readableBytes < RedisConstants.EOL_LENGTH) {
return false; return false;
} }

22
codec-redis/src/test/java/io/netty/handler/codec/redis/RedisDecoderTest.java
View File

@ -27,9 +27,15 @@ import org.junit.Test;
import java.util.List; import java.util.List;
import static io.netty.handler.codec.redis.RedisCodecTestUtil.*; import static io.netty.handler.codec.redis.RedisCodecTestUtil.byteBufOf;
import static org.hamcrest.CoreMatchers.*; import static io.netty.handler.codec.redis.RedisCodecTestUtil.bytesOf;
import static org.junit.Assert.*; import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
/** /**
* Verifies the correct functionality of the {@link RedisDecoder} and {@link RedisArrayAggregator}. * Verifies the correct functionality of the {@link RedisDecoder} and {@link RedisArrayAggregator}.
@ -51,6 +57,16 @@ public class RedisDecoderTest {
assertFalse(channel.finish()); assertFalse(channel.finish());
} }
@Test
public void splitEOLDoesNotInfiniteLoop() throws Exception {
assertFalse(channel.writeInbound(byteBufOf("$6\r\nfoobar\r")));
assertTrue(channel.writeInbound(byteBufOf("\n")));
RedisMessage msg = channel.readInbound();
assertTrue(msg instanceof FullBulkStringRedisMessage);
ReferenceCountUtil.release(msg);
}
@Test @Test
public void shouldDecodeSimpleString() { public void shouldDecodeSimpleString() {
assertFalse(channel.writeInbound(byteBufOf("+"))); assertFalse(channel.writeInbound(byteBufOf("+")));