Skip to content

Commit

Permalink
Only try to find index for QUIC packets with short header format
Browse files Browse the repository at this point in the history
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
- Adjust testing

Result:

Correct dispatching based on destination connection id
  • Loading branch information
normanmaurer committed Mar 11, 2024
1 parent 902d579 commit 8e46225
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 28 deletions.
Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down
Expand Up @@ -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 {

Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 8e46225

Please sign in to comment.