quic: defer server session emit until TLS ClientHello is processed

doc/api/quic.md

@@ -1418,6 +1418,30 @@ will be silently dropped and `0n` returned. The local
 `maxDatagramFrameSize` transport parameter (default: `1200` bytes) controls
 what this endpoint advertises to the peer as its own maximum.
 
+### `session.servername`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* Type: {string|undefined}
+
+The SNI (Server Name Indication) host name associated with the session, or
+`undefined` if none was set. On a client this is the `servername` requested via
+[`quic.connect()`][]. On a server it is the name sent by the peer.
+
+### `session.alpnProtocol`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* Type: {string|undefined}
+
+The negotiated ALPN protocol. On a server this is available synchronously as
+soon as the session is surfaced to the `onsession` callback; on a client it is
+`undefined` until the handshake completes.
+
 ### `session.certificate`
 
 <!-- YAML
@@ -3563,7 +3587,11 @@ added: v23.8.0
 * `this` {quic.QuicEndpoint}
 * `session` {quic.QuicSession}
 
-The callback function that is invoked when a new session is initiated by a remote peer.
+The callback function that is invoked when a new server session is initiated by
+a remote peer. It is called once the peer's TLS `ClientHello` has been
+processed, so the negotiated TLS parameters are immediately available when
+the callback runs. Sessions whose handshake is rejected before this point are
+never surfaced.
 
 ### Callback: `OnStreamCallback`
 

lib/internal/quic/quic.js

