Repository: netty/netty
Issue: ByteBuf#compareTo() "violates its general contract"
Labels: defect

### Expected behavior

`ByteBuf` implements the Comparable interface but it appears to not work correctly.

### Actual behavior

The following exception occurs:

```java
Exception in thread "main" java.lang.IllegalArgumentException: Comparison method violates its general contract!
	at java.util.ComparableTimSort.mergeLo(ComparableTimSort.java:744)
	at java.util.ComparableTimSort.mergeAt(ComparableTimSort.java:481)
	at java.util.ComparableTimSort.mergeCollapse(ComparableTimSort.java:406)
	at java.util.ComparableTimSort.sort(ComparableTimSort.java:213)
	at java.util.Arrays.sort(Arrays.java:1312)
	at java.util.Arrays.sort(Arrays.java:1506)
	at java.util.ArrayList.sort(ArrayList.java:1454)
	at java.util.Collections.sort(Collections.java:141)
	at Main.main(Main.java:23)
```

### Steps to reproduce

See below.

### Minimal yet complete reproducer code (or URL to code)

```java
public class Main {

  public static void main(String[] args) {
    Random generator = new Random();
    
    List<ByteBuf> values = new ArrayList<>();
    
    for (int i = 0; i < 10000; i++) {
      byte[] value = new byte[20];
      generator.nextBytes(value);
      
      values.add(Unpooled.wrappedBuffer(value));
    }
    
    Collections.sort(values);
  }
}
```

### Netty version

4.1.7.Final-SNAPSHOT

### JVM version (e.g. `java -version`)

```
$ java -version
openjdk version "1.8.0_112"
OpenJDK Runtime Environment (build 1.8.0_112-b15)
OpenJDK 64-Bit Server VM (build 25.112-b15, mixed mode)
```

### OS version (e.g. `uname -a`)

Linux

Issue comments:
Thanks for reporting @rkapsi ... standby for a PR.

Pull request review comments:

Patch that resolved the issue:
diff --git a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
index 79403c61be1..1ddcab5ab0e 100644
--- a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
+++ b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
@@ -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;


Test patch:
diff --git a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
index 35613786b1f..720894df296 100644
--- a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
+++ b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
@@ -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
@@ -103,6 +106,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());
@@ -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());
     }