Modify QuicConfig::ProcessTranportParameters() to support client side 0-rtt resumption. Unused code. not protected. PiperOrigin-RevId: 310182843 Change-Id: I171e7141f9ed4f6a98a7a9ea27ecda3138221832
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index 55930e1..8b78d7f 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -97,7 +97,8 @@ EXPECT_TRUE( session()->config()->FillTransportParameters(&transport_parameters)); error = session()->config()->ProcessTransportParameters( - transport_parameters, CLIENT, &error_details); + transport_parameters, CLIENT, /* is_resumption = */ false, + &error_details); } else { CryptoHandshakeMessage msg; session()->config()->ToHandshakeMessage(&msg, transport_version());
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc index d400fc7..bafb0f2 100644 --- a/quic/core/http/quic_spdy_stream_test.cc +++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -84,7 +84,8 @@ EXPECT_TRUE( session()->config()->FillTransportParameters(&transport_parameters)); error = session()->config()->ProcessTransportParameters( - transport_parameters, CLIENT, &error_details); + transport_parameters, CLIENT, /* is_resumption = */ false, + &error_details); } else { CryptoHandshakeMessage msg; session()->config()->ToHandshakeMessage(&msg, transport_version());
diff --git a/quic/core/quic_config.cc b/quic/core/quic_config.cc index 2508764..a0de6cb 100644 --- a/quic/core/quic_config.cc +++ b/quic/core/quic_config.cc
@@ -1107,6 +1107,7 @@ QuicErrorCode QuicConfig::ProcessTransportParameters( const TransportParameters& params, HelloType hello_type, + bool is_resumption, std::string* error_details) { if (params.idle_timeout_milliseconds.value() > 0 && params.idle_timeout_milliseconds.value() < @@ -1118,7 +1119,7 @@ params.idle_timeout_milliseconds.value()); } - if (!params.stateless_reset_token.empty()) { + if (!is_resumption && !params.stateless_reset_token.empty()) { QuicUint128 stateless_reset_token; if (params.stateless_reset_token.size() != sizeof(stateless_reset_token)) { QUIC_BUG << "Bad stateless reset token length " @@ -1166,26 +1167,28 @@ initial_max_stream_data_bytes_unidirectional_.SetReceivedValue( params.initial_max_stream_data_uni.value()); - if (GetQuicReloadableFlag(quic_negotiate_ack_delay_time)) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_negotiate_ack_delay_time, 4, 4); - max_ack_delay_ms_.SetReceivedValue(params.max_ack_delay.value()); - } - if (params.ack_delay_exponent.IsValid()) { - ack_delay_exponent_.SetReceivedValue(params.ack_delay_exponent.value()); - } - if (params.disable_migration) { - connection_migration_disabled_.SetReceivedValue(1u); + if (!is_resumption) { + if (GetQuicReloadableFlag(quic_negotiate_ack_delay_time)) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_negotiate_ack_delay_time, 4, 4); + max_ack_delay_ms_.SetReceivedValue(params.max_ack_delay.value()); + } + if (params.ack_delay_exponent.IsValid()) { + ack_delay_exponent_.SetReceivedValue(params.ack_delay_exponent.value()); + } + if (params.preferred_address != nullptr) { + if (params.preferred_address->ipv6_socket_address.port() != 0) { + alternate_server_address_ipv6_.SetReceivedValue( + params.preferred_address->ipv6_socket_address); + } + if (params.preferred_address->ipv4_socket_address.port() != 0) { + alternate_server_address_ipv4_.SetReceivedValue( + params.preferred_address->ipv4_socket_address); + } + } } - if (params.preferred_address != nullptr) { - if (params.preferred_address->ipv6_socket_address.port() != 0) { - alternate_server_address_ipv6_.SetReceivedValue( - params.preferred_address->ipv6_socket_address); - } - if (params.preferred_address->ipv4_socket_address.port() != 0) { - alternate_server_address_ipv4_.SetReceivedValue( - params.preferred_address->ipv4_socket_address); - } + if (params.disable_migration) { + connection_migration_disabled_.SetReceivedValue(1u); } const CryptoHandshakeMessage* peer_params = params.google_quic_params.get(); @@ -1206,7 +1209,9 @@ received_custom_transport_parameters_ = params.custom_parameters; - negotiated_ = true; + if (!is_resumption) { + negotiated_ = true; + } *error_details = ""; return QUIC_NO_ERROR; }
diff --git a/quic/core/quic_config.h b/quic/core/quic_config.h index a968369..091e5de 100644 --- a/quic/core/quic_config.h +++ b/quic/core/quic_config.h
@@ -460,10 +460,13 @@ // ProcessTransportParameters reads from |params| which was received from a // peer operating as a |hello_type|. It processes values for ICSL, MIDS, CFCW, - // and SFCW and sets the corresponding members of this QuicConfig. On failure, - // it returns a QuicErrorCode and puts a detailed error in |*error_details|. + // and SFCW and sets the corresponding members of this QuicConfig. + // If |is_resumption|, some configs will not be processed. + // On failure, it returns a QuicErrorCode and puts a detailed error in + // |*error_details|. QuicErrorCode ProcessTransportParameters(const TransportParameters& params, HelloType hello_type, + bool is_resumption, std::string* error_details); TransportParameters::ParameterMap& custom_transport_parameters_to_send() {
diff --git a/quic/core/quic_config_test.cc b/quic/core/quic_config_test.cc index 8671d7e..dbdd0f8 100644 --- a/quic/core/quic_config_test.cc +++ b/quic/core/quic_config_test.cc
@@ -12,6 +12,7 @@ #include "net/third_party/quiche/src/quic/core/quic_time.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" #include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" #include "net/third_party/quiche/src/quic/platform/api/quic_uint128.h" #include "net/third_party/quiche/src/quic/test_tools/quic_config_peer.h" @@ -23,6 +24,19 @@ const uint32_t kMaxPacketSizeForTest = 1234; const uint32_t kMaxDatagramFrameSizeForTest = 1333; +const uint8_t kFakeStatelessResetTokenData[16] = { + 0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97, + 0x98, 0x99, 0x9A, 0x9B, 0x9C, 0x9D, 0x9E, 0x9F}; +const uint64_t kFakeAckDelayExponent = 10; +const uint64_t kFakeMaxAckDelay = 51; + +// TODO(b/153726130): Consider merging this with methods in +// transport_parameters_test.cc. +std::vector<uint8_t> CreateFakeStatelessResetToken() { + return std::vector<uint8_t>( + kFakeStatelessResetTokenData, + kFakeStatelessResetTokenData + sizeof(kFakeStatelessResetTokenData)); +} class QuicConfigTest : public QuicTestWithParam<ParsedQuicVersion> { public: @@ -430,9 +444,9 @@ params.idle_timeout_milliseconds.set_value(120000); std::string error_details = "foobar"; - EXPECT_THAT( - config_.ProcessTransportParameters(params, SERVER, &error_details), - IsQuicNoError()); + EXPECT_THAT(config_.ProcessTransportParameters( + params, SERVER, /* is_resumption = */ false, &error_details), + IsQuicNoError()); EXPECT_EQ("", error_details); EXPECT_EQ(quic::QuicTime::Delta::FromSeconds(60), config_.IdleNetworkTimeout()); @@ -475,6 +489,7 @@ // TransportParameters are only used for QUIC+TLS. return; } + SetQuicReloadableFlag(quic_negotiate_ack_delay_time, true); TransportParameters params; params.initial_max_stream_data_bidi_local.set_value( @@ -486,13 +501,18 @@ params.max_packet_size.set_value(kMaxPacketSizeForTest); params.max_datagram_frame_size.set_value(kMaxDatagramFrameSizeForTest); params.initial_max_streams_bidi.set_value(kDefaultMaxStreamsPerConnection); + params.stateless_reset_token = CreateFakeStatelessResetToken(); + params.max_ack_delay.set_value(kFakeMaxAckDelay); + params.ack_delay_exponent.set_value(kFakeAckDelayExponent); std::string error_details; - EXPECT_THAT( - config_.ProcessTransportParameters(params, SERVER, &error_details), - IsQuicNoError()) + EXPECT_THAT(config_.ProcessTransportParameters( + params, SERVER, /* is_resumption = */ true, &error_details), + IsQuicNoError()) << error_details; + EXPECT_FALSE(config_.negotiated()); + ASSERT_TRUE( config_.HasReceivedInitialMaxStreamDataBytesIncomingBidirectional()); EXPECT_EQ(2 * kMinimumFlowControlSendWindow, @@ -520,6 +540,11 @@ EXPECT_FALSE(config_.DisableConnectionMigration()); + // The following config shouldn't be processed because of resumption. + EXPECT_FALSE(config_.HasReceivedStatelessResetToken()); + EXPECT_FALSE(config_.HasReceivedMaxAckDelayMs()); + EXPECT_FALSE(config_.HasReceivedAckDelayExponent()); + // Let the config process another slightly tweaked transport paramters. // Note that the values for flow control and stream limit cannot be smaller // than before. This rule is enforced in QuicSession::OnConfigNegotiated(). @@ -535,11 +560,13 @@ kDefaultMaxStreamsPerConnection); params.disable_migration = true; - EXPECT_THAT( - config_.ProcessTransportParameters(params, SERVER, &error_details), - IsQuicNoError()) + EXPECT_THAT(config_.ProcessTransportParameters( + params, SERVER, /* is_resumption = */ false, &error_details), + IsQuicNoError()) << error_details; + EXPECT_TRUE(config_.negotiated()); + ASSERT_TRUE( config_.HasReceivedInitialMaxStreamDataBytesIncomingBidirectional()); EXPECT_EQ(2 * kMinimumFlowControlSendWindow + 1, @@ -566,6 +593,13 @@ config_.ReceivedMaxBidirectionalStreams()); EXPECT_TRUE(config_.DisableConnectionMigration()); + + ASSERT_TRUE(config_.HasReceivedStatelessResetToken()); + ASSERT_TRUE(config_.HasReceivedMaxAckDelayMs()); + EXPECT_EQ(config_.ReceivedMaxAckDelayMs(), kFakeMaxAckDelay); + + ASSERT_TRUE(config_.HasReceivedAckDelayExponent()); + EXPECT_EQ(config_.ReceivedAckDelayExponent(), kFakeAckDelayExponent); } TEST_P(QuicConfigTest, DisableMigrationTransportParameter) { @@ -576,9 +610,9 @@ TransportParameters params; params.disable_migration = true; std::string error_details; - EXPECT_THAT( - config_.ProcessTransportParameters(params, SERVER, &error_details), - IsQuicNoError()); + EXPECT_THAT(config_.ProcessTransportParameters( + params, SERVER, /* is_resumption = */ false, &error_details), + IsQuicNoError()); EXPECT_TRUE(config_.DisableConnectionMigration()); }
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index 8c6e9dd..9d5e977 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -83,7 +83,8 @@ EXPECT_TRUE( session()->config()->FillTransportParameters(&transport_parameters)); error = session()->config()->ProcessTransportParameters( - transport_parameters, CLIENT, &error_details); + transport_parameters, CLIENT, /* is_resumption = */ false, + &error_details); } else { CryptoHandshakeMessage msg; session()->config()->ToHandshakeMessage(&msg, transport_version());
diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc index 09ab71e..c638040 100644 --- a/quic/core/tls_client_handshaker.cc +++ b/quic/core/tls_client_handshaker.cc
@@ -226,8 +226,8 @@ session()->connection()->server_supported_versions(), error_details) != QUIC_NO_ERROR || session()->config()->ProcessTransportParameters( - *received_transport_params_, SERVER, error_details) != - QUIC_NO_ERROR) { + *received_transport_params_, SERVER, /* is_resumption = */ false, + error_details) != QUIC_NO_ERROR) { DCHECK(!error_details->empty()); return false; }
diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc index 54a066f..229e568 100644 --- a/quic/core/tls_server_handshaker.cc +++ b/quic/core/tls_server_handshaker.cc
@@ -295,7 +295,8 @@ client_params.version, session()->connection()->version(), session()->supported_versions(), error_details) != QUIC_NO_ERROR || session()->config()->ProcessTransportParameters( - client_params, CLIENT, error_details) != QUIC_NO_ERROR) { + client_params, CLIENT, /* is_resumption = */ false, error_details) != + QUIC_NO_ERROR) { return false; } ProcessAdditionalTransportParameters(client_params);