Unblock quic_stop_sending_legacy_version_info We realized that a few tests were failing when the values of quic_stop_sending_legacy_version_info and quic_stop_parsing_legacy_version_info were different, so quic_stop_sending_legacy_version_info was blocked in cl/853491571. This CL fixes the test issue and reverts cl/853491571. We confirmed that tests pass by running all four combinations via: ``` blaze test //third_party/quic/core/crypto:transport_parameters_test --test_arg=--gfe2_restart_flag_quic_stop_parsing_legacy_version_info=true --test_arg=--gfe2_restart_flag_quic_stop_sending_legacy_version_info=true && blaze test //third_party/quic/core/crypto:transport_parameters_test --test_arg=--gfe2_restart_flag_quic_stop_parsing_legacy_version_info=true --test_arg=--gfe2_restart_flag_quic_stop_sending_legacy_version_info=false && blaze test //third_party/quic/core/crypto:transport_parameters_test --test_arg=--gfe2_restart_flag_quic_stop_parsing_legacy_version_info=false --test_arg=--gfe2_restart_flag_quic_stop_sending_legacy_version_info=true && blaze test //third_party/quic/core/crypto:transport_parameters_test --test_arg=--gfe2_restart_flag_quic_stop_parsing_legacy_version_info=false --test_arg=--gfe2_restart_flag_quic_stop_sending_legacy_version_info=false ``` PiperOrigin-RevId: 855471392
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h index 9383d97..47b267c 100755 --- a/quiche/common/quiche_feature_flags_list.h +++ b/quiche/common/quiche_feature_flags_list.h
@@ -72,7 +72,7 @@ QUICHE_FLAG(bool, quiche_restart_flag_quic_dispatcher_close_connection_on_invalid_ack, false, false, "An invalid ack is an ack that the peer sent for a packet that was not sent by the dispatcher. If true, the dispatcher will close the connection if it receives an invalid ack.") QUICHE_FLAG(bool, quiche_restart_flag_quic_shed_tls_handshake_config, false, false, "If true, QUIC connections will call SSL_set_shed_handshake_config to drop BoringSSL handshake state after the handshake finishes in order to save memory.") QUICHE_FLAG(bool, quiche_restart_flag_quic_stop_parsing_legacy_version_info, false, false, "If true, disable parsing the legacy version information transport parameter.") -QUICHE_FLAG(bool, quiche_restart_flag_quic_stop_sending_legacy_version_info, false, false, "If true, disable sending the legacy version information transport parameter.") +QUICHE_FLAG(bool, quiche_restart_flag_quic_stop_sending_legacy_version_info, false, true, "If true, disable sending the legacy version information transport parameter.") QUICHE_FLAG(bool, quiche_restart_flag_quic_support_release_time_for_gso, false, false, "If true, QuicGsoBatchWriter will support release time if it is available and the process has the permission to do so.") QUICHE_FLAG(bool, quiche_restart_flag_quic_testonly_default_false, false, false, "A testonly restart flag that will always default to false.") QUICHE_FLAG(bool, quiche_restart_flag_quic_testonly_default_true, true, true, "A testonly restart flag that will always default to true.")
diff --git a/quiche/quic/core/crypto/transport_parameters_test.cc b/quiche/quic/core/crypto/transport_parameters_test.cc index 7649813..6004f11 100644 --- a/quiche/quic/core/crypto/transport_parameters_test.cc +++ b/quiche/quic/core/crypto/transport_parameters_test.cc
@@ -52,6 +52,8 @@ const char* kCustomParameter1Value = "foo"; const auto kCustomParameter2 = static_cast<TransportParameters::TransportParameterId>(0xff34); +const auto kLegacyVersionInfoParameter = + static_cast<TransportParameters::TransportParameterId>(0x4752); const char* kCustomParameter2Value = "bar"; const char kFakeGoogleHandshakeMessage[] = @@ -325,8 +327,7 @@ TEST_P(TransportParametersTest, RoundTripClient) { TransportParameters orig_params; orig_params.perspective = Perspective::IS_CLIENT; - if (!GetQuicRestartFlag(quic_stop_parsing_legacy_version_info) || - !GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { + if (!GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { orig_params.legacy_version_information = CreateFakeLegacyVersionInformationClient(); } @@ -373,14 +374,18 @@ << error_details; EXPECT_TRUE(error_details.empty()); RemoveGreaseParameters(&new_params); + if (GetQuicRestartFlag(quic_stop_parsing_legacy_version_info) && + !GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { + orig_params.legacy_version_information.reset(); + new_params.custom_parameters.erase(kLegacyVersionInfoParameter); + } EXPECT_EQ(new_params, orig_params); } TEST_P(TransportParametersTest, RoundTripServer) { TransportParameters orig_params; orig_params.perspective = Perspective::IS_SERVER; - if (!GetQuicRestartFlag(quic_stop_parsing_legacy_version_info) || - !GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { + if (!GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { orig_params.legacy_version_information = CreateFakeLegacyVersionInformationServer(); } @@ -423,6 +428,11 @@ << error_details; EXPECT_TRUE(error_details.empty()); RemoveGreaseParameters(&new_params); + if (GetQuicRestartFlag(quic_stop_parsing_legacy_version_info) && + !GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { + orig_params.legacy_version_information.reset(); + new_params.custom_parameters.erase(kLegacyVersionInfoParameter); + } EXPECT_EQ(new_params, orig_params); } @@ -654,8 +664,7 @@ << error_details; EXPECT_TRUE(error_details.empty()); EXPECT_EQ(Perspective::IS_CLIENT, new_params.perspective); - if (!GetQuicRestartFlag(quic_stop_parsing_legacy_version_info) || - !GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { + if (!GetQuicRestartFlag(quic_stop_parsing_legacy_version_info)) { ASSERT_TRUE(new_params.legacy_version_information.has_value()); EXPECT_EQ(kFakeVersionLabel, new_params.legacy_version_information.value().version); @@ -921,8 +930,7 @@ << error_details; EXPECT_TRUE(error_details.empty()); EXPECT_EQ(Perspective::IS_SERVER, new_params.perspective); - if (!GetQuicRestartFlag(quic_stop_parsing_legacy_version_info) || - !GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { + if (!GetQuicRestartFlag(quic_stop_parsing_legacy_version_info)) { ASSERT_TRUE(new_params.legacy_version_information.has_value()); EXPECT_EQ(kFakeVersionLabel, new_params.legacy_version_information.value().version); @@ -1055,8 +1063,7 @@ std::string custom_value(70000, '?'); TransportParameters orig_params; orig_params.perspective = Perspective::IS_CLIENT; - if (!GetQuicRestartFlag(quic_stop_parsing_legacy_version_info) || - !GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { + if (!GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { orig_params.legacy_version_information = CreateFakeLegacyVersionInformationClient(); } @@ -1073,6 +1080,11 @@ << error_details; EXPECT_TRUE(error_details.empty()); RemoveGreaseParameters(&new_params); + if (GetQuicRestartFlag(quic_stop_parsing_legacy_version_info) && + !GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { + orig_params.legacy_version_information.reset(); + new_params.custom_parameters.erase(kLegacyVersionInfoParameter); + } EXPECT_EQ(new_params, orig_params); } @@ -1126,8 +1138,7 @@ TEST_P(TransportParametersTest, Degrease) { TransportParameters orig_params; orig_params.perspective = Perspective::IS_CLIENT; - if (!GetQuicRestartFlag(quic_stop_parsing_legacy_version_info) || - !GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { + if (!GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { orig_params.legacy_version_information = CreateFakeLegacyVersionInformationClient(); } @@ -1178,6 +1189,11 @@ EXPECT_NE(new_params, orig_params); DegreaseTransportParameters(new_params); + if (GetQuicRestartFlag(quic_stop_parsing_legacy_version_info) && + !GetQuicRestartFlag(quic_stop_sending_legacy_version_info)) { + orig_params.legacy_version_information.reset(); + new_params.custom_parameters.erase(kLegacyVersionInfoParameter); + } EXPECT_EQ(new_params, orig_params); }