Handle missing interim header callbacks and enforce stream limits

quiche/quic/core/crypto/transport_parameters.cc

@@ -119,6 +119,8 @@ constexpr uint64_t kMaxMaxAckDelayTransportParam = 16383;
 constexpr uint64_t kDefaultMaxAckDelayTransportParam = 25;
 constexpr uint64_t kMinActiveConnectionIdLimitTransportParam = 2;
 constexpr uint64_t kDefaultActiveConnectionIdLimitTransportParam = 2;
+constexpr uint64_t kMaxInitialMaxStreamsLimit = 1ULL << 60;
+
 
 std::string TransportParameterIdToString(
     TransportParameters::TransportParameterId param_id) {
@@ -512,8 +514,10 @@ TransportParameters::TransportParameters()
       initial_max_stream_data_bidi_local(kInitialMaxStreamDataBidiLocal),
       initial_max_stream_data_bidi_remote(kInitialMaxStreamDataBidiRemote),
       initial_max_stream_data_uni(kInitialMaxStreamDataUni),
-      initial_max_streams_bidi(kInitialMaxStreamsBidi),
-      initial_max_streams_uni(kInitialMaxStreamsUni),
+      initial_max_streams_bidi(kInitialMaxStreamsBidi, 0, 0,
+                               kMaxInitialMaxStreamsLimit),
+      initial_max_streams_uni(kInitialMaxStreamsUni, 0, 0,
+                              kMaxInitialMaxStreamsLimit),
       ack_delay_exponent(kAckDelayExponent,
                          kDefaultAckDelayExponentTransportParam, 0,
                          kMaxAckDelayExponentTransportParam),

quiche/quic/core/crypto/transport_parameters_test.cc

@@ -1127,6 +1127,81 @@ TEST_P(TransportParametersTest, ServerCannotSendDebuggingSni) {
       "debugging_sni");
 }
 
