From 8e462252970e316aae6344596281dd1af2c0ba35 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Mon, 11 Mar 2024 15:22:26 +0100
Subject: [PATCH] Only try to find index for QUIC packets with short header
 format

Motivation:

28ae7d2bedb1485e6da05997607e972f16b5a3ab added the QuicCodecDispatcher which will dispatch packets to the correct ChannelHandlerContext based on which id is encoded in destination connection id. Our implementation was not 100 % correct as it should only try to do the specific dispatching once a short header format was used.

Modifications:

- Only try to decode the index out of the connection id for QUIC packets with short header format
- Adjust testing

Result:

Correct dispatching based on destination connection id
---
 .../codec/quic/QuicCodecDispatcher.java       | 34 +++++--------------
 .../codec/quic/QuicCodecDispatcherTest.java   | 10 ++++--
 2 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicCodecDispatcher.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicCodecDispatcher.java
index 0a4a0cd4..1ae72136 100644
--- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicCodecDispatcher.java
+++ b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicCodecDispatcher.java
@@ -188,9 +188,10 @@ protected int decodeIndex(ByteBuf connectionId) {
     static ByteBuf getDestinationConnectionId(ByteBuf buffer, int localConnectionIdLength) {
         if (buffer.readableBytes() > Byte.BYTES) {
             int offset = buffer.readerIndex();
-            boolean shortHeader = (buffer.getByte(offset) & 0x80) == 0;
-
+            boolean shortHeader = hasShortHeader(buffer);
             offset += Byte.BYTES;
+            // We are only interested in packets with short header as these the packets that
+            // are exchanged after the server did provide the connection id that the client should use.
             if (shortHeader) {
                 // See https://www.rfc-editor.org/rfc/rfc9000.html#section-17.3
                 // 1-RTT Packet {
@@ -207,35 +208,16 @@ static ByteBuf getDestinationConnectionId(ByteBuf buffer, int localConnectionIdL
                 if (buffer.readableBytes() >= offset + localConnectionIdLength) {
                     return buffer.slice(offset, localConnectionIdLength);
                 }
-            } else {
-                // See https://www.rfc-editor.org/rfc/rfc9000.html#section-17.2:
-                // Long Header Packet {
-                //   Header Form (1) = 1,
-                //   Fixed Bit (1) = 1,
-                //   Long Packet Type (2),
-                //   Type-Specific Bits (4),
-                //   Version (32),
-                //   Destination Connection ID Length (8),
-                //   Destination Connection ID (0..160),
-                //   Source Connection ID Length (8),
-                //   Source Connection ID (0..160),
-                //  Type-Specific Payload (..),
-                // }
-
-                // skip version
-                offset += Integer.BYTES;
-                if (buffer.readableBytes() >= offset) {
-                    int len = buffer.getUnsignedByte(offset);
-                    offset += Byte.BYTES;
-                    if (buffer.readableBytes() >= offset + len) {
-                        return buffer.slice(offset, len);
-                    }
-                }
             }
         }
         return null;
     }
 
+    // Package-private for testing
+    static boolean hasShortHeader(ByteBuf buffer) {
+        return (buffer.getByte(buffer.readerIndex()) & 0x80) == 0;
+    }
+
     // Package-private for testing
     static int decodeIdx(ByteBuf connectionId) {
         if (connectionId.readableBytes() >= 2) {
diff --git a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicCodecDispatcherTest.java b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicCodecDispatcherTest.java
index 9dd11f34..8e54f789 100644
--- a/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicCodecDispatcherTest.java
+++ b/codec-native-quic/src/test/java/io/netty/incubator/codec/quic/QuicCodecDispatcherTest.java
@@ -30,6 +30,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 
 public class QuicCodecDispatcherTest {
 
@@ -67,9 +68,14 @@ protected void initChannel(Channel channel, int localConnectionIdLength,
                         break;
                     }
                 try {
+                    boolean hasShortHeader = QuicCodecDispatcher.hasShortHeader(packet.content());
                     ByteBuf id = QuicCodecDispatcher.getDestinationConnectionId(packet.content(), localConnectionIdLength);
-                    assertNotNull(id);
-                    assertEquals(idx, QuicCodecDispatcher.decodeIdx(id));
+                    if (hasShortHeader) {
+                        assertNotNull(id);
+                        assertEquals(idx, QuicCodecDispatcher.decodeIdx(id));
+                    } else {
+                        assertNull(id);
+                    }
                     numPackets--;
                 } finally {
                     packet.release();