Permalink
Browse files

ByteBufUtil.compare int underflow

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 #6169
  • Loading branch information...
1 parent 2368f23 commit 583a59abb1c1b2cd99aeac5cbbd2e58c4b087880 @Scottmitch Scottmitch committed Jan 3, 2017
@@ -246,7 +246,7 @@ public static int compare(ByteBuf bufferA, ByteBuf bufferB) {
}
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;
@@ -38,6 +38,7 @@
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.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
@@ -104,6 +107,24 @@ public void dispose() {
}
@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());
assertEquals(0, buffer.readerIndex());
@@ -1909,7 +1930,7 @@ public void testIndexOf() {
@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 void testNioBuffer1() {
@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 @@ private static void assertRemainingEquals(ByteBuffer expected, ByteBuffer actual
@Test
public void testToByteBuffer3() {
- Assume.assumeTrue(buffer.nioBufferCount() == 1);
+ assumeTrue(buffer.nioBufferCount() == 1);
assertEquals(buffer.order(), buffer.nioBuffer().order());
}

0 comments on commit 583a59a

Please sign in to comment.