+TEST_P(TransportParametersTest, InvalidMaxStreamsBidiOrUni) {
+  {
+    TransportParameters params;
+    std::string error_details;
+    params.perspective = Perspective::IS_CLIENT;
+    EXPECT_TRUE(params.AreValid(&error_details));
+    EXPECT_TRUE(error_details.empty());
+
+    params.initial_max_streams_bidi.set_value(1ULL << 60);
+    EXPECT_TRUE(params.AreValid(&error_details));
+    EXPECT_TRUE(error_details.empty());
+
+    params.initial_max_streams_bidi.set_value((1ULL << 60) + 1);
+    EXPECT_FALSE(params.AreValid(&error_details));
+    EXPECT_EQ(error_details,
+              "Invalid transport parameters [Client initial_max_streams_bidi "
+              "1152921504606846977 (Invalid)]");
+  }
+  {
+    TransportParameters params;
+    std::string error_details;
+    params.perspective = Perspective::IS_CLIENT;
+    EXPECT_TRUE(params.AreValid(&error_details));
+    EXPECT_TRUE(error_details.empty());
+
+    params.initial_max_streams_uni.set_value(1ULL << 60);
+    EXPECT_TRUE(params.AreValid(&error_details));
+    EXPECT_TRUE(error_details.empty());
+
+    params.initial_max_streams_uni.set_value((1ULL << 60) + 1);
+    EXPECT_FALSE(params.AreValid(&error_details));
+    EXPECT_EQ(error_details,
+              "Invalid transport parameters [Client initial_max_streams_uni "
+              "1152921504606846977 (Invalid)]");
+  }
+}
+
+TEST_P(TransportParametersTest, ParseClientParamsFailsWithInvalidInitialMaxStreamsBidi) {
+  // initial_max_streams_bidi parameter ID is 0x08.
+  // 1ULL << 60 + 1 is 1152921504606846977 (0x1000000000000001).
+  // In varint62, it is represented as 8 bytes: 0xD0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01.
+  const uint8_t kClientParams[] = {
+      0x08,  // parameter id
+      0x08,  // length
+      0xd0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,  // value (2^60 + 1)
+  };
+  TransportParameters out_params;
+  std::string error_details;
+  EXPECT_FALSE(ParseTransportParameters(GetParam(), Perspective::IS_CLIENT,
+                                        kClientParams, sizeof(kClientParams),
+                                        &out_params, &error_details));
+  EXPECT_EQ(error_details,
+            "Invalid transport parameters [Client initial_max_streams_bidi "
+            "1152921504606846977 (Invalid)]");
+}
+
+TEST_P(TransportParametersTest, ParseClientParamsFailsWithInvalidInitialMaxStreamsUni) {
+  // initial_max_streams_uni parameter ID is 0x09.
+  // 1ULL << 60 + 1 is 1152921504606846977 (0x1000000000000001).
+  // In varint62, it is represented as 8 bytes: 0xD0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01.
+  const uint8_t kClientParams[] = {
+      0x09,  // parameter id
+      0x08,  // length
+      0xd0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,  // value (2^60 + 1)
+  };
+  TransportParameters out_params;
+  std::string error_details;
+  EXPECT_FALSE(ParseTransportParameters(GetParam(), Perspective::IS_CLIENT,
+                                        kClientParams, sizeof(kClientParams),
+                                        &out_params, &error_details));
+  EXPECT_EQ(error_details,
+            "Invalid transport parameters [Client initial_max_streams_uni "
+            "1152921504606846977 (Invalid)]");
+}
+
 class TransportParametersTicketSerializationTest : public QuicTest {
  protected:
   void SetUp() override {

quiche/quic/core/http/quic_spdy_client_stream_test.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <string>
 #include <utility>
+#include <vector>
 
 #include "absl/strings/str_cat.h"
 #include "quiche/quic/core/crypto/null_encrypter.h"
@@ -20,6 +21,7 @@
 #include "quiche/quic/test_tools/crypto_test_utils.h"
 #include "quiche/quic/test_tools/quic_spdy_session_peer.h"
 #include "quiche/quic/test_tools/quic_test_utils.h"
+#include "quiche/quic/tools/quic_simple_client_session.h"
 #include "quiche/common/simple_buffer_allocator.h"
 
 using quiche::HttpHeaderBlock;
@@ -55,6 +57,23 @@ class MockQuicSpdyClientSession : public QuicSpdyClientSession {
   QuicCryptoClientConfig crypto_config_;
 };
 
+class TestQuicSimpleClientSession : public QuicSimpleClientSession {
+ public:
+  TestQuicSimpleClientSession(
+      const ParsedQuicVersionVector& supported_versions,
+      QuicConnection* connection)
+      : QuicSimpleClientSession(DefaultQuicConfig(), supported_versions,
+                                connection, /*network_helper=*/nullptr,
+                                QuicServerId("example.com", 443),
+                                &crypto_config_,
+                                /*drop_response_body=*/false,
+                                /*enable_web_transport=*/false),
+        crypto_config_(crypto_test_utils::ProofVerifierForTesting()) {}
+
+ private:
+  QuicCryptoClientConfig crypto_config_;
+};
+
 class QuicSpdyClientStreamTest : public QuicTestWithParam<ParsedQuicVersion> {
  public:
   class StreamVisitor;
@@ -311,6 +330,68 @@ TEST_P(QuicSpdyClientStreamTest, TestReceiving101) {
               IsStreamError(QUIC_BAD_APPLICATION_PAYLOAD));
 }
 
+TEST_P(QuicSpdyClientStreamTest,
+       InterimResponseWithoutCallbackDoesNotCrashSimpleClient) {
+  TestQuicSimpleClientSession simple_session(connection_->supported_versions(),
+                                             connection_);
+  simple_session.Initialize();
+  auto* simple_stream = static_cast<QuicSpdyClientStream*>(
+      simple_session.CreateOutgoingBidirectionalStream());
+  ASSERT_NE(simple_stream, nullptr);
+
+  headers_[":status"] = "129";
+  auto headers = AsHeaderList(headers_);
+  simple_stream->OnStreamHeaderList(false, headers.uncompressed_header_bytes(),
+                                    headers);
+
+  EXPECT_EQ(129, simple_stream->response_code());
+  ASSERT_EQ(simple_stream->preliminary_headers().size(), 1);
+  EXPECT_EQ("129",
+            simple_stream->preliminary_headers()
+                .front()
+                .find(":status")
+                ->second);
+  EXPECT_THAT(simple_stream->stream_error(), IsQuicStreamNoError());
+}
+
+TEST_P(QuicSpdyClientStreamTest,
+       InterimResponseCallbackUsesLatestSessionHandler) {
+  TestQuicSimpleClientSession simple_session(connection_->supported_versions(),
+                                             connection_);
+  simple_session.Initialize();
+  auto* simple_stream = static_cast<QuicSpdyClientStream*>(
+      simple_session.CreateOutgoingBidirectionalStream());
+  ASSERT_NE(simple_stream, nullptr);
+
+  std::vector<std::string> statuses;
+  simple_session.set_on_interim_headers(
+      [&statuses](const HttpHeaderBlock& headers) {
+        statuses.push_back(std::string(headers.find(":status")->second));
+      });
+
+  headers_[":status"] = "103";
+  auto headers = AsHeaderList(headers_);
+  simple_stream->OnStreamHeaderList(false, headers.uncompressed_header_bytes(),
+                                    headers);
+
+  simple_session.set_on_interim_headers(
+      quiche::MultiUseCallback<void(const HttpHeaderBlock&)>());
+  headers_[":status"] = "199";
+  headers = AsHeaderList(headers_);
+  simple_stream->OnStreamHeaderList(false, headers.uncompressed_header_bytes(),
+                                    headers);
+
+  EXPECT_THAT(statuses, ElementsAre("103"));
+  ASSERT_EQ(simple_stream->preliminary_headers().size(), 2);
+  EXPECT_EQ("103",
+            simple_stream->preliminary_headers()
+                .front()
+                .find(":status")
+                ->second);
+  EXPECT_EQ("199",
+            simple_stream->preliminary_headers().back().find(":status")->second);
+}
+
 TEST_P(QuicSpdyClientStreamTest, TestFramingOnePacket) {
   auto headers = AsHeaderList(headers_);
   stream_->OnStreamHeaderList(false, headers.uncompressed_header_bytes(),

quiche/quic/core/tls_client_handshaker.cc

@@ -215,7 +215,7 @@ bool TlsClientHandshaker::PrepareZeroRttConfig(
           /*is_resumption = */ true, &error_details) != QUIC_NO_ERROR) {
     QUIC_BUG(quic_bug_10576_2)
         << "Unable to parse cached transport parameters.";
-    CloseConnection(QUIC_HANDSHAKE_FAILED,
+    CloseConnection(QUIC_HANDSHAKE_FAILED, TRANSPORT_PARAMETER_ERROR,
                     "Client failed to parse cached Transport Parameters.");
     return false;
   }
@@ -593,7 +593,8 @@ void TlsClientHandshaker::FinishHandshake() {
   std::string error_details;
   if (!ProcessTransportParameters(&error_details)) {
     QUICHE_DCHECK(!error_details.empty());
-    CloseConnection(QUIC_HANDSHAKE_FAILED, error_details);
+    CloseConnection(QUIC_HANDSHAKE_FAILED, TRANSPORT_PARAMETER_ERROR,
+                    error_details);
     return;
   }
 

quiche/quic/core/tls_server_handshaker.cc

@@ -997,7 +997,8 @@ ssl_select_cert_result_t TlsServerHandshaker::EarlySelectCertCallback(
     // No need to set_extra_error_details() - error_details already contains
     // enough information to indicate this is an error from
     // ProcessTransportParameters.
-    CloseConnection(QUIC_HANDSHAKE_FAILED, error_details);
+    CloseConnection(QUIC_HANDSHAKE_FAILED, TRANSPORT_PARAMETER_ERROR,
+                    error_details);
     return ssl_select_cert_error;
   }
   OverrideQuicConfigDefaults(session()->config());

quiche/quic/tools/quic_simple_client_session.cc

@@ -63,11 +63,18 @@ QuicSimpleClientSession::CreateClientStream() {
       drop_response_body_);
   stream->set_on_interim_headers(
       [this](const quiche::HttpHeaderBlock& headers) {
-        on_interim_headers_(headers);
+        MaybeNotifyInterimHeaders(headers);
       });
   return stream;
 }
 
+void QuicSimpleClientSession::MaybeNotifyInterimHeaders(
+    const quiche::HttpHeaderBlock& headers) {
+  if (on_interim_headers_) {
+    on_interim_headers_(headers);
+  }
+}
+
 WebTransportHttp3VersionSet
 QuicSimpleClientSession::LocallySupportedWebTransportVersions() const {
   return enable_web_transport_ ? kDefaultSupportedWebTransportVersions

quiche/quic/tools/quic_simple_client_session.h

@@ -64,6 +64,8 @@ class QuicSimpleClientSession : public QuicSpdyClientSession {
   }
 
  private:
+  void MaybeNotifyInterimHeaders(const quiche::HttpHeaderBlock& headers);
+
   quiche::MultiUseCallback<void(const quiche::HttpHeaderBlock&)>
       on_interim_headers_;
   QuicClientBase::NetworkHelper* network_helper_;