gfe-relnote: Refactor QuicConnectionCloseFrame. Not protected. QuicConnectionCloseFrame::quic_error_code should not be accessed when using IETF QUIC, however, as it is a public member, this is not enforced. When read, it returns the on-the-wire error code (transport or application) cast into a QuicErrorCode. This is bad. Since public unions cannot enforce restrictions on access, I propose to remove the union for clarity. The price paid is type: wire_error_code will now be uint64_t even for Google QUIC error codes which could be QuicErrorCode, and for IETF QUIC transport errors, which could be QuicIetfTransportErrorCodes. Also rename extracted_error_code to quic_error_code for clarity, because for sent frames, it is not extracted. This refactor is necessary for modifying QuicErrorCodeToTransportErrorCode() to return IETF wire error codes. When using Google QUIC, old fields |quic_error_code| and |extracted_error_code| and new fields |wire_error_code| and |quic_error_code| should always have the same value, therefore this refactor should not introduce any functional change. PiperOrigin-RevId: 304902883 Change-Id: I586a92609fb8a6186310f6585c8d29ee1d60af4f
diff --git a/quic/core/frames/quic_connection_close_frame.cc b/quic/core/frames/quic_connection_close_frame.cc index 35aedf4..6e5115b 100644 --- a/quic/core/frames/quic_connection_close_frame.cc +++ b/quic/core/frames/quic_connection_close_frame.cc
@@ -7,13 +7,14 @@ #include <memory> #include "net/third_party/quiche/src/quic/core/quic_constants.h" +#include "net/third_party/quiche/src/quic/core/quic_types.h" namespace quic { QuicConnectionCloseFrame::QuicConnectionCloseFrame() // Default close type ensures that existing, pre-V99 code works as expected. : close_type(GOOGLE_QUIC_CONNECTION_CLOSE), + wire_error_code(QUIC_NO_ERROR), quic_error_code(QUIC_NO_ERROR), - extracted_error_code(QUIC_NO_ERROR), transport_close_frame_type(0) {} QuicConnectionCloseFrame::QuicConnectionCloseFrame( @@ -21,10 +22,10 @@ QuicErrorCode error_code, std::string error_phrase, uint64_t frame_type) - : extracted_error_code(error_code), error_details(error_phrase) { + : quic_error_code(error_code), error_details(error_phrase) { if (!VersionHasIetfQuicFrames(transport_version)) { close_type = GOOGLE_QUIC_CONNECTION_CLOSE; - quic_error_code = error_code; + wire_error_code = error_code; transport_close_frame_type = 0; return; } @@ -33,34 +34,35 @@ if (mapping.is_transport_close_) { // Maps to a transport close close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE; - transport_error_code = mapping.transport_error_code_; + wire_error_code = mapping.transport_error_code_; transport_close_frame_type = frame_type; return; } // Maps to an application close. close_type = IETF_QUIC_APPLICATION_CONNECTION_CLOSE; - application_error_code = mapping.application_error_code_; + wire_error_code = mapping.application_error_code_; transport_close_frame_type = 0; } std::ostream& operator<<( std::ostream& os, const QuicConnectionCloseFrame& connection_close_frame) { - os << "{ Close type: " << connection_close_frame.close_type - << ", error_code: "; + os << "{ Close type: " << connection_close_frame.close_type; switch (connection_close_frame.close_type) { case IETF_QUIC_TRANSPORT_CONNECTION_CLOSE: - os << connection_close_frame.transport_error_code; + os << ", wire_error_code: " + << static_cast<QuicIetfTransportErrorCodes>( + connection_close_frame.wire_error_code); break; case IETF_QUIC_APPLICATION_CONNECTION_CLOSE: - os << connection_close_frame.application_error_code; + os << ", wire_error_code: " << connection_close_frame.wire_error_code; break; case GOOGLE_QUIC_CONNECTION_CLOSE: - os << connection_close_frame.quic_error_code; + // Do not log, value same as |quic_error_code|. break; } - os << ", extracted_error_code: " - << QuicErrorCodeToString(connection_close_frame.extracted_error_code) + os << ", quic_error_code: " + << QuicErrorCodeToString(connection_close_frame.quic_error_code) << ", error_details: '" << connection_close_frame.error_details << "'"; if (connection_close_frame.close_type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE) {
diff --git a/quic/core/frames/quic_connection_close_frame.h b/quic/core/frames/quic_connection_close_frame.h index 4ee41b9..ce41b03 100644 --- a/quic/core/frames/quic_connection_close_frame.h +++ b/quic/core/frames/quic_connection_close_frame.h
@@ -31,33 +31,25 @@ std::ostream& os, const QuicConnectionCloseFrame& c); - // Indicates whether the received CONNECTION_CLOSE frame is a Google QUIC - // CONNECTION_CLOSE, IETF QUIC CONNECTION_CLOSE. + // Indicates whether the the frame is a Google QUIC CONNECTION_CLOSE frame, + // an IETF QUIC CONNECTION_CLOSE frame with transport error code, + // or an IETF QUIC CONNECTION_CLOSE frame with application error code. QuicConnectionCloseType close_type; - // This is the error field in the frame. - // The CONNECTION_CLOSE frame reports an error code: - // - The transport error code as reported in a CONNECTION_CLOSE/Transport - // frame (serialized as a VarInt), - // - An opaque 64-bit code as reported in CONNECTION_CLOSE/Application frames - // (serialized as a VarInt),, - // - A 16 bit QuicErrorCode, which is used in Google QUIC. - union { - QuicIetfTransportErrorCodes transport_error_code; - uint64_t application_error_code; - QuicErrorCode quic_error_code; - }; + // The error code on the wire. For Google QUIC frames, this has the same + // value as |quic_error_code|. + uint64_t wire_error_code; - // For IETF QUIC frames, this is the error code is extracted from, or added - // to, the error details text. For received Google QUIC frames, the Google - // QUIC error code from the frame's error code field is copied here (as well - // as in quic_error_code, above). - QuicErrorCode extracted_error_code; + // The underlying error. For Google QUIC frames, this has the same value as + // |wire_error_code|. For sent IETF QUIC frames, this is the error that + // triggered the closure of the connection. For received IETF QUIC frames, + // this is parsed from the Reason Phrase field of the CONNECTION_CLOSE frame, + // or QUIC_IETF_GQUIC_ERROR_MISSING. + QuicErrorCode quic_error_code; - // String with additional error details. "QuicErrorCode: 123" will be appended - // to the error details when sending IETF QUIC Connection Close and - // Application Close frames and parsed into extracted_error_code upon receipt, - // when present. + // String with additional error details. |quic_error_code| and a colon will be + // prepended to the error details when sending IETF QUIC frames, and parsed + // into |quic_error_code| upon receipt, when present. std::string error_details; // The frame type present in the IETF transport connection close frame.
diff --git a/quic/core/frames/quic_frames_test.cc b/quic/core/frames/quic_frames_test.cc index 7450adb..22322ed 100644 --- a/quic/core/frames/quic_frames_test.cc +++ b/quic/core/frames/quic_frames_test.cc
@@ -147,11 +147,9 @@ // indicating that, in fact, no extended error code was available from the // underlying frame. EXPECT_EQ( - "{ Close type: GOOGLE_QUIC_CONNECTION_CLOSE, error_code: 25, " - "extracted_error_code: QUIC_NO_ERROR, " - "error_details: 'No recent " - "network activity.'" - "}\n", + "{ Close type: GOOGLE_QUIC_CONNECTION_CLOSE, " + "quic_error_code: QUIC_NETWORK_IDLE_TIMEOUT, " + "error_details: 'No recent network activity.'}\n", stream.str()); QuicFrame quic_frame(&frame); EXPECT_FALSE(IsControlFrame(quic_frame.type)); @@ -160,16 +158,16 @@ TEST_F(QuicFramesTest, TransportConnectionCloseFrameToString) { QuicConnectionCloseFrame frame; frame.close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE; - frame.transport_error_code = FINAL_SIZE_ERROR; - frame.extracted_error_code = QUIC_NETWORK_IDLE_TIMEOUT; + frame.wire_error_code = FINAL_SIZE_ERROR; + frame.quic_error_code = QUIC_NETWORK_IDLE_TIMEOUT; frame.error_details = "No recent network activity."; frame.transport_close_frame_type = IETF_STREAM; std::ostringstream stream; stream << frame; EXPECT_EQ( - "{ Close type: IETF_QUIC_TRANSPORT_CONNECTION_CLOSE, error_code: " - "FINAL_SIZE_ERROR, " - "extracted_error_code: QUIC_NETWORK_IDLE_TIMEOUT, " + "{ Close type: IETF_QUIC_TRANSPORT_CONNECTION_CLOSE, " + "wire_error_code: FINAL_SIZE_ERROR, " + "quic_error_code: QUIC_NETWORK_IDLE_TIMEOUT, " "error_details: 'No recent " "network activity.', " "frame_type: IETF_STREAM"
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index f83dbdd..99e88c3 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -1226,31 +1226,30 @@ case GOOGLE_QUIC_CONNECTION_CLOSE: QUIC_DLOG(INFO) << ENDPOINT << "Received ConnectionClose for connection: " << connection_id() << ", with error: " - << QuicErrorCodeToString(frame.extracted_error_code) - << " (" << frame.error_details << ")"; + << QuicErrorCodeToString(frame.quic_error_code) << " (" + << frame.error_details << ")"; break; case IETF_QUIC_TRANSPORT_CONNECTION_CLOSE: QUIC_DLOG(INFO) << ENDPOINT << "Received Transport ConnectionClose for connection: " << connection_id() << ", with error: " - << QuicErrorCodeToString(frame.extracted_error_code) - << " (" << frame.error_details << ")" - << ", transport error code: " - << frame.transport_error_code << ", error frame type: " + << QuicErrorCodeToString(frame.quic_error_code) << " (" + << frame.error_details << ")" + << ", transport error code: " << frame.wire_error_code + << ", error frame type: " << frame.transport_close_frame_type; break; case IETF_QUIC_APPLICATION_CONNECTION_CLOSE: QUIC_DLOG(INFO) << ENDPOINT << "Received Application ConnectionClose for connection: " << connection_id() << ", with error: " - << QuicErrorCodeToString(frame.extracted_error_code) - << " (" << frame.error_details << ")" - << ", application error code: " - << frame.application_error_code; + << QuicErrorCodeToString(frame.quic_error_code) << " (" + << frame.error_details << ")" + << ", application error code: " << frame.wire_error_code; break; } - if (frame.extracted_error_code == QUIC_BAD_MULTIPATH_FLAG) { + if (frame.quic_error_code == QUIC_BAD_MULTIPATH_FLAG) { QUIC_LOG_FIRST_N(ERROR, 10) << "Unexpected QUIC_BAD_MULTIPATH_FLAG error." << " last_received_header: " << last_header_ << " encryption_level: " << encryption_level_;
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index e5faa35..ded114b 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -1670,8 +1670,11 @@ const std::vector<QuicConnectionCloseFrame>& connection_close_frames = writer_->connection_close_frames(); ASSERT_EQ(1u, connection_close_frames.size()); + + EXPECT_EQ(expected_code, connection_close_frames[0].quic_error_code); + if (!VersionHasIetfQuicFrames(version().transport_version)) { - EXPECT_EQ(expected_code, connection_close_frames[0].quic_error_code); + EXPECT_EQ(expected_code, connection_close_frames[0].wire_error_code); EXPECT_EQ(GOOGLE_QUIC_CONNECTION_CLOSE, connection_close_frames[0].close_type); return; @@ -1685,16 +1688,13 @@ EXPECT_EQ(IETF_QUIC_TRANSPORT_CONNECTION_CLOSE, connection_close_frames[0].close_type); EXPECT_EQ(mapping.transport_error_code_, - connection_close_frames[0].transport_error_code); - // TODO(fkastenholz): when the extracted error code CL lands, - // need to test that extracted==expected. + connection_close_frames[0].wire_error_code); } else { // This maps to an application close. - EXPECT_EQ(expected_code, connection_close_frames[0].quic_error_code); EXPECT_EQ(IETF_QUIC_APPLICATION_CONNECTION_CLOSE, connection_close_frames[0].close_type); - // TODO(fkastenholz): when the extracted error code CL lands, - // need to test that extracted==expected. + EXPECT_EQ(mapping.application_error_code_, + connection_close_frames[0].wire_error_code); } } @@ -7621,7 +7621,7 @@ kSelfAddress, kPeerAddress, QuicReceivedPacket(buffer, encrypted_length, QuicTime::Zero(), false)); EXPECT_EQ(1, connection_close_frame_count_); - EXPECT_THAT(saved_connection_close_frame_.extracted_error_code, + EXPECT_THAT(saved_connection_close_frame_.quic_error_code, IsError(QUIC_PEER_GOING_AWAY)); } @@ -10134,7 +10134,7 @@ ASSERT_EQ(1u, connection_close_frames.size()); EXPECT_EQ(IETF_QUIC_TRANSPORT_CONNECTION_CLOSE, connection_close_frames[0].close_type); - EXPECT_EQ(kQuicErrorCode, connection_close_frames[0].extracted_error_code); + EXPECT_EQ(kQuicErrorCode, connection_close_frames[0].quic_error_code); EXPECT_EQ(kTransportCloseFrameType, connection_close_frames[0].transport_close_frame_type); }
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc index f900191..5813010 100644 --- a/quic/core/quic_framer.cc +++ b/quic/core/quic_framer.cc
@@ -541,16 +541,13 @@ // Prepend the extra error information to the string and get the result's // length. const size_t truncated_error_string_size = TruncatedErrorStringSize( - GenerateErrorString(frame.error_details, frame.extracted_error_code)); + GenerateErrorString(frame.error_details, frame.quic_error_code)); const size_t frame_size = truncated_error_string_size + QuicDataWriter::GetVarInt62Len(truncated_error_string_size) + kQuicFrameTypeSize + - QuicDataWriter::GetVarInt62Len( - (frame.close_type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE) - ? frame.transport_error_code - : frame.application_error_code); + QuicDataWriter::GetVarInt62Len(frame.wire_error_code); if (frame.close_type == IETF_QUIC_APPLICATION_CONNECTION_CLOSE) { return frame_size; } @@ -3975,13 +3972,11 @@ error_code = QUIC_LAST_ERROR; } + // For Google QUIC connection closes, |wire_error_code| and |quic_error_code| + // must have the same value. + frame->wire_error_code = error_code; frame->quic_error_code = static_cast<QuicErrorCode>(error_code); - // For Google QUIC connection closes, copy the Google QUIC error code to - // the extracted error code field so that the Google QUIC error code is always - // available in extracted_error_code. - frame->extracted_error_code = frame->quic_error_code; - quiche::QuicheStringPiece error_details; if (!reader->ReadStringPiece16(&error_details)) { set_detailed_error("Unable to read connection close error details."); @@ -5517,7 +5512,7 @@ if (VersionHasIetfQuicFrames(version_.transport_version)) { return AppendIetfConnectionCloseFrame(frame, writer); } - uint32_t error_code = static_cast<uint32_t>(frame.quic_error_code); + uint32_t error_code = static_cast<uint32_t>(frame.wire_error_code); if (!writer->WriteUInt32(error_code)) { return false; } @@ -5642,10 +5637,7 @@ return false; } - if (!writer->WriteVarInt62( - (frame.close_type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE) - ? frame.transport_error_code - : frame.application_error_code)) { + if (!writer->WriteVarInt62(frame.wire_error_code)) { set_detailed_error("Can not write connection close frame error code"); return false; } @@ -5663,7 +5655,7 @@ // code. Encode the error information in the reason phrase and serialize the // result. std::string final_error_string = - GenerateErrorString(frame.error_details, frame.extracted_error_code); + GenerateErrorString(frame.error_details, frame.quic_error_code); if (!writer->WriteStringPieceVarInt62( TruncateErrorString(final_error_string))) { set_detailed_error("Can not write connection close phrase"); @@ -5684,12 +5676,7 @@ return false; } - if (frame->close_type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE) { - frame->transport_error_code = - static_cast<QuicIetfTransportErrorCodes>(error_code); - } else if (frame->close_type == IETF_QUIC_APPLICATION_CONNECTION_CLOSE) { - frame->application_error_code = error_code; - } + frame->wire_error_code = error_code; if (type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE) { // The frame-type of the frame causing the error is present only @@ -6626,10 +6613,10 @@ if (ed.size() < 2 || !quiche::QuicheTextUtils::IsAllDigits(ed[0]) || !quiche::QuicheTextUtils::StringToUint64(ed[0], &extracted_error_code)) { if (frame->close_type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE && - frame->transport_error_code == NO_IETF_QUIC_ERROR) { - frame->extracted_error_code = QUIC_NO_ERROR; + frame->wire_error_code == NO_IETF_QUIC_ERROR) { + frame->quic_error_code = QUIC_NO_ERROR; } else { - frame->extracted_error_code = QUIC_IETF_GQUIC_ERROR_MISSING; + frame->quic_error_code = QUIC_IETF_GQUIC_ERROR_MISSING; } return; } @@ -6641,8 +6628,7 @@ quiche::QuicheStringPiece x = quiche::QuicheStringPiece(frame->error_details); x.remove_prefix(ed[0].length() + 1); frame->error_details = std::string(x); - frame->extracted_error_code = - static_cast<QuicErrorCode>(extracted_error_code); + frame->quic_error_code = static_cast<QuicErrorCode>(extracted_error_code); } #undef ENDPOINT // undef for jumbo builds
diff --git a/quic/core/quic_framer.h b/quic/core/quic_framer.h index 746e6a1..f0c694a 100644 --- a/quic/core/quic_framer.h +++ b/quic/core/quic_framer.h
@@ -1097,9 +1097,10 @@ // This text, inserted by the peer if it's using Google's QUIC implementation, // contains additional error information that narrows down the exact error. The // extracted error code and (possibly updated) error_details string are returned -// in |*frame|. If an error code is not found in the error details then the -// extracted_error_code is set to QuicErrorCode::QUIC_IETF_GQUIC_ERROR_MISSING. -// If there is an error code in the string then it is removed from the string. +// in |*frame|. If an error code is not found in the error details, then +// frame->quic_error_code is set to +// QuicErrorCode::QUIC_IETF_GQUIC_ERROR_MISSING. If there is an error code in +// the string then it is removed from the string. QUIC_EXPORT_PRIVATE void MaybeExtractQuicErrorCode( QuicConnectionCloseFrame* frame);
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc index 813d4f6..d274c6c 100644 --- a/quic/core/quic_framer_test.cc +++ b/quic/core/quic_framer_test.cc
@@ -4442,19 +4442,18 @@ EXPECT_EQ(0u, visitor_.stream_frames_.size()); EXPECT_EQ(0x11u, static_cast<unsigned>( - visitor_.connection_close_frame_.quic_error_code)); + visitor_.connection_close_frame_.wire_error_code)); EXPECT_EQ("because I can", visitor_.connection_close_frame_.error_details); if (VersionHasIetfQuicFrames(framer_.transport_version())) { EXPECT_EQ(0x1234u, visitor_.connection_close_frame_.transport_close_frame_type); - EXPECT_THAT(visitor_.connection_close_frame_.extracted_error_code, + EXPECT_THAT(visitor_.connection_close_frame_.quic_error_code, IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); } else { - // For Google QUIC closes, the error code is copied into - // extracted_error_code. - EXPECT_EQ(0x11u, - static_cast<unsigned>( - visitor_.connection_close_frame_.extracted_error_code)); + // For Google QUIC frame, |quic_error_code| and |wire_error_code| has the + // same value. + EXPECT_EQ(0x11u, static_cast<unsigned>( + visitor_.connection_close_frame_.quic_error_code)); } ASSERT_EQ(0u, visitor_.ack_frames_.size()); @@ -4576,15 +4575,15 @@ EXPECT_EQ(0u, visitor_.stream_frames_.size()); EXPECT_EQ(0x11u, static_cast<unsigned>( - visitor_.connection_close_frame_.quic_error_code)); + visitor_.connection_close_frame_.wire_error_code)); if (VersionHasIetfQuicFrames(framer_.transport_version())) { EXPECT_EQ(0x1234u, visitor_.connection_close_frame_.transport_close_frame_type); - EXPECT_EQ(17767u, visitor_.connection_close_frame_.extracted_error_code); + EXPECT_EQ(17767u, visitor_.connection_close_frame_.quic_error_code); EXPECT_EQ("because I can", visitor_.connection_close_frame_.error_details); } else { - EXPECT_EQ(0x11u, visitor_.connection_close_frame_.extracted_error_code); + EXPECT_EQ(0x11u, visitor_.connection_close_frame_.quic_error_code); // Error code is not prepended in GQUIC, so it is not removed and should // remain in the reason phrase. EXPECT_EQ("17767:because I can", @@ -4648,8 +4647,8 @@ EXPECT_EQ(IETF_QUIC_APPLICATION_CONNECTION_CLOSE, visitor_.connection_close_frame_.close_type); - EXPECT_EQ(122u, visitor_.connection_close_frame_.extracted_error_code); - EXPECT_EQ(0x11u, visitor_.connection_close_frame_.quic_error_code); + EXPECT_EQ(122u, visitor_.connection_close_frame_.quic_error_code); + EXPECT_EQ(0x11u, visitor_.connection_close_frame_.wire_error_code); EXPECT_EQ("because I can", visitor_.connection_close_frame_.error_details); ASSERT_EQ(0u, visitor_.ack_frames_.size()); @@ -4710,8 +4709,8 @@ EXPECT_EQ(IETF_QUIC_APPLICATION_CONNECTION_CLOSE, visitor_.connection_close_frame_.close_type); - EXPECT_EQ(17767u, visitor_.connection_close_frame_.extracted_error_code); - EXPECT_EQ(0x11u, visitor_.connection_close_frame_.quic_error_code); + EXPECT_EQ(17767u, visitor_.connection_close_frame_.quic_error_code); + EXPECT_EQ(0x11u, visitor_.connection_close_frame_.wire_error_code); EXPECT_EQ("because I can", visitor_.connection_close_frame_.error_details); ASSERT_EQ(0u, visitor_.ack_frames_.size()); @@ -7528,12 +7527,7 @@ header.packet_number = kPacketNumber; 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; + framer_.transport_version(), QUIC_INTERNAL_ERROR, "because I can", 0x05); QuicFrames frames = {QuicFrame(&close_frame)}; // clang-format off @@ -7548,7 +7542,7 @@ // frame type (connection close frame) 0x02, // error code - 0x05, 0x06, 0x07, 0x08, + 0x00, 0x00, 0x00, 0x01, // error details length 0x00, 0x0d, // error details @@ -7569,7 +7563,7 @@ // frame type (connection close frame) 0x02, // error code - 0x05, 0x06, 0x07, 0x08, + 0x00, 0x00, 0x00, 0x01, // error details length 0x00, 0x0d, // error details @@ -7590,16 +7584,16 @@ // frame type (IETF_CONNECTION_CLOSE frame) 0x1c, // error code - kVarInt62OneByte + 0x11, + kVarInt62OneByte + 0x01, // Frame type within the CONNECTION_CLOSE frame kVarInt62OneByte + 0x05, // error details length - kVarInt62OneByte + 0x0d, + kVarInt62OneByte + 0x0f, // error details - 'b', 'e', 'c', 'a', - 'u', 's', 'e', ' ', - 'I', ' ', 'c', 'a', - 'n', + '1', ':', 'b', 'e', + 'c', 'a', 'u', 's', + 'e', ' ', 'I', ' ', + 'c', 'a', 'n', }; // clang-format on @@ -7636,7 +7630,7 @@ "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); + close_frame.quic_error_code = static_cast<QuicErrorCode>(0x4567); QuicFrames frames = {QuicFrame(&close_frame)}; @@ -7733,13 +7727,9 @@ header.version_flag = false; header.packet_number = kPacketNumber; - 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; + QuicConnectionCloseFrame close_frame(framer_.transport_version(), + QUIC_INTERNAL_ERROR, + std::string(2048, 'A'), 0x05); QuicFrames frames = {QuicFrame(&close_frame)}; // clang-format off @@ -7754,7 +7744,7 @@ // frame type (connection close frame) 0x02, // error code - 0x05, 0x06, 0x07, 0x08, + 0x00, 0x00, 0x00, 0x01, // error details length 0x01, 0x00, // error details (truncated to 256 bytes) @@ -7803,7 +7793,7 @@ // frame type (connection close frame) 0x02, // error code - 0x05, 0x06, 0x07, 0x08, + 0x00, 0x00, 0x00, 0x01, // error details length 0x01, 0x00, // error details (truncated to 256 bytes) @@ -7852,13 +7842,13 @@ // frame type (IETF_CONNECTION_CLOSE frame) 0x1c, // error code - kVarInt62OneByte + 0x0a, + kVarInt62OneByte + 0x01, // Frame type within the CONNECTION_CLOSE frame kVarInt62OneByte + 0x05, // error details length kVarInt62TwoBytes + 0x01, 0x00, // error details (truncated to 256 bytes) - 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', + '1', ':', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', @@ -7923,8 +7913,7 @@ header.packet_number = kPacketNumber; QuicConnectionCloseFrame app_close_frame; - app_close_frame.application_error_code = - static_cast<uint64_t>(QUIC_INVALID_STREAM_ID); + app_close_frame.wire_error_code = 0x11; app_close_frame.error_details = "because I can"; app_close_frame.close_type = IETF_QUIC_APPLICATION_CONNECTION_CLOSE; @@ -7975,13 +7964,12 @@ header.packet_number = kPacketNumber; QuicConnectionCloseFrame app_close_frame; - app_close_frame.application_error_code = - static_cast<uint64_t>(QUIC_INVALID_STREAM_ID); + app_close_frame.wire_error_code = 0x11; 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; + app_close_frame.quic_error_code = QUIC_IETF_GQUIC_ERROR_MISSING; QuicFrames frames = {QuicFrame(&app_close_frame)}; @@ -14254,84 +14242,77 @@ frame.error_details = "this has no error code info in it"; MaybeExtractQuicErrorCode(&frame); - EXPECT_THAT(frame.extracted_error_code, - IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); + EXPECT_THAT(frame.quic_error_code, IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); EXPECT_EQ("this has no error code info in it", frame.error_details); frame.error_details = "1234this does not have the colon in it"; MaybeExtractQuicErrorCode(&frame); - EXPECT_THAT(frame.extracted_error_code, - IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); + EXPECT_THAT(frame.quic_error_code, IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); EXPECT_EQ("1234this does not have the colon in it", frame.error_details); frame.error_details = "1a234:this has a colon, but a malformed error number"; MaybeExtractQuicErrorCode(&frame); - EXPECT_THAT(frame.extracted_error_code, - IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); + EXPECT_THAT(frame.quic_error_code, IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); EXPECT_EQ("1a234:this has a colon, but a malformed error number", frame.error_details); frame.error_details = "1234:this is good"; MaybeExtractQuicErrorCode(&frame); - EXPECT_EQ(1234u, frame.extracted_error_code); + EXPECT_EQ(1234u, frame.quic_error_code); EXPECT_EQ("this is good", frame.error_details); frame.error_details = "1234 :this is not good, space between last digit and colon"; MaybeExtractQuicErrorCode(&frame); - EXPECT_THAT(frame.extracted_error_code, - IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); + EXPECT_THAT(frame.quic_error_code, IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); EXPECT_EQ("1234 :this is not good, space between last digit and colon", frame.error_details); frame.error_details = "123456789"; MaybeExtractQuicErrorCode(&frame); EXPECT_THAT( - frame.extracted_error_code, + frame.quic_error_code, IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); // Not good, all numbers, no : EXPECT_EQ("123456789", frame.error_details); frame.error_details = "1234:"; MaybeExtractQuicErrorCode(&frame); EXPECT_EQ(1234u, - frame.extracted_error_code); // corner case. + frame.quic_error_code); // corner case. EXPECT_EQ("", frame.error_details); frame.error_details = "1234:5678"; MaybeExtractQuicErrorCode(&frame); EXPECT_EQ(1234u, - frame.extracted_error_code); // another corner case. + frame.quic_error_code); // another corner case. EXPECT_EQ("5678", frame.error_details); frame.error_details = "12345 6789:"; MaybeExtractQuicErrorCode(&frame); - EXPECT_THAT(frame.extracted_error_code, + EXPECT_THAT(frame.quic_error_code, IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); // Not good EXPECT_EQ("12345 6789:", frame.error_details); frame.error_details = ":no numbers, is not good"; MaybeExtractQuicErrorCode(&frame); - EXPECT_THAT(frame.extracted_error_code, - IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); + EXPECT_THAT(frame.quic_error_code, IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); EXPECT_EQ(":no numbers, is not good", frame.error_details); frame.error_details = "qwer:also no numbers, is not good"; MaybeExtractQuicErrorCode(&frame); - EXPECT_THAT(frame.extracted_error_code, - IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); + EXPECT_THAT(frame.quic_error_code, IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); EXPECT_EQ("qwer:also no numbers, is not good", frame.error_details); frame.error_details = " 1234:this is not good, space before first digit"; MaybeExtractQuicErrorCode(&frame); - EXPECT_THAT(frame.extracted_error_code, - IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); + EXPECT_THAT(frame.quic_error_code, IsError(QUIC_IETF_GQUIC_ERROR_MISSING)); EXPECT_EQ(" 1234:this is not good, space before first digit", frame.error_details); frame.error_details = "1234:"; MaybeExtractQuicErrorCode(&frame); EXPECT_EQ(1234u, - frame.extracted_error_code); // this is good + frame.quic_error_code); // this is good EXPECT_EQ("", frame.error_details); }
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index bd29b8e..62a1ae3 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -398,7 +398,7 @@ RecordConnectionCloseAtServer(frame.quic_error_code, source); } - if (on_closed_frame_.extracted_error_code == QUIC_NO_ERROR) { + if (on_closed_frame_.quic_error_code == QUIC_NO_ERROR) { // Save all of the connection close information on_closed_frame_ = frame; } @@ -431,8 +431,8 @@ if (visitor_) { visitor_->OnConnectionClosed(connection_->connection_id(), - frame.extracted_error_code, - frame.error_details, source); + frame.quic_error_code, frame.error_details, + source); } }
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index 6e07323..ee8e512 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -365,7 +365,7 @@ bool goaway_received() const { return goaway_received_; } // Returns the Google QUIC error code - QuicErrorCode error() const { return on_closed_frame_.extracted_error_code; } + QuicErrorCode error() const { return on_closed_frame_.quic_error_code; } const std::string& error_details() const { return on_closed_frame_.error_details; } @@ -375,12 +375,6 @@ QuicConnectionCloseType close_type() const { return on_closed_frame_.close_type; } - QuicIetfTransportErrorCodes transport_error_code() const { - return on_closed_frame_.transport_error_code; - } - uint16_t application_error_code() const { - return on_closed_frame_.application_error_code; - } Perspective perspective() const { return perspective_; }
diff --git a/quic/tools/quic_client_interop_test_bin.cc b/quic/tools/quic_client_interop_test_bin.cc index 9745955..25e9c1e 100644 --- a/quic/tools/quic_client_interop_test_bin.cc +++ b/quic/tools/quic_client_interop_test_bin.cc
@@ -105,20 +105,21 @@ QUIC_LOG(ERROR) << "Received unexpected GoogleQUIC connection close"; break; case IETF_QUIC_TRANSPORT_CONNECTION_CLOSE: - if (frame.transport_error_code == NO_IETF_QUIC_ERROR) { + if (frame.wire_error_code == NO_IETF_QUIC_ERROR) { InsertFeature(Feature::kConnectionClose); } else { QUIC_LOG(ERROR) << "Received transport connection close " << QuicIetfTransportErrorCodeString( - frame.transport_error_code); + static_cast<QuicIetfTransportErrorCodes>( + frame.wire_error_code)); } break; case IETF_QUIC_APPLICATION_CONNECTION_CLOSE: - if (frame.application_error_code == 0) { + if (frame.wire_error_code == 0) { InsertFeature(Feature::kConnectionClose); } else { QUIC_LOG(ERROR) << "Received application connection close " - << frame.application_error_code; + << frame.wire_error_code; } break; }