From c8e1f8101ec62dbbb236870a23df1707b7aea5c6 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Wed, 24 Jan 2024 07:52:45 -0800
Subject: [PATCH] Fire SniCompletionEvent as soon as the SNI hostname was
 selected (#644)

Motivation:

We did fire the SniCompletionEvent only once we were done with the
handshake which is not really the correct time to do this. We should
fire the event as soon as we were able to process the client hello and
selected the hostname.

Modifications:

- Fire the event as soon as we select the hostname

Result:

Fire the event at the correct time
---
 .../io/netty/incubator/codec/quic/QuicheQuicChannel.java  | 8 ++------
 .../netty/incubator/codec/quic/QuicheQuicConnection.java  | 4 +++-
 .../netty/incubator/codec/quic/QuicheQuicSslEngine.java   | 8 ++++++--
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicChannel.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicChannel.java
index 4e6f2520f..a9a9da625 100644
--- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicChannel.java
+++ b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicChannel.java
@@ -247,11 +247,6 @@ private void notifyAboutHandshakeCompletionIfNeeded(SSLHandshakeException cause)
             case NOT_HANDSHAKING:
             case FINISHED:
                 handshakeCompletionNotified = true;
-                String sniHostname = connection.engine().sniHostname;
-                if (sniHostname != null) {
-                    connection.engine().sniHostname = null;
-                    pipeline().fireUserEventTriggered(new SniCompletionEvent(sniHostname));
-                }
                 pipeline().fireUserEventTriggered(SslHandshakeCompletionEvent.SUCCESS);
                 break;
             default:
@@ -279,7 +274,8 @@ void attachQuicheConnection(QuicheQuicConnection connection) {
             this.traceId = new String(traceId);
         }
 
-        connection.initInfo(local, remote);
+        connection.init(local, remote,
+                sniHostname -> pipeline().fireUserEventTriggered(new SniCompletionEvent(sniHostname)));
 
         // Setup QLOG if needed.
         QLogConfiguration configuration = config.getQLogConfiguration();
diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicConnection.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicConnection.java
index b9642fe0a..a407b6965 100644
--- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicConnection.java
+++ b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicConnection.java
@@ -23,6 +23,7 @@
 
 import java.net.InetSocketAddress;
 import java.nio.ByteBuffer;
+import java.util.function.Consumer;
 import java.util.function.Supplier;
 
 final class QuicheQuicConnection {
@@ -178,7 +179,7 @@ long address() {
         return connection;
     }
 
-    void initInfo(InetSocketAddress local, InetSocketAddress remote) {
+    void init(InetSocketAddress local, InetSocketAddress remote, Consumer<String> sniSelectedCallback) {
         assert connection != -1;
         assert recvInfoBuffer.refCnt() != 0;
         assert sendInfoBuffer.refCnt() != 0;
@@ -190,6 +191,7 @@ void initInfo(InetSocketAddress local, InetSocketAddress remote) {
         // Fill both quiche_send_info structs with the same address.
         QuicheSendInfo.setSendInfo(sendInfoBuffer1, local, remote);
         QuicheSendInfo.setSendInfo(sendInfoBuffer2, local, remote);
+        engine.sniSelectedCallback = sniSelectedCallback;
     }
 
     ByteBuffer nextRecvInfo() {
diff --git a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicSslEngine.java b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicSslEngine.java
index 9565dba20..5bb4eb766 100644
--- a/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicSslEngine.java
+++ b/codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicSslEngine.java
@@ -39,6 +39,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Consumer;
 import java.util.function.LongFunction;
 
 final class QuicheQuicSslEngine extends QuicSslEngine {
@@ -54,7 +55,7 @@ final class QuicheQuicSslEngine extends QuicSslEngine {
     final String tlsHostName;
     volatile QuicheQuicConnection connection;
 
-    String sniHostname;
+    volatile Consumer<String> sniSelectedCallback;
 
     QuicheQuicSslEngine(QuicheQuicSslContext ctx, String peerHost, int peerPort) {
         this.ctx = ctx;
@@ -75,7 +76,10 @@ long moveTo(String hostname, QuicheQuicSslContext ctx) {
         this.ctx.remove(this);
         this.ctx = ctx;
         long added = ctx.add(this);
-        sniHostname = hostname;
+        Consumer<String> sniSelectedCallback = this.sniSelectedCallback;
+        if (sniSelectedCallback != null) {
+            sniSelectedCallback.accept(hostname);
+        }
         return added;
     }