[#5718] Result of ByteBufUtil.compare(ByteBuf a, ByteBuf b) is dependent on ByteOrder of supplied ByteBufs

Motivation:

Result of ByteBufUtil.compare(ByteBuf a, ByteBuf b) is dependent on ByteOrder of supplied ByteBufs which should not be the case (as stated in the javadocs).

Modifications:

Ensure we get a consistent behavior when calling ByteBufUtil.compare(ByteBuf a, ByteBuf b) and not depend on ByteOrder.

Result:

ByteBufUtil.compare(ByteBuf a, ByteBuf b) and so AbstractByteBuf.compare(...) works correctly as stated in the javadocs.

buffer/src/main/java/io/netty/buffer/ByteBuf.java

@@ -2332,7 +2332,7 @@ public abstract class ByteBuf implements ReferenceCounted, Comparable<ByteBuf> {
 
     /**
      * Compares the content of the specified buffer to the content of this
-     * buffer.  Comparison is performed in the same manner with the string
+     * buffer. Comparison is performed in the same manner with the string
      * comparison functions of various languages such as {@code strcmp},
      * {@code memcmp} and {@link String#compareTo(String)}.
      */

buffer/src/main/java/io/netty/buffer/ByteBufUtil.java

@@ -229,54 +229,83 @@ public final class ByteBufUtil {
         final int minLength = Math.min(aLen, bLen);
         final int uintCount = minLength >>> 2;
         final int byteCount = minLength & 3;
-
         int aIndex = bufferA.readerIndex();
         int bIndex = bufferB.readerIndex();
 
-        if (bufferA.order() == bufferB.order()) {
+        if (uintCount > 0) {
-            for (int i = uintCount; i > 0; i --) {
+            boolean bufferAIsBigEndian = bufferA.order() == ByteOrder.BIG_ENDIAN;
-                long va = bufferA.getUnsignedInt(aIndex);
+            final long res;
-                long vb = bufferB.getUnsignedInt(bIndex);
+            int uintCountIncrement = uintCount << 2;
-                if (va > vb) {
+
-                    return 1;
+            if (bufferA.order() == bufferB.order()) {
-                }
+                res = bufferAIsBigEndian ? compareUintBigEndian(bufferA, bufferB, aIndex, bIndex, uintCountIncrement) :
-                if (va < vb) {
+                        compareUintLittleEndian(bufferA, bufferB, aIndex, bIndex, uintCountIncrement);
-                    return -1;
+            } else {
-                }
+                res = bufferAIsBigEndian ? compareUintBigEndianA(bufferA, bufferB, aIndex, bIndex, uintCountIncrement) :
-                aIndex += 4;
+                        compareUintBigEndianB(bufferA, bufferB, aIndex, bIndex, uintCountIncrement);
-                bIndex += 4;
             }
-        } else {
+            if (res != 0) {
-            for (int i = uintCount; i > 0; i --) {
+                // Ensure we not overflow when cast
-                long va = bufferA.getUnsignedInt(aIndex);
+                return (int) Math.min(Integer.MAX_VALUE, res);
-                long vb = swapInt(bufferB.getInt(bIndex)) & 0xFFFFFFFFL;
-                if (va > vb) {
-                    return 1;
-                }
-                if (va < vb) {
-                    return -1;
-                }
-                aIndex += 4;
-                bIndex += 4;
             }
+            aIndex += uintCountIncrement;
+            bIndex += uintCountIncrement;
         }
 
-        for (int i = byteCount; i > 0; i --) {
+        for (int aEnd = aIndex + byteCount; aIndex < aEnd; ++aIndex, ++bIndex) {
-            short va = bufferA.getUnsignedByte(aIndex);
+            int comp = bufferA.getUnsignedByte(aIndex) - bufferB.getUnsignedByte(bIndex);
-            short vb = bufferB.getUnsignedByte(bIndex);
+            if (comp != 0) {
-            if (va > vb) {
+                return comp;
-                return 1;
             }
-            if (va < vb) {
-                return -1;
-            }
-            aIndex ++;
-            bIndex ++;
         }
 
         return aLen - bLen;
     }
 
+    private static long compareUintBigEndian(
+            ByteBuf bufferA, ByteBuf bufferB, int aIndex, int bIndex, int uintCountIncrement) {
+        for (int aEnd = aIndex + uintCountIncrement; aIndex < aEnd; aIndex += 4, bIndex += 4) {
+            long comp = bufferA.getUnsignedInt(aIndex) - bufferB.getUnsignedInt(bIndex);
+            if (comp != 0) {
+                return comp;
+            }
+        }
+        return 0;
+    }
+
+    private static long compareUintLittleEndian(
+            ByteBuf bufferA, ByteBuf bufferB, int aIndex, int bIndex, int uintCountIncrement) {
+        for (int aEnd = aIndex + uintCountIncrement; aIndex < aEnd; aIndex += 4, bIndex += 4) {
+            long comp = bufferA.getUnsignedIntLE(aIndex) - bufferB.getUnsignedIntLE(bIndex);
+            if (comp != 0) {
+                return comp;
+            }
+        }
+        return 0;
+    }
+
+    private static long compareUintBigEndianA(
+            ByteBuf bufferA, ByteBuf bufferB, int aIndex, int bIndex, int uintCountIncrement) {
+        for (int aEnd = aIndex + uintCountIncrement; aIndex < aEnd; aIndex += 4, bIndex += 4) {
+            long comp =  bufferA.getUnsignedInt(aIndex) - bufferB.getUnsignedIntLE(bIndex);
+            if (comp != 0) {
+                return comp;
+            }
+        }
+        return 0;
+    }
+
+    private static long compareUintBigEndianB(
+            ByteBuf bufferA, ByteBuf bufferB, int aIndex, int bIndex, int uintCountIncrement) {
+        for (int aEnd = aIndex + uintCountIncrement; aIndex < aEnd; aIndex += 4, bIndex += 4) {
+            long comp =  bufferA.getUnsignedIntLE(aIndex) - bufferB.getUnsignedInt(bIndex);
+            if (comp != 0) {
+                return comp;
+            }
+        }
+        return 0;
+    }
+
     /**
      * The default implementation of {@link ByteBuf#indexOf(int, int, byte)}.
      * This method is useful when implementing a new buffer type.

buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java

@@ -31,6 +31,7 @@ import java.io.File;
 import java.io.IOException;
 import java.io.RandomAccessFile;
 import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
 import java.nio.ReadOnlyBufferException;
 import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
@@ -1822,6 +1823,28 @@ public abstract class AbstractByteBufTest {
                 buffer.retainedSlice(0, 31).compareTo(wrappedBuffer(value).order(LITTLE_ENDIAN)) < 0);
     }
 
+    @Test
+    public void testCompareTo2() {
+        byte[] bytes = {1, 2, 3, 4};
+        byte[] bytesReversed = {4, 3, 2, 1};
+
+        ByteBuf buf1 = newBuffer(4).clear().writeBytes(bytes).order(ByteOrder.LITTLE_ENDIAN);
+        ByteBuf buf2 = newBuffer(4).clear().writeBytes(bytesReversed).order(ByteOrder.LITTLE_ENDIAN);
+        ByteBuf buf3 = newBuffer(4).clear().writeBytes(bytes).order(ByteOrder.BIG_ENDIAN);
+        ByteBuf buf4 = newBuffer(4).clear().writeBytes(bytesReversed).order(ByteOrder.BIG_ENDIAN);
+        try {
+            assertEquals(buf1.compareTo(buf2), buf3.compareTo(buf4));
+            assertEquals(buf2.compareTo(buf1), buf4.compareTo(buf3));
+            assertEquals(buf1.compareTo(buf3), buf2.compareTo(buf4));
+            assertEquals(buf3.compareTo(buf1), buf4.compareTo(buf2));
+        } finally {
+            buf1.release();
+            buf2.release();
+            buf3.release();
+            buf4.release();
+        }
+    }
+
     @Test
     public void testToString() {
         buffer.clear();