Skip to content

GitLab

T
tangled
Commit 8ca037ff authored by Ryan Hamilton's avatar Ryan Hamilton Committed by Commit Bot
Browse files
Options

Truncate long error details in QUIC frames to 256 bytes. 

Protected by enabled flag FLAGS_quic_reloadable_flag_quic_truncate_long_details.

merge internal change: 175959960

BUG=785223

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Id7e15f179c7db5a19e86ddb9b474d96d2bd14342
Reviewed-on: https://chromium-review.googlesource.com/773405
Commit-Queue: Ryan Hamilton <rch@chromium.org>
Reviewed-by: default avatarZhongyi Shi <zhongyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517477}
parent 2fff6665
Hide whitespace changes
Inline Side-by-side
Showing with 297 additions and 8 deletions
net/quic/core/quic_flags_list.h
View file @ 8ca037ff
...@@ -200,3 +200,7 @@ QUIC_FLAG(bool, FLAGS_quic_reloadable_flag_quic_bbr_less_probe_rtt, false) ...@@ -200,3 +200,7 @@ QUIC_FLAG(bool, FLAGS_quic_reloadable_flag_quic_bbr_less_probe_rtt, false)
QUIC_FLAG(bool, QUIC_FLAG(bool,
FLAGS_quic_reloadable_flag_quic_server_reply_to_connectivity_probing, FLAGS_quic_reloadable_flag_quic_server_reply_to_connectivity_probing,
true) true)
// If true, truncates QUIC error strings to 256 characters before writing them
// to the wire.
QUIC_FLAG(bool, FLAGS_quic_reloadable_flag_quic_truncate_long_details, true)
net/quic/core/quic_framer.cc
View file @ 8ca037ff
...@@ -103,6 +103,9 @@ const uint8_t kLargestAckedOffset = 2; ...@@ -103,6 +103,9 @@ const uint8_t kLargestAckedOffset = 2;
const uint8_t kQuicHasMultipleAckBlocksOffset_Pre40 = 5; const uint8_t kQuicHasMultipleAckBlocksOffset_Pre40 = 5;
const uint8_t kQuicHasMultipleAckBlocksOffset = 4; const uint8_t kQuicHasMultipleAckBlocksOffset = 4;
// Maximum length of encoded error strings.
const int kMaxErrorStringLength = 256;
// Returns the absolute value of the difference between |a| and |b|. // Returns the absolute value of the difference between |a| and |b|.
QuicPacketNumber Delta(QuicPacketNumber a, QuicPacketNumber b) { QuicPacketNumber Delta(QuicPacketNumber a, QuicPacketNumber b) {
// Since these are unsigned numbers, we can't just return abs(a - b) // Since these are unsigned numbers, we can't just return abs(a - b)
...@@ -1885,9 +1888,11 @@ size_t QuicFramer::ComputeFrameLength( ...@@ -1885,9 +1888,11 @@ size_t QuicFramer::ComputeFrameLength(
return GetRstStreamFrameSize(); return GetRstStreamFrameSize();
case CONNECTION_CLOSE_FRAME: case CONNECTION_CLOSE_FRAME:
return GetMinConnectionCloseFrameSize() + return GetMinConnectionCloseFrameSize() +
frame.connection_close_frame->error_details.size(); TruncateErrorString(frame.connection_close_frame->error_details)
.size();
case GOAWAY_FRAME: case GOAWAY_FRAME:
return GetMinGoAwayFrameSize() + frame.goaway_frame->reason_phrase.size(); return GetMinGoAwayFrameSize() +
TruncateErrorString(frame.goaway_frame->reason_phrase).size();
case WINDOW_UPDATE_FRAME: case WINDOW_UPDATE_FRAME:
return GetWindowUpdateFrameSize(); return GetWindowUpdateFrameSize();
case BLOCKED_FRAME: case BLOCKED_FRAME:
...@@ -2373,7 +2378,7 @@ bool QuicFramer::AppendConnectionCloseFrame( ...@@ -2373,7 +2378,7 @@ bool QuicFramer::AppendConnectionCloseFrame(
if (!writer->WriteUInt32(error_code)) { if (!writer->WriteUInt32(error_code)) {
return false; return false;
} }
if (!writer->WriteStringPiece16(frame.error_details)) { if (!writer->WriteStringPiece16(TruncateErrorString(frame.error_details))) {
return false; return false;
} }
return true; return true;
...@@ -2389,7 +2394,7 @@ bool QuicFramer::AppendGoAwayFrame(const QuicGoAwayFrame& frame, ...@@ -2389,7 +2394,7 @@ bool QuicFramer::AppendGoAwayFrame(const QuicGoAwayFrame& frame,
if (!writer->WriteUInt32(stream_id)) { if (!writer->WriteUInt32(stream_id)) {
return false; return false;
} }
if (!writer->WriteStringPiece16(frame.reason_phrase)) { if (!writer->WriteStringPiece16(TruncateErrorString(frame.reason_phrase))) {
return false; return false;
} }
return true; return true;
...@@ -2466,4 +2471,13 @@ bool QuicFramer::StartsWithChlo(QuicStreamId id, ...@@ -2466,4 +2471,13 @@ bool QuicFramer::StartsWithChlo(QuicStreamId id,
0; 0;
} }
QuicStringPiece QuicFramer::TruncateErrorString(QuicStringPiece error) {
if (error.length() <= kMaxErrorStringLength ||
!FLAGS_quic_reloadable_flag_quic_truncate_long_details) {
return error;
}
QUIC_FLAG_COUNT(quic_reloadable_flag_quic_truncate_long_details);
return QuicStringPiece(error.data(), kMaxErrorStringLength);
}
} // namespace net } // namespace net
net/quic/core/quic_framer.h
View file @ 8ca037ff
...@@ -501,6 +501,8 @@ class QUIC_EXPORT_PRIVATE QuicFramer { ...@@ -501,6 +501,8 @@ class QUIC_EXPORT_PRIVATE QuicFramer {
void set_detailed_error(const char* error) { detailed_error_ = error; } void set_detailed_error(const char* error) { detailed_error_ = error; }
QuicStringPiece TruncateErrorString(QuicStringPiece error);
std::string detailed_error_; std::string detailed_error_;
QuicFramerVisitorInterface* visitor_; QuicFramerVisitorInterface* visitor_;
QuicErrorCode error_; QuicErrorCode error_;
......
net/quic/core/quic_framer_test.cc
View file @ 8ca037ff
...@@ -4712,6 +4712,136 @@ TEST_P(QuicFramerTest, BuildCloseFramePacket) { ...@@ -4712,6 +4712,136 @@ TEST_P(QuicFramerTest, BuildCloseFramePacket) {
arraysize(packet)); arraysize(packet));
} }
TEST_P(QuicFramerTest, BuildTruncatedCloseFramePacket) {
FLAGS_quic_reloadable_flag_quic_truncate_long_details = true;
QuicPacketHeader header;
header.connection_id = kConnectionId;
header.reset_flag = false;
header.version_flag = false;
header.packet_number = kPacketNumber;
QuicConnectionCloseFrame close_frame;
close_frame.error_code = static_cast<QuicErrorCode>(0x05060708);
close_frame.error_details = string(2048, 'A');
QuicFrames frames = {QuicFrame(&close_frame)};
// clang-format off
unsigned char packet[] = {
// public flags (8 byte connection_id)
0x38,
// connection_id
0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
// packet number
0xBC, 0x9A, 0x78, 0x56,
0x34, 0x12,
// frame type (connection close frame)
0x02,
// error code
0x08, 0x07, 0x06, 0x05,
// error details length
0x00, 0x01,
// error details (truncated to 256 bytes)
'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', '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', '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', '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',
'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', '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', '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', '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',
'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A',
'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A',
};
unsigned char packet39[] = {
// public flags (8 byte connection_id)
0x38,
// connection_id
0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
// packet number
0x12, 0x34, 0x56, 0x78,
0x9A, 0xBC,
// frame type (connection close frame)
0x02,
// error code
0x05, 0x06, 0x07, 0x08,
// error details length
0x01, 0x00,
// error details (truncated to 256 bytes)
'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', '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', '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', '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',
'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', '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', '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', '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',
'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A',
'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A',
};
// clang-format on
unsigned char* p = packet;
size_t packet_size = arraysize(packet);
if (framer_.transport_version() > QUIC_VERSION_38) {
p = packet39;
packet_size = arraysize(packet39);
}
std::unique_ptr<QuicPacket> data(BuildDataPacket(header, frames));
ASSERT_TRUE(data != nullptr);
test::CompareCharArraysWithHexError("constructed packet", data->data(),
data->length(), AsChars(p), packet_size);
}
TEST_P(QuicFramerTest, BuildGoAwayPacket) { TEST_P(QuicFramerTest, BuildGoAwayPacket) {
QuicPacketHeader header; QuicPacketHeader header;
header.connection_id = kConnectionId; header.connection_id = kConnectionId;
...@@ -4786,6 +4916,141 @@ TEST_P(QuicFramerTest, BuildGoAwayPacket) { ...@@ -4786,6 +4916,141 @@ TEST_P(QuicFramerTest, BuildGoAwayPacket) {
arraysize(packet)); arraysize(packet));
} }
TEST_P(QuicFramerTest, BuildTruncatedGoAwayPacket) {
FLAGS_quic_reloadable_flag_quic_truncate_long_details = true;
QuicPacketHeader header;
header.connection_id = kConnectionId;
header.reset_flag = false;
header.version_flag = false;
header.packet_number = kPacketNumber;
QuicGoAwayFrame goaway_frame;
goaway_frame.error_code = static_cast<QuicErrorCode>(0x05060708);
goaway_frame.last_good_stream_id = kStreamId;
goaway_frame.reason_phrase = string(2048, 'A');
QuicFrames frames = {QuicFrame(&goaway_frame)};
// clang-format off
unsigned char packet[] = {
// public flags (8 byte connection_id)
0x38,
// connection_id
0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
// packet number
0xBC, 0x9A, 0x78, 0x56,
0x34, 0x12,
// frame type (go away frame)
0x03,
// error code
0x08, 0x07, 0x06, 0x05,
// stream id
0x04, 0x03, 0x02, 0x01,
// error details length
0x00, 0x01,
// error details (truncated to 256 bytes)
'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', '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', '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', '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',
'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', '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', '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', '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',
'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A',
'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A',
};
unsigned char packet39[] = {
// public flags (8 byte connection_id)
0x38,
// connection_id
0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
// packet number
0x12, 0x34, 0x56, 0x78,
0x9A, 0xBC,
// frame type (go away frame)
0x03,
// error code
0x05, 0x06, 0x07, 0x08,
// stream id
0x01, 0x02, 0x03, 0x04,
// error details length
0x01, 0x00,
// error details (truncated to 256 bytes)
'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', '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', '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', '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',
'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', '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', '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', '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',
'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A',
'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A',
};
// clang-format on
unsigned char* p = packet;
size_t packet_size = arraysize(packet);
if (framer_.transport_version() > QUIC_VERSION_38) {
p = packet39;
packet_size = arraysize(packet39);
}
std::unique_ptr<QuicPacket> data(BuildDataPacket(header, frames));
ASSERT_TRUE(data != nullptr);
test::CompareCharArraysWithHexError("constructed packet", data->data(),
data->length(), AsChars(p), packet_size);
}
TEST_P(QuicFramerTest, BuildWindowUpdatePacket) { TEST_P(QuicFramerTest, BuildWindowUpdatePacket) {
QuicPacketHeader header; QuicPacketHeader header;
header.connection_id = kConnectionId; header.connection_id = kConnectionId;
......
net/quic/core/quic_packet_generator_test.cc
View file @ 8ca037ff
...@@ -1074,10 +1074,14 @@ TEST_F(QuicPacketGeneratorTest, ConnectionCloseFrameLargerThanPacketSize) { ...@@ -1074,10 +1074,14 @@ TEST_F(QuicPacketGeneratorTest, ConnectionCloseFrameLargerThanPacketSize) {
char buf[2000] = {}; char buf[2000] = {};
QuicStringPiece error_details(buf, 2000); QuicStringPiece error_details(buf, 2000);
frame->error_details = error_details.as_string(); frame->error_details = error_details.as_string();
EXPECT_CALL(delegate_, if (FLAGS_quic_reloadable_flag_quic_truncate_long_details) {
OnUnrecoverableError(QUIC_FAILED_TO_SERIALIZE_PACKET, generator_.AddControlFrame(QuicFrame(frame));
"Single frame cannot fit into a packet", _)); } else {
EXPECT_QUIC_BUG(generator_.AddControlFrame(QuicFrame(frame)), ""); EXPECT_CALL(delegate_, OnUnrecoverableError(
QUIC_FAILED_TO_SERIALIZE_PACKET,
"Single frame cannot fit into a packet", _));
EXPECT_QUIC_BUG(generator_.AddControlFrame(QuicFrame(frame)), "");
}
EXPECT_TRUE(generator_.HasQueuedFrames()); EXPECT_TRUE(generator_.HasQueuedFrames());
EXPECT_TRUE(generator_.HasRetransmittableFrames()); EXPECT_TRUE(generator_.HasRetransmittableFrames());
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment