Use version-aware QuicConnectionCloseFrame constructor. Several places omitted using this constructor. They have been updated to use it. gfe-relnote: N/A just refactors a constructor. PiperOrigin-RevId: 268329266 Change-Id: Ib9edbc826690aed524d8888ae7893eed5c2006da
diff --git a/quic/core/frames/quic_connection_close_frame.cc b/quic/core/frames/quic_connection_close_frame.cc index 8f1fe24..88c1109 100644 --- a/quic/core/frames/quic_connection_close_frame.cc +++ b/quic/core/frames/quic_connection_close_frame.cc
@@ -10,48 +10,13 @@ #include "net/third_party/quiche/src/quic/platform/api/quic_str_cat.h" namespace quic { - QuicConnectionCloseFrame::QuicConnectionCloseFrame() // Default close type ensures that existing, pre-V99 code works as expected. : close_type(GOOGLE_QUIC_CONNECTION_CLOSE), quic_error_code(QUIC_NO_ERROR), - extracted_error_code(QUIC_IETF_GQUIC_ERROR_MISSING), + extracted_error_code(QUIC_NO_ERROR), transport_close_frame_type(0) {} -QuicConnectionCloseFrame::QuicConnectionCloseFrame(QuicErrorCode error_code, - std::string error_details) - // Default close type ensures that existing, pre-V99 code works as expected. - : close_type(GOOGLE_QUIC_CONNECTION_CLOSE), - quic_error_code(error_code), - extracted_error_code(error_code), - error_details(std::move(error_details)), - transport_close_frame_type(0) {} - -QuicConnectionCloseFrame::QuicConnectionCloseFrame( - QuicErrorCode quic_error_code, - std::string error_details, - uint64_t ietf_application_error_code) - : close_type(IETF_QUIC_APPLICATION_CONNECTION_CLOSE), - application_error_code(ietf_application_error_code), - extracted_error_code(quic_error_code), - error_details(std::move(error_details)), - transport_close_frame_type(0) { - DCHECK_LE(ietf_application_error_code, kMaxIetfVarInt); -} - -QuicConnectionCloseFrame::QuicConnectionCloseFrame( - QuicErrorCode quic_error_code, - std::string error_details, - QuicIetfTransportErrorCodes transport_error_code, - uint64_t transport_frame_type) - : close_type(IETF_QUIC_TRANSPORT_CONNECTION_CLOSE), - transport_error_code(transport_error_code), - extracted_error_code(quic_error_code), - error_details(std::move(error_details)), - transport_close_frame_type(transport_frame_type) { - DCHECK_LE(transport_error_code, kMaxIetfVarInt); -} - QuicConnectionCloseFrame::QuicConnectionCloseFrame( QuicTransportVersion transport_version, QuicErrorCode error_code,
diff --git a/quic/core/frames/quic_connection_close_frame.h b/quic/core/frames/quic_connection_close_frame.h index 9b6fdd4..4ee41b9 100644 --- a/quic/core/frames/quic_connection_close_frame.h +++ b/quic/core/frames/quic_connection_close_frame.h
@@ -27,21 +27,6 @@ std::string error_phrase, uint64_t transport_close_frame_type); - // TODO(fkastenholz): After migration to supporting IETF QUIC, this probably - // should be deprecated. - QuicConnectionCloseFrame(QuicErrorCode error_code, std::string error_details); - - // Sets close_type to IETF_QUIC_APPLICATION_CONNECTION_CLOSE. - QuicConnectionCloseFrame(QuicErrorCode quic_error_code, - std::string error_details, - uint64_t ietf_application_error_code); - - // Sets close_type to IETF_QUIC_TRANSPORT_CONNECTION_CLOSE. - QuicConnectionCloseFrame(QuicErrorCode quic_error_code, - std::string error_details, - QuicIetfTransportErrorCodes transport_error_code, - uint64_t transport_frame_type); - friend QUIC_EXPORT_PRIVATE std::ostream& operator<<( std::ostream& os, const QuicConnectionCloseFrame& c);
diff --git a/quic/core/frames/quic_frames_test.cc b/quic/core/frames/quic_frames_test.cc index b1030ee..8492a0c 100644 --- a/quic/core/frames/quic_frames_test.cc +++ b/quic/core/frames/quic_frames_test.cc
@@ -148,7 +148,7 @@ // underlying frame. EXPECT_EQ( "{ Close type: GOOGLE_QUIC_CONNECTION_CLOSE, error_code: 25, " - "extracted_error_code: QUIC_IETF_GQUIC_ERROR_MISSING, " + "extracted_error_code: QUIC_NO_ERROR, " "error_details: 'No recent " "network activity.'" "}\n",
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index e85ec9d..87deb70 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -2875,7 +2875,8 @@ QuicErrorCode error, const std::string& error_details, ConnectionCloseSource source) { - QuicConnectionCloseFrame frame(error, error_details); + QuicConnectionCloseFrame frame(transport_version(), error, error_details, + framer_.current_received_frame_type()); return TearDownLocalConnectionState(frame, source); }
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc index ed52624..933b975 100644 --- a/quic/core/quic_framer_test.cc +++ b/quic/core/quic_framer_test.cc
@@ -7222,17 +7222,13 @@ header.version_flag = false; header.packet_number = kPacketNumber; - QuicConnectionCloseFrame close_frame; - if (VersionHasIetfQuicFrames(framer_.transport_version())) { - close_frame.transport_error_code = - static_cast<QuicIetfTransportErrorCodes>(0x11); - close_frame.transport_close_frame_type = 0x05; - close_frame.close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE; - } else { - close_frame.quic_error_code = static_cast<QuicErrorCode>(0x05060708); - } - close_frame.error_details = "because I can"; - + QuicConnectionCloseFrame close_frame( + framer_.transport_version(), + static_cast<QuicErrorCode>( + VersionHasIetfQuicFrames(framer_.transport_version()) ? 0x11 + : 0x05060708), + "because I can", 0x05); + close_frame.extracted_error_code = QUIC_IETF_GQUIC_ERROR_MISSING; QuicFrames frames = {QuicFrame(&close_frame)}; // clang-format off @@ -7327,23 +7323,16 @@ header.version_flag = false; header.packet_number = kPacketNumber; - QuicConnectionCloseFrame close_frame; - if (VersionHasIetfQuicFrames(framer_.transport_version())) { - close_frame.transport_error_code = - static_cast<QuicIetfTransportErrorCodes>(0x11); - close_frame.transport_close_frame_type = 0x05; - close_frame.close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE; - } else { - close_frame.quic_error_code = static_cast<QuicErrorCode>(0x05060708); - } + QuicConnectionCloseFrame close_frame( + framer_.transport_version(), + static_cast<QuicErrorCode>( + VersionHasIetfQuicFrames(framer_.transport_version()) ? 0x11 + : 0x05060708), + "because I can", 0x05); // Set this so that it is "there" for both Google QUIC and IETF QUIC // framing. It better not show up for Google QUIC! close_frame.extracted_error_code = static_cast<QuicErrorCode>(0x4567); - // For IETF QUIC this will be prefaced with "17767:" - // (17767 == 0x4567). - close_frame.error_details = "because I can"; - QuicFrames frames = {QuicFrame(&close_frame)}; // clang-format off @@ -7439,15 +7428,13 @@ header.version_flag = false; header.packet_number = kPacketNumber; - QuicConnectionCloseFrame close_frame; - if (VersionHasIetfQuicFrames(framer_.transport_version())) { - close_frame.transport_error_code = PROTOCOL_VIOLATION; // value is 0x0a - EXPECT_EQ(0u, close_frame.transport_close_frame_type); - close_frame.close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE; - } else { - close_frame.quic_error_code = static_cast<QuicErrorCode>(0x05060708); - } - close_frame.error_details = std::string(2048, 'A'); + QuicConnectionCloseFrame close_frame( + framer_.transport_version(), + static_cast<QuicErrorCode>( + VersionHasIetfQuicFrames(framer_.transport_version()) ? 0xa + : 0x05060708), + std::string(2048, 'A'), 0x05); + close_frame.extracted_error_code = QUIC_IETF_GQUIC_ERROR_MISSING; QuicFrames frames = {QuicFrame(&close_frame)}; // clang-format off @@ -7562,7 +7549,7 @@ // error code kVarInt62OneByte + 0x0a, // Frame type within the CONNECTION_CLOSE frame - kVarInt62OneByte + 0x00, + kVarInt62OneByte + 0x05, // error details length kVarInt62TwoBytes + 0x01, 0x00, // error details (truncated to 256 bytes) @@ -7653,12 +7640,12 @@ // error code kVarInt62OneByte + 0x11, // error details length - kVarInt62OneByte + 0x0d, - // error details - 'b', 'e', 'c', 'a', - 'u', 's', 'e', ' ', - 'I', ' ', 'c', 'a', - 'n', + kVarInt62OneByte + 0x0f, + // error details, note that it includes an extended error code. + '0', ':', 'b', 'e', + 'c', 'a', 'u', 's', + 'e', ' ', 'I', ' ', + 'c', 'a', 'n', }; // clang-format on @@ -7687,6 +7674,9 @@ static_cast<uint64_t>(QUIC_INVALID_STREAM_ID); app_close_frame.error_details = std::string(2048, 'A'); app_close_frame.close_type = IETF_QUIC_APPLICATION_CONNECTION_CLOSE; + // Setting to missing ensures that if it is missing, the extended + // code is not added to the text message. + app_close_frame.extracted_error_code = QUIC_IETF_GQUIC_ERROR_MISSING; QuicFrames frames = {QuicFrame(&app_close_frame)};
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 68cfd6d..9b812de 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -71,7 +71,6 @@ num_outgoing_static_streams_(0), num_incoming_static_streams_(0), num_locally_closed_incoming_streams_highest_offset_(0), - on_closed_frame_(QUIC_NO_ERROR, ""), flow_controller_( this, QuicUtils::GetInvalidStreamId(connection->transport_version()),