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

ByteBufUtil.compare int underflow

Browse Source 
Motivation:
ByteBufUtil.compare uses long arithmetic but doesn't check for underflow on when converting from long to int to satisfy the Comparable interface. This will result in incorrect comparisons and violate the Comparable interface contract.

Modifications:
- ByteBufUtil.compare should protect against int underflow

Result:
Fixes https://github.com/netty/netty/issues/6169
This commit is contained in:
Scott Mitchell 2017-01-03 09:51:00 -08:00
parent 2368f238ad
commit 583a59abb1
2 changed files with 25 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
View File

@ -246,7 +246,7 @@ public final class ByteBufUtil {
}
if (res != 0) {
// Ensure we not overflow when cast
return (int) Math.min(Integer.MAX_VALUE, res);
return (int) Math.min(Integer.MAX_VALUE, Math.max(Integer.MIN_VALUE, res));
}
aIndex += uintCountIncrement;
bIndex += uintCountIncrement;

27
buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
View File

@ -38,6 +38,7 @@ import java.nio.channels.GatheringByteChannel;
import java.nio.channels.ScatteringByteChannel;
import java.nio.channels.WritableByteChannel;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Random;
import java.util.Set;
@ -62,6 +63,8 @@ import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;
/**
* An abstract test class for channel buffers
@ -103,6 +106,24 @@ public abstract class AbstractByteBufTest {
}
}
@Test
public void comparableInterfaceNotViolated() {
assumeFalse(buffer.isReadOnly());
buffer.writerIndex(buffer.readerIndex());
assumeTrue(buffer.writableBytes() >= 4);
buffer.writeLong(0);
ByteBuf buffer2 = newBuffer(CAPACITY);
assumeFalse(buffer2.isReadOnly());
buffer2.writerIndex(buffer2.readerIndex());
// Write an unsigned integer that will cause buffer.getUnsignedInt() - buffer2.getUnsignedInt() to underflow the
// int type and wrap around on the negative side.
buffer2.writeLong(0xF0000000L);
assertTrue(buffer.compareTo(buffer2) < 0);
assertTrue(buffer2.compareTo(buffer) > 0);
buffer2.release();
}
@Test
public void initialState() {
assertEquals(CAPACITY, buffer.capacity());
@ -1909,7 +1930,7 @@ public abstract class AbstractByteBufTest {
@Test
public void testNioBuffer1() {
Assume.assumeTrue(buffer.nioBufferCount() == 1);
assumeTrue(buffer.nioBufferCount() == 1);
byte[] value = new byte[buffer.capacity()];
random.nextBytes(value);
@ -1921,7 +1942,7 @@ public abstract class AbstractByteBufTest {
@Test
public void testToByteBuffer2() {
Assume.assumeTrue(buffer.nioBufferCount() == 1);
assumeTrue(buffer.nioBufferCount() == 1);
byte[] value = new byte[buffer.capacity()];
random.nextBytes(value);
@ -1947,7 +1968,7 @@ public abstract class AbstractByteBufTest {
@Test
public void testToByteBuffer3() {
Assume.assumeTrue(buffer.nioBufferCount() == 1);
assumeTrue(buffer.nioBufferCount() == 1);
assertEquals(buffer.order(), buffer.nioBuffer().order());
}