diff --git a/quiche/quic/core/crypto/transport_parameters.cc b/quiche/quic/core/crypto/transport_parameters.cc index 06faf053e..c5c6d512e 100644 --- a/quiche/quic/core/crypto/transport_parameters.cc +++ b/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), diff --git a/quiche/quic/core/crypto/transport_parameters_test.cc b/quiche/quic/core/crypto/transport_parameters_test.cc index efc592677..d1c96324e 100644 --- a/quiche/quic/core/crypto/transport_parameters_test.cc +++ b/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 { diff --git a/quiche/quic/core/http/quic_spdy_client_stream_test.cc b/quiche/quic/core/http/quic_spdy_client_stream_test.cc index fce247548..fb80686a2 100644 --- a/quiche/quic/core/http/quic_spdy_client_stream_test.cc +++ b/quiche/quic/core/http/quic_spdy_client_stream_test.cc @@ -7,6 +7,7 @@ #include #include #include +#include #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 { 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( + 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( + simple_session.CreateOutgoingBidirectionalStream()); + ASSERT_NE(simple_stream, nullptr); + + std::vector 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()); + 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(), diff --git a/quiche/quic/core/tls_client_handshaker.cc b/quiche/quic/core/tls_client_handshaker.cc index 298ffb017..d0d703f51 100644 --- a/quiche/quic/core/tls_client_handshaker.cc +++ b/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; } diff --git a/quiche/quic/core/tls_server_handshaker.cc b/quiche/quic/core/tls_server_handshaker.cc index 065edbe18..4b9bdbeeb 100644 --- a/quiche/quic/core/tls_server_handshaker.cc +++ b/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()); diff --git a/quiche/quic/tools/quic_simple_client_session.cc b/quiche/quic/tools/quic_simple_client_session.cc index 7d0e50ebf..5f9344a52 100644 --- a/quiche/quic/tools/quic_simple_client_session.cc +++ b/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 diff --git a/quiche/quic/tools/quic_simple_client_session.h b/quiche/quic/tools/quic_simple_client_session.h index 41bd52a8c..99baa4d95 100644 --- a/quiche/quic/tools/quic_simple_client_session.h +++ b/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 on_interim_headers_; QuicClientBase::NetworkHelper* network_helper_;