@@ -744,7 +744,9 @@ setCallbacks({
     this[kOwner][kFinishClose](context, status);
   },
   /**
-   * Called when the QuicEndpoint C++ handle receives a new server-side session
+   * Called when a new server session is surfaced. The emit happens once the
+   * session's ClientHello has been processed, so its servername/protocol
+   * getters are already readable.
    * @param {object} session The QuicSession C++ handle
    */
   onSessionNew(session) {
@@ -2877,6 +2879,26 @@ class QuicSession {
     }
   }
 
+  /**
+   * The SNI servername, or undefined when none was sent.
+   * @type {string|undefined}
+   */
+  get servername() {
+    assertIsQuicSession(this);
+    if (this.destroyed) return undefined;
+    return this.#handle.getServername();
+  }
+
+  /**
+   * The negotiated ALPN protocol.
+   * @type {string|undefined}
+   */
+  get alpnProtocol() {
+    assertIsQuicSession(this);
+    if (this.destroyed) return undefined;
+    return this.#handle.getAlpnProtocol();
+  }
+
   /** @type {OnDatagramCallback} */
   get ondatagram() {
     assertIsQuicSession(this);

src/quic/endpoint.cc

@@ -814,8 +814,8 @@ void Endpoint::AddSession(const CID& cid, BaseObjectPtr<Session> session) {
   // For server sessions, associate the client's original DCID (ocid) so
   // that 0-RTT packets arriving in a separate UDP datagram can be routed
   // to this session. This must happen after the session is added (so
-  // FindSession can resolve the mapping) but before EmitNewSession (which
-  // runs JS and may yield to libuv, allowing the 0-RTT packet to arrive).
+  // FindSession can resolve the mapping) and before any JS runs (which
+  // may yield to libuv, allowing the 0-RTT packet to arrive).
   if (session->is_server() && session->config().ocid) {
     AssociateCID(session->config().ocid, session->config().scid);
   }
@@ -825,22 +825,22 @@ void Endpoint::AddSession(const CID& cid, BaseObjectPtr<Session> session) {
   if (session->is_server() && session->config().retry_scid) {
     AssociateCID(session->config().retry_scid, session->config().scid);
   }
-  // Increment the primary session count and ref the handle BEFORE
-  // EmitNewSession. EmitNewSession calls into JS, which may close/destroy
-  // the session synchronously. The session's ~Impl calls RemoveSession
-  // which decrements the count. If we increment after EmitNewSession,
-  // RemoveSession would see count=0 and the count would be permanently
-  // off by one.
+  // Increment the primary session count and ref the handle BEFORE any
+  // JS can run for this session (the deferred EmitNewSession, or packet
+  // processing callbacks). JS may close/destroy the session
+  // synchronously; the session's ~Impl calls RemoveSession which
+  // decrements the count. If we incremented after, RemoveSession would
+  // see count=0 and the count would be permanently off by one.
   if (primary_session_count_++ == 0) {
     idle_timer_.Stop();
     udp_.Ref();
   }
   if (session->is_server()) {
     STAT_INCREMENT(Stats, server_sessions);
-    // We only emit the new session event for server sessions.
-    EmitNewSession(session);
-    // It is important to note that the session may be closed/destroyed
-    // when it is emitted here.
+    // Note that we don't emit new sessions here - that's deferred until the
+    // ClientHello has been processed (see Session::ReadPacket), so the
+    // session is exposed to JS only once its SNI/ALPN are known and invalid
+    // handshakes never surface.
   } else {
     STAT_INCREMENT(Stats, client_sessions);
   }
@@ -1980,6 +1980,12 @@ void Endpoint::EmitNewSession(const BaseObjectPtr<Session>& session) {
   // the call to MakeCallback. If that's the case, the session object still
   // exists but it is in a destroyed state. Care should be taken accessing
   // session after this point.
+
+  // Deliver any stream events that were held until the stream was setup,
+  // e.g. 0-RTT streams from the first flight.
+  if (!session->is_destroyed()) {
+    session->ReplayDeferredEmits();
+  }
 }
 
 void Endpoint::EmitClose(CloseContext context, int status) {

src/quic/session.cc

@@ -184,6 +184,8 @@ uint64_t MaxDatagramPayload(uint64_t max_frame_size) {
 #define SESSION_JS_METHODS(V)                                                  \
   V(Destroy, destroy, SIDE_EFFECT)                                             \
   V(GetRemoteAddress, getRemoteAddress, NO_SIDE_EFFECT)                        \
+  V(GetServername, getServername, NO_SIDE_EFFECT)                              \
+  V(GetAlpnProtocol, getAlpnProtocol, NO_SIDE_EFFECT)                          \
   V(GetLocalAddress, getLocalAddress, NO_SIDE_EFFECT)                          \
   V(GetCertificate, getCertificate, NO_SIDE_EFFECT)                            \
   V(GetEphemeralKeyInfo, getEphemeralKey, NO_SIDE_EFFECT)                      \
@@ -781,6 +783,8 @@ struct Session::Impl final : public MemoryRetainer {
   SocketAddress remote_address_;
   std::unique_ptr<Application> application_;
   StreamsMap streams_;
+  // Emits deferred until after session setup is completed
+  std::vector<std::function<void()>> deferred_emits_;
   TimerWrapHandle timer_;
   size_t send_scope_depth_ = 0;
   QuicError last_error_;
@@ -1001,6 +1005,36 @@ struct Session::Impl final : public MemoryRetainer {
     session->Destroy();
   }
 
+  // The SNI servername from the TLS handshake; empty (-> undefined) only if
+  // none was sent.
+  JS_METHOD(GetServername) {
+    auto env = Environment::GetCurrent(args);
+    Session* session;
+    ASSIGN_OR_RETURN_UNWRAP(&session, args.This());
+    if (session->is_destroyed()) return;
+    auto sn = session->tls_session().servername();
+    if (sn.empty()) return;
+    Local<Value> ret;
+    if (ToV8Value(env->context(), sn).ToLocal(&ret)) {
+      args.GetReturnValue().Set(ret);
+    }
+  }
+
+  // The negotiated ALPN protocol. Undefined only for clients before the
+  // handshake is completed, as ALPN is mandatory for QUIC.
+  JS_METHOD(GetAlpnProtocol) {
+    auto env = Environment::GetCurrent(args);
+    Session* session;
+    ASSIGN_OR_RETURN_UNWRAP(&session, args.This());
+    if (session->is_destroyed()) return;
+    auto proto = session->tls_session().protocol();
+    if (proto.empty()) return;
+    Local<Value> ret;
+    if (ToV8Value(env->context(), proto).ToLocal(&ret)) {
+      args.GetReturnValue().Set(ret);
+    }
+  }
+
   JS_METHOD(GetRemoteAddress) {
     auto env = Environment::GetCurrent(args);
     Session* session;
@@ -2190,6 +2224,12 @@ const Session::Options& Session::options() const {
 void Session::EmitQlog(uint32_t flags, std::string_view data) {
   if (!env()->can_call_into_js()) return;
 
+  if (!is_destroyed() && must_defer_emits()) {
+    QueueDeferredEmit(
+        [this, flags, held = std::string(data)]() { EmitQlog(flags, held); });
+    return;
+  }
+
   bool fin = (flags & NGTCP2_QLOG_WRITE_FLAG_FIN) != 0;
 
   // Fun fact... ngtcp2 does not emit the final qlog statement until the
@@ -2312,6 +2352,16 @@ bool Session::ReadPacket(const uint8_t* data,
         // Process deferred operations that couldn't run inside callback
         // scopes (e.g., HTTP/3 GOAWAY handling that calls into JS).
         application().PostReceive();
+        // Surface a server session to JS once its ClientHello has been
+        // processed (OnSelectAlpn fired: SNI + ALPN are known and reliable).
+        // Held first-flight events - including 0-RTT request streams - replay
+        // at emit. The !wrapped guard makes this fire exactly once, on
+        // whichever packet completes the ClientHello (so a multi-datagram
+        // ClientHello is handled correctly).
+        if (is_server() && hello_processed_ && !impl_->state()->wrapped &&
+            !is_destroyed()) {
+          endpoint().EmitNewSession(BaseObjectPtr<Session>(this));
+        }
       }
       return true;
     }
@@ -2975,6 +3025,30 @@ void Session::set_wrapped() {
   impl_->state()->wrapped = 1;
 }
 
+bool Session::must_defer_emits() const {
+  // Server sessions are surfaced to JS (via the deferred new-session emit)
+  // only after the ClientHello has been processed and wrapped; anything
+  // emitted before then has no JS wrapper to receive it and must be held
+  // for replay.
+  return is_server() && !impl_->state()->wrapped;
+}
+
+void Session::QueueDeferredEmit(std::function<void()> fn) {
+  impl_->deferred_emits_.emplace_back(std::move(fn));
+}
+
+void Session::ReplayDeferredEmits() {
+  if (is_destroyed()) return;
+  DCHECK(impl_->state()->wrapped);
+  // Runs synchronously immediately after the new-session callback
+  // returns (still within first-flight processing).
+  auto emits = std::move(impl_->deferred_emits_);
+  for (auto& emit : emits) {
+    if (is_destroyed()) return;
+    emit();
+  }
+}
+
 void Session::set_priority_supported(bool on) {
   DCHECK(!is_destroyed());
   impl_->state()->priority_supported = on ? 1 : 0;
@@ -3284,6 +3358,10 @@ bool Session::HandshakeCompleted() {
 
   Debug(this, "Session handshake completed");
   impl_->state()->handshake_completed = 1;
+  // This implies fully completing a handshake without setting hello_processed
+  // (set during ALPN negotiation). Should be impossible unless ALPN flow is
+  // changed drastically, but good to check as it'd lose sessions.
+  DCHECK(!is_server() || hello_processed_);
 
   STAT_RECORD_TIMESTAMP(Stats, handshake_completed_at);
   SetStreamOpenAllowed();
@@ -3482,6 +3560,7 @@ void Session::set_max_datagram_size(uint16_t size) {
 
 void Session::EmitGoaway(stream_id last_stream_id) {
   if (is_destroyed()) return;
+  if (DeferEmit([this, last_stream_id] { EmitGoaway(last_stream_id); })) return;
   if (!env()->can_call_into_js()) return;
 
   CallbackScope<Session> cb_scope(this);
@@ -3496,6 +3575,14 @@ void Session::EmitGoaway(stream_id last_stream_id) {
 
 void Session::EmitDatagram(Store&& datagram, DatagramReceivedFlags flag) {
   DCHECK(!is_destroyed());
+
+  if (must_defer_emits()) {
+    QueueDeferredEmit([this, datagram = std::move(datagram), flag]() mutable {
+      EmitDatagram(std::move(datagram), flag);
+    });
+    return;
+  }
+
   if (!env()->can_call_into_js()) return;
 
   CallbackScope<Session> cbv_scope(this);
@@ -3511,6 +3598,8 @@ void Session::EmitDatagram(Store&& datagram, DatagramReceivedFlags flag) {
 void Session::EmitDatagramStatus(datagram_id id, quic::DatagramStatus status) {
   DCHECK(!is_destroyed());
 
+  if (DeferEmit([this, id, status] { EmitDatagramStatus(id, status); })) return;
+
   if (!env()->can_call_into_js()) return;
 
   CallbackScope<Session> cb_scope(this);
@@ -3672,6 +3761,7 @@ void Session::EmitSessionTicket(Store&& ticket) {
 
 void Session::EmitApplication() {
   if (is_destroyed()) return;
+  if (DeferEmit([this] { EmitApplication(); })) return;
   if (!env()->can_call_into_js()) return;
 
   if (!has_application()) {
@@ -3742,6 +3832,10 @@ void Session::EmitNewToken(const uint8_t* token, size_t len) {
 void Session::EmitStream(const BaseObjectWeakPtr<Stream>& stream) {
   DCHECK(!is_destroyed());
 
+  if (DeferEmit([this, stream] { EmitStream(stream); })) return;
+
+  if (!stream) return;
+
   if (!env()->can_call_into_js()) return;
   CallbackScope<Session> cb_scope(this);
 
@@ -3797,6 +3891,14 @@ void Session::EmitVersionNegotiation(const ngtcp2_pkt_hd& hd,
 
 void Session::EmitOrigins(std::vector<std::string>&& origins) {
   DCHECK(!is_destroyed());
+
+  if (must_defer_emits()) {
+    QueueDeferredEmit([this, origins = std::move(origins)]() mutable {
+      EmitOrigins(std::move(origins));
+    });
+    return;
+  }
+
   if (!HasListenerFlag(impl_->state()->listener_flags,
                        SessionListenerFlags::ORIGIN))
     return;
@@ -3822,11 +3924,17 @@ void Session::EmitOrigins(std::vector<std::string>&& origins) {
 
 void Session::EmitKeylog(const char* line) {
   DCHECK(!is_destroyed());
+
+  if (must_defer_emits()) {
+    QueueDeferredEmit(
+        [this, str = std::string(line)]() { EmitKeylog(str.c_str()); });
+    return;
+  }
+
   if (!env()->can_call_into_js()) return;
 
-  auto str = std::string(line);
   Local<Value> argv[] = {Undefined(env()->isolate())};
-  if (!ToV8Value(env()->context(), str).ToLocal(&argv[0])) {
+  if (!ToV8Value(env()->context(), std::string(line)).ToLocal(&argv[0])) {
     Debug(this, "Failed to convert keylog line to V8 string");
     return;
   }

src/quic/session.h

@@ -536,11 +536,32 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source {
   // before the handshake completes.
   void PopulateEarlyTransportParamsState();
 
+  void set_hello_processed() { hello_processed_ = true; }
+
   // It's a terrible name but "wrapped" here means that the Session has been
   // passed out to JavaScript and should be "wrapped" by whatever handler is
   // defined there to manage it.
   void set_wrapped();
 
+  // True while JS emits must be held for later replay, before the handshake
+  // is complete and the server session event has been emitted.
+  bool must_defer_emits() const;
+
+  // Replays, in order, any emits held while must_defer_emits() was true.
+  // Called synchronously right after the new-session emit.
+  void ReplayDeferredEmits();
+
+  // Queues fn to be replayed by ReplayDeferredEmits(). Out-of-line so the
+  // header does not need the full Impl definition.
+  void QueueDeferredEmit(std::function<void()> fn);
+
+  template <typename F>
+  bool DeferEmit(F&& fn) {
+    if (!must_defer_emits()) return false;
+    QueueDeferredEmit(std::forward<F>(fn));
+    return true;
+  }
+
   enum class CloseMethod : uint8_t {
     // Immediate close with a roundtrip through JavaScript, causing all
     // currently opened streams to be closed. An attempt will be made to
@@ -656,6 +677,8 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source {
   };
   Flags flags_;
 
+  bool hello_processed_ = false;
+
   QuicConnectionPointer connection_;
   std::unique_ptr<TLSSession> tls_session_;
   friend struct NgTcp2CallbackScope;