Only try to find index for QUIC packets with short header format (#692)

Motivation:

28ae7d2 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
- Ensure we always correctly propagate channelReadComplete(...) even if
we did not find the index
- Adjust testing

Result:

Correct dispatching based on destination connection id

codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicCodecDispatcher.java

@@ -138,12 +138,20 @@ public final void channelReadComplete(ChannelHandlerContext ctx) {
         // Loop over all ChannelHandlerContextDispatchers and ensure fireChannelReadComplete() is called if required.
         // We use and old style for loop as CopyOnWriteArrayList implements RandomAccess and so we can
         // reduce the object creations.
+        boolean dispatchForOwnContextAlready = false;
         for (int i = 0; i < contextList.size(); i++) {
             ChannelHandlerContextDispatcher ctxDispatcher = contextList.get(i);
             if (ctxDispatcher != null) {
-                ctxDispatcher.fireChannelReadCompleteIfNeeded();
+                boolean fired = ctxDispatcher.fireChannelReadCompleteIfNeeded();
+                if (fired && !dispatchForOwnContextAlready) {
+                    // Check if we dispatched to ctx so if we didnt at the end we can do it manually.
+                    dispatchForOwnContextAlready = ctx.equals(ctxDispatcher.ctx);
+                }
             }
         }
+        if (!dispatchForOwnContextAlready) {
+            ctx.fireChannelReadComplete();
+        }
     }
 
 
@@ -188,9 +196,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 +216,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) {
@@ -317,12 +307,14 @@ void fireChannelRead(Object msg) {
             set(true);
         }
 
-        void fireChannelReadCompleteIfNeeded() {
+        boolean fireChannelReadCompleteIfNeeded() {
             if (getAndSet(false)) {
                 // There was a fireChannelRead() before, let's call fireChannelReadComplete()
                 // so the user is aware that we might be done with the reading loop.
                 ctx.fireChannelReadComplete();
+                return true;
             }
+            return false;
         }
     }
 }

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();