Generate new connection id when no tokens are used (java-native-acces…

…s#71)

Motivation:

When no tokens are used we should generate a new connection id and not use dcid

Modifications:

- Generate new id and pass it to accept(...)
- Adjust unit test to ensure that the mapping from id to channel still works

Result:

Correctly handle the case for no token usage

src/main/java/io/netty/incubator/codec/quic/QuicheQuicServerCodec.java

@@ -158,24 +158,30 @@ private QuicheQuicChannel handleServer(ChannelHandlerContext ctx, InetSocketAddr
         }
 
         final long conn;
+        final ByteBuffer key;
         if (noToken) {
+            connIdBuffer.clear();
+            key = connectionIdAddressGenerator.newId(
+                    dcid.internalNioBuffer(dcid.readerIndex(), dcid.readableBytes()), localConnIdLength);
+            connIdBuffer.writeBytes(key.duplicate());
             conn = Quiche.quiche_accept_no_token(
-                    Quiche.memoryAddress(dcid) + dcid.readerIndex(), localConnIdLength, nativeConfig);
+                    Quiche.memoryAddress(connIdBuffer) + connIdBuffer.readerIndex(), localConnIdLength, nativeConfig);
         } else {
             conn = Quiche.quiche_accept(Quiche.memoryAddress(dcid) + dcid.readerIndex(), localConnIdLength,
                     Quiche.memoryAddress(token) + offset, token.readableBytes() - offset, nativeConfig);
+
+            // Now create the key to store the channel in the map.
+            byte[] bytes = new byte[localConnIdLength];
+            dcid.getBytes(dcid.readerIndex(), bytes);
+            key = ByteBuffer.wrap(bytes);
         }
         if (conn < 0) {
             LOGGER.debug("quiche_accept failed");
             return null;
         }
 
-        // Now create the key to store the channel in the map.
-        byte[] key = new byte[localConnIdLength];
-        dcid.getBytes(dcid.readerIndex(), key);
-
         QuicheQuicChannel channel = QuicheQuicChannel.forServer(
-                ctx.channel(), ByteBuffer.wrap(key), conn, Quiche.traceId(conn, dcid), sender,
+                ctx.channel(), key, conn, Quiche.traceId(conn, dcid), sender,
                 streamHandler, streamOptionsArray, streamAttrsArray);
         Quic.setupChannel(channel, optionsArray, attrsArray, handler, LOGGER);
         putChannel(channel);

src/test/java/io/netty/incubator/codec/quic/QuicChannelConnectTest.java

@@ -16,6 +16,7 @@
 package io.netty.incubator.codec.quic;
 
 import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelHandlerContext;
@@ -30,6 +31,7 @@
 import java.net.InetSocketAddress;
 import java.nio.channels.AlreadyConnectedException;
 import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.LinkedBlockingQueue;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -164,8 +166,10 @@ public void testConnectAlreadyConnected() throws Throwable {
 
     @Test
     public void testConnectWithoutTokenValidation() throws Throwable {
+        int numBytes = 8;
         ChannelActiveVerifyHandler serverQuicChannelHandler = new ChannelActiveVerifyHandler();
-        ChannelStateVerifyHandler serverQuicStreamHandler = new ChannelStateVerifyHandler();
+        CountDownLatch serverLatch = new CountDownLatch(1);
+        CountDownLatch clientLatch = new CountDownLatch(1);
 
         Channel server = QuicTestUtils.newServer(new QuicTokenHandler() {
             // Disable token validation
@@ -183,7 +187,7 @@ public int validateToken(ByteBuf token, InetSocketAddress address) {
             public int maxTokenLength() {
                 return 0;
             }
-        }, serverQuicChannelHandler, serverQuicStreamHandler);
+        }, serverQuicChannelHandler, new BytesCountingHandler(serverLatch, numBytes));
         InetSocketAddress address = (InetSocketAddress) server.localAddress();
         Channel channel = QuicTestUtils.newClient();
         try {
@@ -194,19 +198,45 @@ public int maxTokenLength() {
                     .remoteAddress(address)
                     .connect()
                     .get();
-            assertTrue(quicChannel.close().await().isSuccess());
+            QuicStreamChannel stream = quicChannel.createStream(QuicStreamType.BIDIRECTIONAL,
+                    new BytesCountingHandler(clientLatch, numBytes)).get();
+            stream.writeAndFlush(Unpooled.directBuffer().writeZero(numBytes)).sync();
+            clientLatch.await();
+
+            stream.close().sync();
+            quicChannel.close().sync();
             ChannelFuture closeFuture = quicChannel.closeFuture().await();
             assertTrue(closeFuture.isSuccess());
             clientQuicChannelHandler.assertState();
         } finally {
+            serverLatch.await();
             serverQuicChannelHandler.assertState();
-            serverQuicStreamHandler.assertState();
 
             server.close().sync();
             // Close the parent Datagram channel as well.
             channel.close().sync();
         }
     }
+    private static final class BytesCountingHandler extends ChannelInboundHandlerAdapter {
+        private final CountDownLatch latch;
+        private final int numBytes;
+        private int bytes;
+
+        BytesCountingHandler(CountDownLatch latch, int numBytes) {
+            this.latch = latch;
+            this.numBytes = numBytes;
+        }
+
+        @Override
+        public void channelRead(ChannelHandlerContext ctx, Object msg) {
+            ByteBuf buffer = (ByteBuf) msg;
+            bytes += buffer.readableBytes();
+            ctx.writeAndFlush(buffer);
+            if (bytes == numBytes) {
+                latch.countDown();
+            }
+        }
+    }
 
     private static final class ChannelStateVerifyHandler extends ChannelInboundHandlerAdapter {
         private volatile Throwable cause;