Change Application and Transport ConnectionClose codes to uint64_t
IETF QUIC Transport and Application ConnectionClose Error Codes were 16 bit
unsigned numbers. Draft 22 of the IETF QUIC specification changed them to be a
varint62, which we keep internally as a uint64_t. This CL effects that change.
The benefits of this CL are A) somewhat cleaner internal code (elimination of random casts) and B) applications can use the full 62 bits of the connection close codes when using IETF QUIC.
gfe-relnote: N/A IETF QUIC only, V99 flag protected.
PiperOrigin-RevId: 267594523
Change-Id: I0d6a8af0bb134c15d6b034de4237a02e0458ee47
diff --git a/quic/core/frames/quic_connection_close_frame.cc b/quic/core/frames/quic_connection_close_frame.cc
index a7b3d53..63e0aee 100644
--- a/quic/core/frames/quic_connection_close_frame.cc
+++ b/quic/core/frames/quic_connection_close_frame.cc
@@ -6,6 +6,7 @@
#include <memory>
+#include "net/third_party/quiche/src/quic/core/quic_constants.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_str_cat.h"
namespace quic {
@@ -34,7 +35,9 @@
application_error_code(ietf_application_error_code),
extracted_error_code(quic_error_code),
error_details(std::move(error_details)),
- transport_close_frame_type(0) {}
+ transport_close_frame_type(0) {
+ DCHECK_LE(ietf_application_error_code, kMaxIetfVarInt);
+}
QuicConnectionCloseFrame::QuicConnectionCloseFrame(
QuicErrorCode quic_error_code,
@@ -45,7 +48,9 @@
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) {}
+ transport_close_frame_type(transport_frame_type) {
+ DCHECK_LE(transport_error_code, kMaxIetfVarInt);
+}
std::ostream& operator<<(
std::ostream& os,
diff --git a/quic/core/frames/quic_connection_close_frame.h b/quic/core/frames/quic_connection_close_frame.h
index b4f4fd7..b0cef45 100644
--- a/quic/core/frames/quic_connection_close_frame.h
+++ b/quic/core/frames/quic_connection_close_frame.h
@@ -49,8 +49,7 @@
// - A 16 bit QuicErrorCode, which is used in Google QUIC.
union {
QuicIetfTransportErrorCodes transport_error_code;
- // TODO(fkastenholz): Change this to uint64_t to reflect -22 of the ID.
- uint16_t application_error_code;
+ uint64_t application_error_code;
QuicErrorCode quic_error_code;
};
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index ec09686..9fc985f 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -8697,10 +8697,26 @@
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
EXPECT_CALL(visitor_, OnConnectionClosed(_, _));
EXPECT_EQ(Perspective::IS_CLIENT, connection_.perspective());
+ const QuicErrorCode kErrorCode = QUIC_INTERNAL_ERROR;
std::unique_ptr<QuicConnectionCloseFrame> connection_close_frame(
- new QuicConnectionCloseFrame(QUIC_INTERNAL_ERROR, ""));
+ new QuicConnectionCloseFrame(kErrorCode, ""));
if (VersionHasIetfQuicFrames(connection_.transport_version())) {
- connection_close_frame->close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE;
+ QuicErrorCodeToIetfMapping mapping =
+ QuicErrorCodeToTransportErrorCode(kErrorCode);
+ if (mapping.is_transport_close_) {
+ // Maps to a transport close
+ connection_close_frame->close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE;
+ connection_close_frame->transport_error_code =
+ mapping.transport_error_code_;
+ connection_close_frame->transport_close_frame_type = 0;
+ } else {
+ // Maps to an application close.
+ connection_close_frame->close_type =
+ IETF_QUIC_APPLICATION_CONNECTION_CLOSE;
+ connection_close_frame->application_error_code =
+ mapping.application_error_code_;
+ }
+ connection_close_frame->extracted_error_code = kErrorCode;
}
// Received 2 packets.
QuicFrame frame;
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc
index 4d7b70e..94dc3bf 100644
--- a/quic/core/quic_dispatcher.cc
+++ b/quic/core/quic_dispatcher.cc
@@ -136,7 +136,21 @@
QuicConnectionCloseFrame* frame =
new QuicConnectionCloseFrame(error_code, error_details);
if (VersionHasIetfQuicFrames(framer_.transport_version())) {
- frame->close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE;
+ QuicErrorCodeToIetfMapping mapping =
+ QuicErrorCodeToTransportErrorCode(error_code);
+ if (mapping.is_transport_close_) {
+ // Maps to a transport close
+ frame->close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE;
+ frame->transport_error_code = mapping.transport_error_code_;
+ // If closing the connection in the stateless terminator then there is
+ // no frame that is being processed.
+ frame->transport_close_frame_type = 0;
+ } else {
+ // Maps to an application close.
+ frame->close_type = IETF_QUIC_APPLICATION_CONNECTION_CLOSE;
+ frame->application_error_code = mapping.application_error_code_;
+ }
+ frame->extracted_error_code = error_code;
}
if (!creator_.AddSavedFrame(QuicFrame(frame), NOT_RETRANSMISSION)) {
diff --git a/quic/core/quic_error_codes.h b/quic/core/quic_error_codes.h
index 534d0b5..e07094c 100644
--- a/quic/core/quic_error_codes.h
+++ b/quic/core/quic_error_codes.h
@@ -340,10 +340,12 @@
// No error. Used as bound while iterating.
QUIC_LAST_ERROR = 125,
};
-// QuicErrorCodes is encoded as a single octet on-the-wire.
-static_assert(static_cast<int>(QUIC_LAST_ERROR) <=
- std::numeric_limits<uint8_t>::max(),
- "QuicErrorCode exceeds single octet");
+// QuicErrorCodes is encoded as four octets on-the-wire when doing Google QUIC,
+// or a varint62 when doing IETF QUIC. Ensure that its value does not exceed
+// the smaller of the two limits.
+static_assert(static_cast<uint64_t>(QUIC_LAST_ERROR) <=
+ static_cast<uint64_t>(std::numeric_limits<uint32_t>::max()),
+ "QuicErrorCode exceeds four octets");
// Returns the name of the QuicRstStreamErrorCode as a char*
QUIC_EXPORT_PRIVATE const char* QuicRstStreamErrorCodeToString(
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 9890f36..3a1be5e 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -524,17 +524,14 @@
const size_t truncated_error_string_size = TruncatedErrorStringSize(
GenerateErrorString(frame.error_details, frame.extracted_error_code));
- uint64_t close_code = 0;
- if (frame.close_type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE) {
- close_code = static_cast<uint64_t>(frame.transport_error_code);
- } else if (frame.close_type == IETF_QUIC_APPLICATION_CONNECTION_CLOSE) {
- close_code = static_cast<uint64_t>(frame.application_error_code);
- }
-
const size_t frame_size =
truncated_error_string_size +
QuicDataWriter::GetVarInt62Len(truncated_error_string_size) +
- kQuicFrameTypeSize + QuicDataWriter::GetVarInt62Len(close_code);
+ kQuicFrameTypeSize +
+ QuicDataWriter::GetVarInt62Len(
+ (frame.close_type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE)
+ ? frame.transport_error_code
+ : frame.application_error_code);
if (frame.close_type == IETF_QUIC_APPLICATION_CONNECTION_CLOSE) {
return frame_size;
}
@@ -5695,14 +5692,10 @@
return false;
}
- uint64_t close_code = 0;
- if (frame.close_type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE) {
- close_code = static_cast<uint64_t>(frame.transport_error_code);
- } else if (frame.close_type == IETF_QUIC_APPLICATION_CONNECTION_CLOSE) {
- close_code = static_cast<uint64_t>(frame.application_error_code);
- }
-
- if (!writer->WriteVarInt62(close_code)) {
+ if (!writer->WriteVarInt62(
+ (frame.close_type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE)
+ ? frame.transport_error_code
+ : frame.application_error_code)) {
set_detailed_error("Can not write connection close frame error code");
return false;
}
@@ -5734,32 +5727,18 @@
QuicConnectionCloseType type,
QuicConnectionCloseFrame* frame) {
frame->close_type = type;
- uint64_t error_code;
+ uint64_t error_code;
if (!reader->ReadVarInt62(&error_code)) {
set_detailed_error("Unable to read connection close error code.");
return false;
}
- // TODO(fkastenholz): When error codes uniformly go to uint64, remove the
- // range check.
if (frame->close_type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE) {
- if (error_code > 0xffff) {
- frame->transport_error_code =
- static_cast<QuicIetfTransportErrorCodes>(0xffff);
- QUIC_DLOG(ERROR) << "Transport error code " << error_code << " > 0xffff";
- } else {
- frame->transport_error_code =
- static_cast<QuicIetfTransportErrorCodes>(error_code);
- }
+ frame->transport_error_code =
+ static_cast<QuicIetfTransportErrorCodes>(error_code);
} else if (frame->close_type == IETF_QUIC_APPLICATION_CONNECTION_CLOSE) {
- if (error_code > 0xffff) {
- frame->application_error_code = 0xffff;
- QUIC_DLOG(ERROR) << "Application error code " << error_code
- << " > 0xffff";
- } else {
- frame->application_error_code = static_cast<uint16_t>(error_code);
- }
+ frame->application_error_code = error_code;
}
if (type == IETF_QUIC_TRANSPORT_CONNECTION_CLOSE) {
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index b13f7e1..bcb717f 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -4473,7 +4473,7 @@
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(0x11, visitor_.connection_close_frame_.quic_error_code);
+ EXPECT_EQ(0x11u, visitor_.connection_close_frame_.quic_error_code);
EXPECT_EQ("because I can", visitor_.connection_close_frame_.error_details);
ASSERT_EQ(0u, visitor_.ack_frames_.size());
@@ -4628,7 +4628,7 @@
PACKET_8BYTE_CONNECTION_ID, PACKET_0BYTE_CONNECTION_ID));
EXPECT_EQ(kStreamId, visitor_.goaway_frame_.last_good_stream_id);
- EXPECT_EQ(0x9, visitor_.goaway_frame_.error_code);
+ EXPECT_EQ(0x9u, visitor_.goaway_frame_.error_code);
EXPECT_EQ("because I can", visitor_.goaway_frame_.reason_phrase);
CheckFramingBoundaries(fragments, QUIC_INVALID_GOAWAY_DATA);
diff --git a/quic/core/quic_packet_generator_test.cc b/quic/core/quic_packet_generator_test.cc
index 9c2e3f3..bd66cb1 100644
--- a/quic/core/quic_packet_generator_test.cc
+++ b/quic/core/quic_packet_generator_test.cc
@@ -1290,10 +1290,24 @@
delegate_.SetCanWriteAnything();
char buf[2000] = {};
QuicStringPiece error_details(buf, 2000);
- QuicConnectionCloseFrame* frame = new QuicConnectionCloseFrame(
- QUIC_PACKET_WRITE_ERROR, std::string(error_details));
+ const QuicErrorCode kQuicErrorCode = QUIC_PACKET_WRITE_ERROR;
+ QuicConnectionCloseFrame* frame =
+ new QuicConnectionCloseFrame(kQuicErrorCode, std::string(error_details));
if (VersionHasIetfQuicFrames(framer_.transport_version())) {
- frame->close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE;
+ QuicErrorCodeToIetfMapping mapping =
+ QuicErrorCodeToTransportErrorCode(kQuicErrorCode);
+ if (mapping.is_transport_close_) {
+ // Maps to a transport close
+ frame->close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE;
+ frame->transport_error_code = mapping.transport_error_code_;
+ // Frame type is not important for the tests that invoke this method.
+ frame->transport_close_frame_type = 0;
+ } else {
+ // Maps to an application close.
+ frame->close_type = IETF_QUIC_APPLICATION_CONNECTION_CLOSE;
+ frame->application_error_code = mapping.application_error_code_;
+ }
+ frame->extracted_error_code = kQuicErrorCode;
}
generator_.ConsumeRetransmittableControlFrame(QuicFrame(frame),
/*bundle_ack=*/false);
diff --git a/quic/core/quic_types.h b/quic/core/quic_types.h
index 2d89392..814b63c 100644
--- a/quic/core/quic_types.h
+++ b/quic/core/quic_types.h
@@ -508,8 +508,7 @@
// A vector of lost packets.
typedef std::vector<LostPacket> LostPacketVector;
-// TODO(fkastenholz): Change this to uint64_t to reflect -22 of the ID.
-enum QuicIetfTransportErrorCodes : uint16_t {
+enum QuicIetfTransportErrorCodes : uint64_t {
NO_IETF_QUIC_ERROR = 0x0,
INTERNAL_ERROR = 0x1,
SERVER_BUSY_ERROR = 0x2,
