Repository: netty/netty
Issue: ByteBuf.order() seems to have wrong behavior

Found unexpected behavior when calling 
```java
Unpooled.buffer(1024).order(ByteOrder.LITTLE_ENDIAN).writeShortLE(message)
```
since netty 4.1.54.Final.

The `order()` method has already been deprecated, but nevertheless it shouldn't be broken. 

### Expected behavior

Test passes

### Actual behavior

Test failed 
```
expected: -12208
but was : 20688
	at NettyEndianTest.endianTestCallingOrder(NettyEndianTest.java:26)
```

### Steps to reproduce

Create the following project

*./build.gradle*
```gradle
plugins {
    id 'java'
}

group 'org.example'
version '1.0-SNAPSHOT'

repositories {
    mavenCentral()
}

dependencies {
    testImplementation 'com.google.truth:truth:1.0.1'
    testImplementation 'io.netty:netty-buffer:4.1.68.Final'
    testImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.0'
    testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.7.0'
}

test {
    useJUnitPlatform()
}
```

*./src/NettyEndianTest.java*
```java
import static com.google.common.truth.Truth.assertThat;

import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import java.nio.ByteOrder;
import org.junit.jupiter.api.Test;

class NettyEndianTest {

  // Test will fail for netty 4.1.54.Final and above 
  @SuppressWarnings("deprecation")
  @Test
  void endianTestCallingOrder() {
    short message = (short) 0xD050;
    short reverse = (short) 0x50D0;
    ByteBuf b = Unpooled.buffer(1024).order(ByteOrder.LITTLE_ENDIAN).writeShortLE(message);

    assertThat(b.getShortLE(0)).isEqualTo(message);
    assertThat(b.getShort(0)).isEqualTo(reverse);
  }

  // Test is passing
  @Test
  void endianTestWithoutCallingOrder() {
    short message = (short) 0xD050;
    short reverse = (short) 0x50D0;
    ByteBuf b = Unpooled.buffer(1024).writeShortLE(message);

    assertThat(b.getShortLE(0)).isEqualTo(message);
    assertThat(b.getShort(0)).isEqualTo(reverse);
  }
}
```

Run *gradle test*


### Minimal yet complete reproducer code (or URL to code)

### Netty version
4.1.54.Final - 4.1.68.Final

Earlier versions before 4.1.54 were good.

### JVM version (e.g. `java -version`)

openjdk version "11.0.11" 2021-04-20 LTS
OpenJDK Runtime Environment Zulu11.48+21-CA (build 11.0.11+9-LTS)
OpenJDK 64-Bit Server VM Zulu11.48+21-CA (build 11.0.11+9-LTS, mixed mode)

Other JVMs the same.

### OS version (e.g. `uname -a`)
Darwin Kernel Version 20.6.0
Other platforms the same.

Issue comments:
Actually your this test has bug in line 26, 27.
> Actually your this test has bug in line 26, 27.

@forchid  The test where line 26 and 27 are is passing. What bug does it have? The issue is that the other test is failing.
> The test where line 26 and 27 are is passing. What bug does it have? The issue is that the other test is failing.

In the method endianTestCallingOrder(), the byte order is LITTLE_ENDIAN, so these asserts should be:
```java
assertThat(b.getShort(0)).isEqualTo(message);
assertThat(b.getShortLE(0)).isEqualTo(reverse);
```
# The detail of these steps:
## Step-1 writeShortLE()
```java
ByteBuf b = Unpooled.buffer(1024).order(ByteOrder.LITTLE_ENDIAN).writeShortLE(message)
```
-> Storage arrange: 0x50,0xD0

## Step-2 getShort() in LITTLE_ENDIAN ByteBuf
-> Result: 0x50 | (0xD0 >> 8) for the java short byte order BIG_ENDIAN!

## Step-3 getShortLe() in LITTLE_ENDIAN ByteBuf
-> Result: (0x50 >> 8) | 0xD0

> > The test where line 26 and 27 are is passing. What bug does it have? The issue is that the other test is failing.

