Fix DefaultChannelId#asLongText NPE

Motivation: The DefaultChannelId class is serializable, but it does not account for variations in the length of machineId. Modifications: Adjusted code to accommodate varying lengths of MachineId. Result: Fixed NPE. Respects varying length of machineId.
netty · Apr 12, 2024 · 6b2e2e7 · 6b2e2e7
1 parent e19c91f
commit 6b2e2e7
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 9 deletions.
diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelId.java b/transport/src/main/java/io/netty/channel/DefaultChannelId.java
@@ -264,9 +264,10 @@ public String asLongText() {
     }
 
     private String newLongValue() {
-        StringBuilder buf = new StringBuilder(2 * data.length + 5);
+        final StringBuilder buf = new StringBuilder(2 * data.length + 5);
+        final int machineIdLen = data.length - PROCESS_ID_LEN - SEQUENCE_LEN - TIMESTAMP_LEN - RANDOM_LEN;
         int i = 0;
-        i = appendHexDumpField(buf, i, MACHINE_ID.length);
+        i = appendHexDumpField(buf, i, machineIdLen);
         i = appendHexDumpField(buf, i, PROCESS_ID_LEN);
         i = appendHexDumpField(buf, i, SEQUENCE_LEN);
         i = appendHexDumpField(buf, i, TIMESTAMP_LEN);
@@ -296,19 +297,37 @@ public int compareTo(final ChannelId o) {
         if (o instanceof DefaultChannelId) {
             // lexicographic comparison
             final byte[] otherData = ((DefaultChannelId) o).data;
-            int len1 = data.length;
-            int len2 = otherData.length;
-            int len = Math.min(len1, len2);
+            final int otherLen = otherData.length;
+            final byte[] data = this.data;
+            final int len = data.length;
+            if (len == otherLen) { // when machineIdLen is the same
+                for (int i = 0; i < len; ++i) {
+                    final byte x = data[i];
+                    final byte y = otherData[i];
+                    if (x != y) {
+                        return (x & 0xff) - (y & 0xff);
+                    }
+                }
+                return 0;
+            }
 
-            for (int k = 0; k < len; k++) {
-                byte x = data[k];
-                byte y = otherData[k];
+            // when machineIdLen is different
+            final int nonMachineIdLen = PROCESS_ID_LEN + SEQUENCE_LEN + TIMESTAMP_LEN + RANDOM_LEN;
+            final int machineIdLen = len - nonMachineIdLen;
+            final int otherMachineIdLen = otherLen - nonMachineIdLen;
+            final int minMachineIdLen = Math.min(machineIdLen, otherMachineIdLen);
+            assert machineIdLen != otherMachineIdLen;
+
+            // compare machineId bytes
+            for (int i = 0; i < minMachineIdLen; i++) {
+                byte x = data[i];
+                byte y = otherData[i];
                 if (x != y) {
                     // treat these as unsigned bytes for comparison
                     return (x & 0xff) - (y & 0xff);
                 }
             }
-            return len1 - len2;
+            return machineIdLen - otherMachineIdLen;
         }
 
         return asLongText().compareTo(o.asLongText());

diff --git a/transport/src/test/java/io/netty/channel/DefaultChannelIdTest.java b/transport/src/test/java/io/netty/channel/DefaultChannelIdTest.java
@@ -20,10 +20,14 @@
 import io.netty.buffer.ByteBufInputStream;
 import io.netty.buffer.ByteBufOutputStream;
 import io.netty.buffer.Unpooled;
+import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import java.io.ByteArrayInputStream;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
+import java.util.Arrays;
+import java.util.Comparator;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
@@ -33,6 +37,26 @@
 
 @SuppressWarnings("DynamicRegexReplaceableByCompiledPattern")
 public class DefaultChannelIdTest {
+
+    private static final byte[] EIGHT_BYTES_MACHINE_ID = new byte[] { // machienId: 01-23-45-67-89-ab-cd-ef
+            -84, -19, 0, 5, 115, 114, 0, 33, 105, 111, 46, 110, 101, 116, 116, 121, 46, 99, 104, 97, 110, 110, 101, 108,
+            46, 68, 101, 102, 97, 117, 108, 116, 67, 104, 97, 110, 110, 101, 108, 73, 100, 53, -25, 2, -75, -50, 80,
+            -75, 79, 2, 0, 2, 73, 0, 8, 104, 97, 115, 104, 67, 111, 100, 101, 91, 0, 4, 100, 97, 116, 97, 116, 0, 2, 91,
+            66, 120, 112, -81, 121, -24, -9, 117, 114, 0, 2, 91, 66, -84, -13, 23, -8, 6, 8, 84, -32, 2, 0, 0, 120, 112,
+            0, 0, 0, 28, 1, 35, 69, 103, -119, -85, -51, -17, 0, 0, 82, -81, 0, 0, 0, 0, 6, 80, 79, 99, -114, -76, -61,
+            -122, -39, 100, -33, 94
+
+    };
+
+    private static final byte[] SIX_BYTES_MACHINE_ID = new byte[] { // machineId: 01-23-45-67-89-ab
+            -84, -19, 0, 5, 115, 114, 0, 33, 105, 111, 46, 110, 101, 116, 116, 121, 46, 99, 104, 97, 110, 110, 101, 108,
+            46, 68, 101, 102, 97, 117, 108, 116, 67, 104, 97, 110, 110, 101, 108, 73, 100, 53, -25, 2, -75, -50, 80,
+            -75, 79, 2, 0, 2, 73, 0, 8, 104, 97, 115, 104, 67, 111, 100, 101, 91, 0, 4, 100, 97, 116, 97, 116, 0, 2, 91,
+            66, 120, 112, 69, 123, -15, -54, 117, 114, 0, 2, 91, 66, -84, -13, 23, -8, 6, 8, 84, -32, 2, 0, 0, 120, 112,
+            0, 0, 0, 26, 1, 35, 69, 103, -119, -85, -50, 0, 82, -125, 0, 0, 0, 0, 6, -98, 109, -50, -98, -76, 81, 111,
+            114, 23, 87, -73
+    };
+
     @Test
     public void testShortText() {
         String text = DefaultChannelId.newInstance().asShortText();
@@ -84,4 +108,28 @@ public void testSerialization() throws Exception {
         assertThat(a, is(not(sameInstance(b))));
         assertThat(a.asLongText(), is(b.asLongText()));
     }
+
+    @Test
+    public void testDeserialization() throws Exception {
+        // deseiralize DefaultChannelId with 8 byte machineId
+        final ObjectInputStream in8 = new ObjectInputStream(new ByteArrayInputStream(EIGHT_BYTES_MACHINE_ID));
+        final DefaultChannelId c8 = (DefaultChannelId) in8.readObject();
+
+        // deserialize DefaultChannelId with 6 byte machineId
+        final ObjectInputStream in6 = new ObjectInputStream(new ByteArrayInputStream(SIX_BYTES_MACHINE_ID));
+        final DefaultChannelId c6 = (DefaultChannelId) in6.readObject();
+
+        assertThat(c8.asLongText(), is("0123456789abcdef-000052af-00000000-06504f638eb4c386-d964df5e"));
+        assertThat(c6.asLongText(), is("0123456789ab-ce005283-00000000-069e6dce9eb4516f-721757b7"));
+
+        final ChannelId[] channelIds = {c8, c6};
+        Arrays.sort(channelIds);
+        Assertions.assertThat(channelIds).isSortedAccordingTo(new Comparator<ChannelId>() {
+            @Override
+            public int compare(ChannelId o1, ChannelId o2) {
+                return o1.asLongText().compareTo(o2.asLongText());
+            }
+        });
+
+    }
 }