quic: fix two small bugs in HTTP/3 stream internals

Signed-off-by: Tim Perry <pimterry@gmail.com>
PR-URL: #65970
Reviewed-By: James M Snell <jasnell@gmail.com>

Author: pimterry

src/quic/application.cc

@@ -347,11 +347,6 @@ class DefaultApplication final : public Session::Application {
   }
 
   int GetStreamData(Session::StreamData* stream_data) override {
-    // Reset the state of stream_data before proceeding...
-    stream_data->id = -1;
-    stream_data->count = 0;
-    stream_data->fin = false;
-    stream_data->stream.reset();
     Debug(&session(), "Default application getting stream data");
     DCHECK_NOT_NULL(stream_data);
     // If the queue is empty, there aren't any streams with data yet

src/quic/http3.cc

@@ -212,7 +212,6 @@ class Http3ApplicationImpl final : public Session::Application {
 
   bool Start() override {
     if (started_) return true;
-    started_ = true;
     Debug(&session(), "Starting HTTP/3 application.");
 
     const auto params = session().remote_transport_params();
@@ -246,15 +245,15 @@ class Http3ApplicationImpl final : public Session::Application {
     }
 
     Debug(&session(), "Creating and binding HTTP/3 control streams");
-    bool ret =
+    started_ =
         session().OpenUnidirectionalStream(&control_stream_id_) &&
         session().OpenUnidirectionalStream(&qpack_enc_stream_id_) &&
         session().OpenUnidirectionalStream(&qpack_dec_stream_id_) &&
         nghttp3_conn_bind_control_stream(*this, control_stream_id_) == 0 &&
         nghttp3_conn_bind_qpack_streams(
             *this, qpack_enc_stream_id_, qpack_dec_stream_id_) == 0;
 
-    if (env()->enabled_debug_list()->enabled(DebugCategory::QUIC) && ret) {
+    if (env()->enabled_debug_list()->enabled(DebugCategory::QUIC) && started_) {
       Debug(&session(),
             "Created and bound control stream %" PRIi64,
             control_stream_id_);
@@ -266,7 +265,7 @@ class Http3ApplicationImpl final : public Session::Application {
             qpack_dec_stream_id_);
     }
 
-    return ret;
+    return started_;
   }
 
   void BeginShutdown() override {
@@ -688,18 +687,16 @@ class Http3ApplicationImpl final : public Session::Application {
             offsetof(ngtcp2_vec, base) == offsetof(nghttp3_vec, base) &&
             offsetof(ngtcp2_vec, len) == offsetof(nghttp3_vec, len),
         "ngtcp2_vec and nghttp3_vec must have identical layout");
-    data->count = kMaxVectorCount;
-    ssize_t ret = 0;
     Debug(&session(), "HTTP/3 application getting stream data");
     if (conn_ && session().max_data_left()) {
       // nghttp3 reports fin through an int out-param; bridge it to the bool.
       int fin = 0;
-      ret =
+      ssize_t ret =
           nghttp3_conn_writev_stream(*this,
                                      &data->id,
                                      &fin,
                                      reinterpret_cast<nghttp3_vec*>(data->data),
-                                     data->count);
+                                     kMaxVectorCount);
       // A negative return value indicates an error.
       if (ret < 0) {
         return static_cast<int>(ret);

src/quic/session.cc

@@ -2016,6 +2016,12 @@ void Session::SendPendingData() {
     }
 
     // The stream_data is the next block of data from the application stream.
+    // It is reused across iterations, so we reset before it's populated:
+    stream_data.count = 0;
+    stream_data.id = -1;
+    stream_data.fin = false;
+    stream_data.stream.reset();
+
     if (application().GetStreamData(&stream_data) < 0) {
       Debug(this, "Application failed to get stream data");
       SetLastError(QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL));

test/parallel/test-quic-h3-uni-stream-limit-start-failure.mjs

@@ -0,0 +1,38 @@
+// Flags: --experimental-quic --no-warnings
+
+// An HTTP/3 session must cleanly fail if the peer advertises fewer than
+// the 3 unidirectional streams that HTTP/3 needs for control and QPACK.
+
+import { hasQuic, skip, mustNotCall } from '../common/index.mjs';
+import assert from 'node:assert';
+import * as fixtures from '../common/fixtures.mjs';
+
+if (!hasQuic) {
+  skip('QUIC is not enabled');
+}
+
+const { listen, connect } = await import('node:quic');
+const { createPrivateKey } = await import('node:crypto');
+
+const key = createPrivateKey(fixtures.readKey('agent1-key.pem'));
+const cert = fixtures.readKey('agent1-cert.pem');
+
+const serverEndpoint = await listen(async (serverSession) => {
+  await serverSession.closed;
+}, {
+  sni: { '*': { keys: [key], certs: [cert] } },
+  // No uni streams allowed:
+  transportParams: { initialMaxStreamsUni: 0 },
+  onheaders: mustNotCall(),
+});
+
+// Expect the client to cleanly fail - not crash the process
+await assert.rejects(async () => {
+  const clientSession = await connect(serverEndpoint.address, {
+    servername: 'localhost',
+    verifyPeer: 'manual',
+  });
+  await clientSession.opened;
+}, { code: 'ERR_QUIC_TRANSPORT_ERROR' });
+
+await serverEndpoint.close();