> In the method endianTestCallingOrder(), the byte order is LITTLE_ENDIAN, so these asserts should be:
>  ```
>  assertThat(b.getShort(0)).isEqualTo(message);
>  assertThat(b.getShortLE(0)).isEqualTo(reverse);
>  ```

Line 26 and 27 are not in method `endianTestCallingOrder()`. Is there still a bug in line 26 and 27 in method `endianTestWithoutCallingOrder()`?

The method `endianTestCallingOrder()` was passing in Netty 4.1.53.Final, does that mean there was a bug in Netty 4.1.53.Final? (Is it related to #10747 ?)
Yes, this bug was existing before Netty 4.1.54.Final. Actually getShort() in java is getShortBE() for instruction set BIG_ENDIAN in jvm specification.
The `*LE` methods have different behaviour depending on the concrete buffer implementation and how it's put together with wrappers and so on.
@chrisvest, "different behavior depending on the implementation" sounds like a bug or a useless API. It seems especially bad with the methods being encouraged over `order()`. We had assumed "maybe it is a bug with order()", but if the API is "a box of chocolates and you never know what you are going to get" then that is a more severe issue.

But even if there is some variation in implementation, it seems particularly surprising that `writeShortLE` and `getShortLE` disagree with each other; I don't know how to interpret that other than there's a bug.
Oh, I didn't catch that those two were disagreeing, but either way, yeah I think it's a bug.
Looks like the entirety of `SwappedByteBuf` needs an audit. But it looks like it has always been broken https://github.com/netty/netty/commit/0f9492c9affc528c766f9677952412564d4a3f6d . It seems what changed is https://github.com/netty/netty/commit/96da45de2d53d74ad3a77e83c840410eb3f847c7 fixed many of the methods, but also missed many of them. Before the methods were wrong, but in agreement. Now some methods are just a bit more obviously broken.

https://github.com/netty/netty/blob/23405e2000427e9cad104913e7071d8d08e91a3c/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java#L247-L250

And getMediumLE, getIntLE, and getLongLE look busted. And then I see some other busted methods like:

https://github.com/netty/netty/blob/23405e2000427e9cad104913e7071d8d08e91a3c/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java#L402-L406
Opened #11706 to fix this.

@dapengzhang0 Note the tests I added to `UnpooledTest` are different from the ones you posted above. Specifically, the test you said was failing is now making the _opposite_ assertion, namely that `LE` methods always perform little-endian accesses.

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 7f7a4902cc3..68a0ab5aaa0 100644
--- a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
+++ b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
@@ -474,7 +474,7 @@ private static long compareUintBigEndian(
     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);
+            long comp = uintFromLE(bufferA.getUnsignedIntLE(aIndex)) - uintFromLE(bufferB.getUnsignedIntLE(bIndex));
             if (comp != 0) {
                 return comp;
             }
@@ -485,7 +485,9 @@ private static long compareUintLittleEndian(
     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);
+            long a = bufferA.getUnsignedInt(aIndex);
+            long b = uintFromLE(bufferB.getUnsignedIntLE(bIndex));
+            long comp =  a - b;
             if (comp != 0) {
                 return comp;
             }
@@ -496,7 +498,9 @@ private static long compareUintBigEndianA(
     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);
+            long a = uintFromLE(bufferA.getUnsignedIntLE(aIndex));
+            long b = bufferB.getUnsignedInt(bIndex);
+            long comp =  a - b;
             if (comp != 0) {
                 return comp;
             }
@@ -504,6 +508,10 @@ private static long compareUintBigEndianB(
         return 0;
     }
 
+    private static long uintFromLE(long value) {
+        return Long.reverseBytes(value) >>> Integer.SIZE;
+    }
+
     private static final class SWARByteSearch {
 
         private static long compilePattern(byte byteToFind) {
diff --git a/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java b/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java
index 7e038a6e87b..3b1b4d53854 100644
--- a/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java
@@ -246,7 +246,7 @@ public short getShort(int index) {
 
     @Override
     public short getShortLE(int index) {
-        return buf.getShort(index);
+        return buf.getShortLE(index);
     }
 
     @Override
@@ -266,7 +266,7 @@ public int getMedium(int index) {
 
     @Override
     public int getMediumLE(int index) {
-        return buf.getMedium(index);
+        return buf.getMediumLE(index);
     }
 
     @Override
@@ -286,7 +286,7 @@ public int getInt(int index) {
 
     @Override
     public int getIntLE(int index) {
-        return buf.getInt(index);
+        return buf.getIntLE(index);
     }
 
     @Override
@@ -306,7 +306,7 @@ public long getLong(int index) {
 
     @Override
     public long getLongLE(int index) {
-        return buf.getLong(index);
+        return buf.getLongLE(index);
     }
 
     @Override
@@ -401,7 +401,7 @@ public ByteBuf setShort(int index, int value) {
 
     @Override
     public ByteBuf setShortLE(int index, int value) {
-        buf.setShort(index, (short) value);
+        buf.setShortLE(index, (short) value);
         return this;
     }
 
@@ -413,7 +413,7 @@ public ByteBuf setMedium(int index, int value) {
 
     @Override
     public ByteBuf setMediumLE(int index, int value) {
-        buf.setMedium(index, value);
+        buf.setMediumLE(index, value);
         return this;
     }
 
@@ -425,7 +425,7 @@ public ByteBuf setInt(int index, int value) {
 
     @Override
     public ByteBuf setIntLE(int index, int value) {
-        buf.setInt(index, value);
+        buf.setIntLE(index, value);
         return this;
     }
 
@@ -437,7 +437,7 @@ public ByteBuf setLong(int index, long value) {
 
     @Override
     public ByteBuf setLongLE(int index, long value) {
-        buf.setLong(index, value);
+        buf.setLongLE(index, value);
         return this;
     }
 
diff --git a/handler/src/main/java/io/netty/handler/ssl/SslUtils.java b/handler/src/main/java/io/netty/handler/ssl/SslUtils.java
index 49665002bad..c9d06ed4386 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SslUtils.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SslUtils.java
@@ -327,15 +327,21 @@ static int getEncryptedPacketLength(ByteBuf buffer, int offset) {
     // Reads a big-endian unsigned short integer from the buffer
     @SuppressWarnings("deprecation")
     private static int unsignedShortBE(ByteBuf buffer, int offset) {
-        return buffer.order() == ByteOrder.BIG_ENDIAN ?
-                buffer.getUnsignedShort(offset) : buffer.getUnsignedShortLE(offset);
+        int value = buffer.getUnsignedShort(offset);
+        if (buffer.order() == ByteOrder.LITTLE_ENDIAN) {
+            value = Integer.reverseBytes(value) >>> Short.SIZE;
+        }
+        return value;
     }
 
     // Reads a big-endian short integer from the buffer
     @SuppressWarnings("deprecation")
     private static short shortBE(ByteBuf buffer, int offset) {
-        return buffer.order() == ByteOrder.BIG_ENDIAN ?
-                buffer.getShort(offset) : buffer.getShortLE(offset);
+        short value = buffer.getShort(offset);
+        if (buffer.order() == ByteOrder.LITTLE_ENDIAN) {
+            value = Short.reverseBytes(value);
+        }
+        return value;
     }
 
     private static short unsignedByte(byte b) {


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 c61dd334d16..6a2c98fbdf6 100644
--- a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
+++ b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
@@ -5943,6 +5943,16 @@ public void explicitLittleEndianReadMethodsMustAlwaysUseLittleEndianByteOrder()
         buffer.readerIndex(0);
         assertEquals(0x0807060504030201L, Double.doubleToRawLongBits(buffer.readDoubleLE()));
         buffer.readerIndex(0);
+
+        assertEquals(0x0201, buffer.getShortLE(0));
+        assertEquals(0x0201, buffer.getUnsignedShortLE(0));
+        assertEquals(0x030201, buffer.getMediumLE(0));
+        assertEquals(0x030201, buffer.getUnsignedMediumLE(0));
+        assertEquals(0x04030201, buffer.getIntLE(0));
+        assertEquals(0x04030201, buffer.getUnsignedIntLE(0));
+        assertEquals(0x04030201, Float.floatToRawIntBits(buffer.getFloatLE(0)));
+        assertEquals(0x0807060504030201L, buffer.getLongLE(0));
+        assertEquals(0x0807060504030201L, Double.doubleToRawLongBits(buffer.getDoubleLE(0)));
     }
 
     @Test
@@ -5965,5 +5975,18 @@ public void explicitLittleEndianWriteMethodsMustAlwaysUseLittleEndianByteOrder()
         buffer.clear();
         buffer.writeDoubleLE(Double.longBitsToDouble(0x0102030405060708L));
         assertEquals(0x0102030405060708L, Double.doubleToRawLongBits(buffer.readDoubleLE()));
+
+        buffer.setShortLE(0, 0x0102);
+        assertEquals(0x0102, buffer.getShortLE(0));
+        buffer.setMediumLE(0, 0x010203);
+        assertEquals(0x010203, buffer.getMediumLE(0));
+        buffer.setIntLE(0, 0x01020304);
+        assertEquals(0x01020304, buffer.getIntLE(0));
+        buffer.setFloatLE(0, Float.intBitsToFloat(0x01020304));
+        assertEquals(0x01020304, Float.floatToRawIntBits(buffer.getFloatLE(0)));
+        buffer.setLongLE(0, 0x0102030405060708L);
+        assertEquals(0x0102030405060708L, buffer.getLongLE(0));
+        buffer.setDoubleLE(0, Double.longBitsToDouble(0x0102030405060708L));
+        assertEquals(0x0102030405060708L, Double.doubleToRawLongBits(buffer.getDoubleLE(0)));
     }
 }
diff --git a/buffer/src/test/java/io/netty/buffer/UnpooledTest.java b/buffer/src/test/java/io/netty/buffer/UnpooledTest.java
index 0d1fc2131a3..efc1dafd1ed 100644
--- a/buffer/src/test/java/io/netty/buffer/UnpooledTest.java
+++ b/buffer/src/test/java/io/netty/buffer/UnpooledTest.java
@@ -22,6 +22,7 @@
 
 import java.io.InputStream;
 import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
 import java.nio.channels.ScatteringByteChannel;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
@@ -768,4 +769,53 @@ public void execute() throws Throwable {
             wrappedBuffer.release();
         }
     }
+
+    @SuppressWarnings("deprecation")
+    @Test
+    public void littleEndianWriteOnLittleEndianBufferMustStoreLittleEndianValue() {
+        ByteBuf b = buffer(1024).order(ByteOrder.LITTLE_ENDIAN);
+
+        b.writeShortLE(0x0102);
+        assertEquals((short) 0x0102, b.getShortLE(0));
+        assertEquals((short) 0x0102, b.getShort(0));
+        b.clear();
+
+        b.writeMediumLE(0x010203);
+        assertEquals(0x010203, b.getMediumLE(0));
+        assertEquals(0x010203, b.getMedium(0));
+        b.clear();
+
+        b.writeIntLE(0x01020304);
+        assertEquals(0x01020304, b.getIntLE(0));
+        assertEquals(0x01020304, b.getInt(0));
+        b.clear();
+
+        b.writeLongLE(0x0102030405060708L);
+        assertEquals(0x0102030405060708L, b.getLongLE(0));
+        assertEquals(0x0102030405060708L, b.getLong(0));
+    }
+
+    @Test
+    public void littleEndianWriteOnDefaultBufferMustStoreLittleEndianValue() {
+        ByteBuf b = buffer(1024);
+
+        b.writeShortLE(0x0102);
+        assertEquals((short) 0x0102, b.getShortLE(0));
+        assertEquals((short) 0x0201, b.getShort(0));
+        b.clear();
+
+        b.writeMediumLE(0x010203);
+        assertEquals(0x010203, b.getMediumLE(0));
+        assertEquals(0x030201, b.getMedium(0));
+        b.clear();
+
+        b.writeIntLE(0x01020304);
+        assertEquals(0x01020304, b.getIntLE(0));
+        assertEquals(0x04030201, b.getInt(0));
+        b.clear();
+
+        b.writeLongLE(0x0102030405060708L);
+        assertEquals(0x0102030405060708L, b.getLongLE(0));
+        assertEquals(0x0807060504030201L, b.getLong(0));
+    }
 }