Deprecate quic_google_transport_param_omit_old
Deprecate gfe2_restart_flag_quic_google_transport_param_omit_old
PiperOrigin-RevId: 327087161
Change-Id: I10cd15ce6ad6e59d230198fbf10c805ec4fc9361
diff --git a/quic/core/crypto/transport_parameters.cc b/quic/core/crypto/transport_parameters.cc
index 60f6c26..71a9dfb 100644
--- a/quic/core/crypto/transport_parameters.cc
+++ b/quic/core/crypto/transport_parameters.cc
@@ -12,8 +12,6 @@
#include "third_party/boringssl/src/include/openssl/digest.h"
#include "third_party/boringssl/src/include/openssl/sha.h"
-#include "net/third_party/quiche/src/quic/core/crypto/crypto_framer.h"
-#include "net/third_party/quiche/src/quic/core/crypto/crypto_handshake_message.h"
#include "net/third_party/quiche/src/quic/core/quic_connection_id.h"
#include "net/third_party/quiche/src/quic/core/quic_data_reader.h"
#include "net/third_party/quiche/src/quic/core/quic_data_writer.h"
@@ -58,7 +56,8 @@
kGoogleUserAgentId = 0x3129,
kGoogleSupportHandshakeDone = 0x312A, // Only used in T050.
kGoogleKeyUpdateNotYetSupported = 0x312B,
- kGoogleQuicParam = 0x4751, // Used for non-standard Google-specific params.
+ // 0x4751 was used for non-standard Google-specific parameters encoded as a
+ // Google QUIC_CRYPTO CHLO, it has been replaced by individual parameters.
kGoogleQuicVersion =
0x4752, // Used to transmit version and supported_versions.
@@ -128,8 +127,6 @@
return "support_handshake_done";
case TransportParameters::kGoogleKeyUpdateNotYetSupported:
return "key_update_not_yet_supported";
- case TransportParameters::kGoogleQuicParam:
- return "google";
case TransportParameters::kGoogleQuicVersion:
return "google-version";
case TransportParameters::kMinAckDelay:
@@ -164,7 +161,6 @@
case TransportParameters::kGoogleUserAgentId:
case TransportParameters::kGoogleSupportHandshakeDone:
case TransportParameters::kGoogleKeyUpdateNotYetSupported:
- case TransportParameters::kGoogleQuicParam:
case TransportParameters::kGoogleQuicVersion:
case TransportParameters::kMinAckDelay:
return true;
@@ -485,9 +481,6 @@
if (key_update_not_yet_supported) {
rv += " " + TransportParameterIdToString(kGoogleKeyUpdateNotYetSupported);
}
- if (google_quic_params) {
- rv += " " + TransportParameterIdToString(kGoogleQuicParam);
- }
for (const auto& kv : custom_parameters) {
rv += " 0x" + quiche::QuicheTextUtils::Hex(static_cast<uint32_t>(kv.first));
rv += "=";
@@ -580,10 +573,6 @@
preferred_address = std::make_unique<TransportParameters::PreferredAddress>(
*other.preferred_address);
}
- if (other.google_quic_params) {
- google_quic_params =
- std::make_unique<CryptoHandshakeMessage>(*other.google_quic_params);
- }
}
bool TransportParameters::operator==(const TransportParameters& rhs) const {
@@ -626,21 +615,15 @@
}
if ((!preferred_address && rhs.preferred_address) ||
- (preferred_address && !rhs.preferred_address) ||
- (!google_quic_params && rhs.google_quic_params) ||
- (google_quic_params && !rhs.google_quic_params)) {
+ (preferred_address && !rhs.preferred_address)) {
return false;
}
- bool address = true;
- if (preferred_address && rhs.preferred_address) {
- address = (*preferred_address == *rhs.preferred_address);
+ if (preferred_address && rhs.preferred_address &&
+ *preferred_address != *rhs.preferred_address) {
+ return false;
}
- bool google_quic = true;
- if (google_quic_params && rhs.google_quic_params) {
- google_quic = (*google_quic_params == *rhs.google_quic_params);
- }
- return address && google_quic;
+ return true;
}
bool TransportParameters::operator!=(const TransportParameters& rhs) const {
@@ -783,7 +766,6 @@
kTypeAndValueLength + // user_agent_id
kTypeAndValueLength + // support_handshake_done
kTypeAndValueLength + // key_update_not_yet_supported
- kTypeAndValueLength + // google
kTypeAndValueLength + // google-version
kGreaseParameterLength; // GREASE
@@ -805,11 +787,6 @@
for (const auto& kv : in.custom_parameters) {
max_transport_param_length += kTypeAndValueLength + kv.second.length();
}
- // Google-specific non-standard parameter.
- if (in.google_quic_params) {
- max_transport_param_length +=
- in.google_quic_params->GetSerialized().length();
- }
out->resize(max_transport_param_length);
QuicDataWriter writer(out->size(), reinterpret_cast<char*>(out->data()));
@@ -1027,20 +1004,6 @@
}
}
- // Google-specific non-standard parameter.
- if (in.google_quic_params) {
- const QuicData& serialized_google_quic_params =
- in.google_quic_params->GetSerialized();
- if (!WriteTransportParameterId(
- &writer, TransportParameters::kGoogleQuicParam, version) ||
- !WriteTransportParameterStringPiece(
- &writer, serialized_google_quic_params.AsStringPiece(), version)) {
- QUIC_BUG << "Failed to write Google params of length "
- << serialized_google_quic_params.length() << " for " << in;
- return false;
- }
- }
-
// Google-specific version extension.
static_assert(sizeof(QuicVersionLabel) == sizeof(uint32_t), "bad length");
uint64_t google_version_length = sizeof(in.version);
@@ -1402,14 +1365,6 @@
}
out->key_update_not_yet_supported = true;
break;
- case TransportParameters::kGoogleQuicParam: {
- if (out->google_quic_params) {
- *error_details = "Received a second Google parameter";
- return false;
- }
- out->google_quic_params =
- CryptoFramer::ParseMessage(value_reader.ReadRemainingPayload());
- } break;
case TransportParameters::kGoogleQuicVersion: {
if (!value_reader.ReadUInt32(&out->version)) {
*error_details = "Failed to read Google version extension version";
diff --git a/quic/core/crypto/transport_parameters.h b/quic/core/crypto/transport_parameters.h
index 8034dd1..bf8c607 100644
--- a/quic/core/crypto/transport_parameters.h
+++ b/quic/core/crypto/transport_parameters.h
@@ -8,13 +8,14 @@
#include <memory>
#include <vector>
-#include "net/third_party/quiche/src/quic/core/crypto/crypto_handshake_message.h"
#include "net/third_party/quiche/src/quic/core/quic_connection_id.h"
#include "net/third_party/quiche/src/quic/core/quic_data_reader.h"
#include "net/third_party/quiche/src/quic/core/quic_data_writer.h"
+#include "net/third_party/quiche/src/quic/core/quic_tag.h"
#include "net/third_party/quiche/src/quic/core/quic_types.h"
#include "net/third_party/quiche/src/quic/core/quic_versions.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_containers.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_socket_address.h"
#include "net/third_party/quiche/src/common/platform/api/quiche_optional.h"
#include "net/third_party/quiche/src/common/platform/api/quiche_string_piece.h"
@@ -213,11 +214,6 @@
// yet been implemented. This will be removed once we implement it.
bool key_update_not_yet_supported;
- // Transport parameters used by Google QUIC but not IETF QUIC. This is
- // serialized into a TransportParameter struct with a TransportParameterId of
- // kGoogleQuicParamId.
- std::unique_ptr<CryptoHandshakeMessage> google_quic_params;
-
// Validates whether transport parameters are valid according to
// the specification. If the transport parameters are not valid, this method
// will write a human-readable error message to |error_details|.
diff --git a/quic/core/crypto/transport_parameters_test.cc b/quic/core/crypto/transport_parameters_test.cc
index a87ff1b..0578e29 100644
--- a/quic/core/crypto/transport_parameters_test.cc
+++ b/quic/core/crypto/transport_parameters_test.cc
@@ -189,27 +189,6 @@
EXPECT_TRUE(orig_params == new_params);
EXPECT_FALSE(orig_params != new_params);
- // Test comparison on CryptoHandshakeMessage.
- orig_params.google_quic_params = std::make_unique<CryptoHandshakeMessage>();
- const std::string kTestString = "test string";
- orig_params.google_quic_params->SetStringPiece(42, kTestString);
- const uint32_t kTestValue = 12;
- orig_params.google_quic_params->SetValue(1337, kTestValue);
- EXPECT_NE(orig_params, new_params);
- EXPECT_FALSE(orig_params == new_params);
- EXPECT_TRUE(orig_params != new_params);
-
- new_params.google_quic_params = std::make_unique<CryptoHandshakeMessage>();
- new_params.google_quic_params->SetStringPiece(42, kTestString);
- new_params.google_quic_params->SetValue(1337, kTestValue + 1);
- EXPECT_NE(orig_params, new_params);
- EXPECT_FALSE(orig_params == new_params);
- EXPECT_TRUE(orig_params != new_params);
- new_params.google_quic_params->SetValue(1337, kTestValue);
- EXPECT_EQ(orig_params, new_params);
- EXPECT_TRUE(orig_params == new_params);
- EXPECT_FALSE(orig_params != new_params);
-
// Test comparison on CustomMap
orig_params.custom_parameters[kCustomParameter1] = kCustomParameter1Value;
orig_params.custom_parameters[kCustomParameter2] = kCustomParameter2Value;
@@ -1246,42 +1225,6 @@
EmptyQuicConnectionId());
}
-TEST_P(TransportParametersTest, CryptoHandshakeMessageRoundtrip) {
- TransportParameters orig_params;
- orig_params.perspective = Perspective::IS_CLIENT;
- orig_params.version = kFakeVersionLabel;
- orig_params.max_udp_payload_size.set_value(kMaxPacketSizeForTest);
-
- orig_params.google_quic_params = std::make_unique<CryptoHandshakeMessage>();
- const std::string kTestString = "test string";
- orig_params.google_quic_params->SetStringPiece(42, kTestString);
- const uint32_t kTestValue = 12;
- orig_params.google_quic_params->SetValue(1337, kTestValue);
-
- std::vector<uint8_t> serialized;
- ASSERT_TRUE(SerializeTransportParameters(version_, orig_params, &serialized));
-
- TransportParameters new_params;
- std::string error_details;
- ASSERT_TRUE(ParseTransportParameters(version_, Perspective::IS_CLIENT,
- serialized.data(), serialized.size(),
- &new_params, &error_details))
- << error_details;
- EXPECT_TRUE(error_details.empty());
- ASSERT_NE(new_params.google_quic_params.get(), nullptr);
- EXPECT_EQ(new_params.google_quic_params->tag(),
- orig_params.google_quic_params->tag());
- quiche::QuicheStringPiece test_string;
- EXPECT_TRUE(new_params.google_quic_params->GetStringPiece(42, &test_string));
- EXPECT_EQ(test_string, kTestString);
- uint32_t test_value;
- EXPECT_THAT(new_params.google_quic_params->GetUint32(1337, &test_value),
- IsQuicNoError());
- EXPECT_EQ(test_value, kTestValue);
- EXPECT_EQ(kFakeVersionLabel, new_params.version);
- EXPECT_EQ(kMaxPacketSizeForTest, new_params.max_udp_payload_size.value());
-}
-
TEST_P(TransportParametersTest, VeryLongCustomParameter) {
// Ensure we can handle a 70KB custom parameter on both send and receive.
size_t custom_value_length = 70000;
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 189deac..706ee2d 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -109,8 +109,7 @@
EXPECT_TRUE(
session()->config()->FillTransportParameters(&transport_parameters));
error = session()->config()->ProcessTransportParameters(
- transport_parameters, CLIENT, /* is_resumption = */ false,
- &error_details);
+ transport_parameters, /* 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 c9ed83c..81de165 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -95,8 +95,7 @@
EXPECT_TRUE(
session()->config()->FillTransportParameters(&transport_parameters));
error = session()->config()->ProcessTransportParameters(
- transport_parameters, CLIENT, /* is_resumption = */ false,
- &error_details);
+ transport_parameters, /* 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 0fd26a9..23597e2 100644
--- a/quic/core/quic_config.cc
+++ b/quic/core/quic_config.cc
@@ -1228,17 +1228,6 @@
params->google_connection_options = connection_options_.GetSendValues();
}
- if (!GetQuicRestartFlag(quic_google_transport_param_omit_old)) {
- if (!params->google_quic_params) {
- params->google_quic_params = std::make_unique<CryptoHandshakeMessage>();
- }
- initial_round_trip_time_us_.ToHandshakeMessage(
- params->google_quic_params.get());
- connection_options_.ToHandshakeMessage(params->google_quic_params.get());
- } else {
- QUIC_RESTART_FLAG_COUNT_N(quic_google_transport_param_omit_old, 1, 3);
- }
-
if (GetQuicReloadableFlag(quic_send_key_update_not_yet_supported)) {
QUIC_RELOADABLE_FLAG_COUNT(quic_send_key_update_not_yet_supported);
params->key_update_not_yet_supported = true;
@@ -1251,7 +1240,6 @@
QuicErrorCode QuicConfig::ProcessTransportParameters(
const TransportParameters& params,
- HelloType hello_type,
bool is_resumption,
std::string* error_details) {
if (!is_resumption && params.original_destination_connection_id.has_value()) {
@@ -1377,26 +1365,6 @@
params.google_connection_options.value());
}
- if (!GetQuicRestartFlag(quic_google_transport_param_omit_old)) {
- const CryptoHandshakeMessage* peer_params = params.google_quic_params.get();
- if (peer_params != nullptr && !google_params_already_parsed) {
- QuicErrorCode error = initial_round_trip_time_us_.ProcessPeerHello(
- *peer_params, hello_type, error_details);
- if (error != QUIC_NO_ERROR) {
- DCHECK(!error_details->empty());
- return error;
- }
- error = connection_options_.ProcessPeerHello(*peer_params, hello_type,
- error_details);
- if (error != QUIC_NO_ERROR) {
- DCHECK(!error_details->empty());
- return error;
- }
- }
- } else {
- QUIC_RESTART_FLAG_COUNT_N(quic_google_transport_param_omit_old, 2, 3);
- }
-
received_custom_transport_parameters_ = params.custom_parameters;
if (!is_resumption) {
diff --git a/quic/core/quic_config.h b/quic/core/quic_config.h
index 1c329dc..4132987 100644
--- a/quic/core/quic_config.h
+++ b/quic/core/quic_config.h
@@ -485,14 +485,11 @@
// if something prevents them from being written (e.g. a value is too large).
bool FillTransportParameters(TransportParameters* params) const;
- // 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.
- // If |is_resumption|, some configs will not be processed.
+ // ProcessTransportParameters reads from |params| which were received from a
+ // peer. 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);
diff --git a/quic/core/quic_config_test.cc b/quic/core/quic_config_test.cc
index 9a4d951..dad2423 100644
--- a/quic/core/quic_config_test.cc
+++ b/quic/core/quic_config_test.cc
@@ -422,7 +422,7 @@
std::string error_details = "foobar";
EXPECT_THAT(config_.ProcessTransportParameters(
- params, SERVER, /* is_resumption = */ false, &error_details),
+ params, /* is_resumption = */ false, &error_details),
IsQuicNoError());
EXPECT_EQ("", error_details);
EXPECT_EQ(quic::QuicTime::Delta::FromSeconds(60),
@@ -441,14 +441,14 @@
params.min_ack_delay_us.set_value(25 * kNumMicrosPerMilli + 1);
std::string error_details = "foobar";
EXPECT_THAT(config_.ProcessTransportParameters(
- params, SERVER, /* is_resumption = */ false, &error_details),
+ params, /* is_resumption = */ false, &error_details),
IsError(IETF_QUIC_PROTOCOL_VIOLATION));
EXPECT_EQ("MinAckDelay is greater than MaxAckDelay.", error_details);
params.max_ack_delay.set_value(25 /*ms*/);
params.min_ack_delay_us.set_value(25 * kNumMicrosPerMilli);
EXPECT_THAT(config_.ProcessTransportParameters(
- params, SERVER, /* is_resumption = */ false, &error_details),
+ params, /* is_resumption = */ false, &error_details),
IsQuicNoError());
EXPECT_TRUE(error_details.empty());
}
@@ -536,7 +536,7 @@
std::string error_details;
EXPECT_THAT(config_.ProcessTransportParameters(
- params, SERVER, /* is_resumption = */ true, &error_details),
+ params, /* is_resumption = */ true, &error_details),
IsQuicNoError())
<< error_details;
@@ -596,7 +596,7 @@
params.support_handshake_done = true;
EXPECT_THAT(config_.ProcessTransportParameters(
- params, SERVER, /* is_resumption = */ false, &error_details),
+ params, /* is_resumption = */ false, &error_details),
IsQuicNoError())
<< error_details;
@@ -669,7 +669,7 @@
params.disable_active_migration = true;
std::string error_details;
EXPECT_THAT(config_.ProcessTransportParameters(
- params, SERVER, /* is_resumption = */ false, &error_details),
+ params, /* is_resumption = */ false, &error_details),
IsQuicNoError());
EXPECT_TRUE(config_.DisableConnectionMigration());
}
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index cb55eb0..73c371d 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -1680,13 +1680,7 @@
const TransportParameters& params,
bool is_resumption,
std::string* error_details) {
- HelloType hello_type;
- if (perspective_ == Perspective::IS_CLIENT) {
- hello_type = SERVER;
- } else {
- hello_type = CLIENT;
- }
- return config_.ProcessTransportParameters(params, hello_type, is_resumption,
+ return config_.ProcessTransportParameters(params, is_resumption,
error_details);
}
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index e8dbce5..f9dd5ac 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -96,8 +96,7 @@
EXPECT_TRUE(
session()->config()->FillTransportParameters(&transport_parameters));
error = session()->config()->ProcessTransportParameters(
- transport_parameters, CLIENT, /* is_resumption = */ false,
- &error_details);
+ transport_parameters, /* 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 5465fc6..093e2dc 100644
--- a/quic/core/tls_client_handshaker.cc
+++ b/quic/core/tls_client_handshaker.cc
@@ -206,9 +206,6 @@
if (!user_agent_id_.empty()) {
params.user_agent_id = user_agent_id_;
}
- if (!GetQuicRestartFlag(quic_google_transport_param_omit_old)) {
- params.google_quic_params->SetStringPiece(kUAID, user_agent_id_);
- }
// Notify QuicConnectionDebugVisitor.
session()->connection()->OnTransportParametersSent(params);
diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc
index e8024e0..66d8cc3 100644
--- a/quic/core/tls_server_handshaker.cc
+++ b/quic/core/tls_server_handshaker.cc
@@ -303,16 +303,9 @@
return false;
}
ProcessAdditionalTransportParameters(client_params);
- if (!session()->user_agent_id().has_value()) {
- if (client_params.user_agent_id.has_value()) {
- session()->SetUserAgentId(client_params.user_agent_id.value());
- } else if (client_params.google_quic_params) {
- quiche::QuicheStringPiece user_agent_id;
- client_params.google_quic_params->GetStringPiece(kUAID, &user_agent_id);
- if (!user_agent_id.empty()) {
- session()->SetUserAgentId(user_agent_id.data());
- }
- }
+ if (!session()->user_agent_id().has_value() &&
+ client_params.user_agent_id.has_value()) {
+ session()->SetUserAgentId(client_params.user_agent_id.value());
}
return true;
