From 431bb4fda45880ce104a305fe07675d1b86e405f Mon Sep 17 00:00:00 2001 From: "rch@chromium.org" Date: Fri, 19 Oct 2012 17:46:09 +0000 Subject: Adding (but not widely using) a reason phrase to Rst and ConnectionClose for QUIC. Merging internal change 35422119 Review URL: https://chromiumcodereview.appspot.com/11185008 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@163019 0039d316-1c4b-4281-b951-d872f2087c98 --- net/quic/quic_data_writer.h | 9 +++++++ net/quic/quic_framer.cc | 53 +++++++++++++++++++++++++++---------- net/quic/quic_framer_test.cc | 63 +++++++++++++++++++++++++++++++++++--------- net/quic/quic_protocol.h | 12 +++++---- 4 files changed, 106 insertions(+), 31 deletions(-) diff --git a/net/quic/quic_data_writer.h b/net/quic/quic_data_writer.h index fa0e05a..8178ad7 100644 --- a/net/quic/quic_data_writer.h +++ b/net/quic/quic_data_writer.h @@ -64,6 +64,15 @@ class NET_EXPORT_PRIVATE QuicDataWriter { bool WriteUInt128(uint128 value) { return WriteUInt64(value.lo) && WriteUInt64(value.hi); } + bool WriteStringPiece16(base::StringPiece val) { + if (val.length() > std::numeric_limits::max()) { + return false; + } + if (!WriteUInt16(val.size())) { + return false; + } + return WriteBytes(val.data(), val.size()); + } bool AdvancePointer(uint32 len); diff --git a/net/quic/quic_framer.cc b/net/quic/quic_framer.cc index e392306e2..a706a22 100644 --- a/net/quic/quic_framer.cc +++ b/net/quic/quic_framer.cc @@ -503,12 +503,19 @@ bool QuicFramer::ProcessRstStreamFragment() { return false; } - uint32 details; - if (!reader_->ReadUInt32(&details)) { - set_detailed_error("Unable to read rst stream details."); + uint32 error_code; + if (!reader_->ReadUInt32(&error_code)) { + set_detailed_error("Unable to read rst stream error code."); return false; } - fragment.details = static_cast(details); + fragment.error_code = static_cast(error_code); + + StringPiece error_details; + if (!reader_->ReadStringPiece16(&error_details)) { + set_detailed_error("Unable to read rst stream error details."); + return false; + } + fragment.error_details = error_details.as_string(); visitor_->OnRstStreamFragment(fragment); return true; @@ -517,12 +524,19 @@ bool QuicFramer::ProcessRstStreamFragment() { bool QuicFramer::ProcessConnectionCloseFragment() { QuicConnectionCloseFragment fragment; - uint32 details; - if (!reader_->ReadUInt32(&details)) { - set_detailed_error("Unable to read connection close details."); + uint32 error_code; + if (!reader_->ReadUInt32(&error_code)) { + set_detailed_error("Unable to read connection close error code."); + return false; + } + fragment.error_code = static_cast(error_code); + + StringPiece error_details; + if (!reader_->ReadStringPiece16(&error_details)) { + set_detailed_error("Unable to read connection close error details."); return false; } - fragment.details = static_cast(details); + fragment.error_details = error_details.as_string(); if (!ProcessAckFragment(&fragment.ack_fragment)) { DLOG(WARNING) << "Unable to process ack fragment."; @@ -620,10 +634,14 @@ size_t QuicFramer::ComputeFragmentPayloadLength(const QuicFragment& fragment) { case RST_STREAM_FRAGMENT: len += 4; // stream id len += 8; // offset - len += 4; // details + len += 4; // error code + len += 2; // error details size + len += fragment.rst_stream_fragment->error_details.size(); break; case CONNECTION_CLOSE_FRAGMENT: - len += 4; // details + len += 4; // error code + len += 2; // error details size + len += fragment.connection_close_fragment->error_details.size(); len += ComputeFragmentPayloadLength( QuicFragment(&fragment.connection_close_fragment->ack_fragment)); break; @@ -755,8 +773,12 @@ bool QuicFramer::AppendRstStreamFragmentPayload( return false; } - uint32 details = static_cast(fragment.details); - if (!writer->WriteUInt32(details)) { + uint32 error_code = static_cast(fragment.error_code); + if (!writer->WriteUInt32(error_code)) { + return false; + } + + if (!writer->WriteStringPiece16(fragment.error_details)) { return false; } return true; @@ -765,8 +787,11 @@ bool QuicFramer::AppendRstStreamFragmentPayload( bool QuicFramer::AppendConnectionCloseFragmentPayload( const QuicConnectionCloseFragment& fragment, QuicDataWriter* writer) { - uint32 details = static_cast(fragment.details); - if (!writer->WriteUInt32(details)) { + uint32 error_code = static_cast(fragment.error_code); + if (!writer->WriteUInt32(error_code)) { + return false; + } + if (!writer->WriteStringPiece16(fragment.error_details)) { return false; } AppendAckFragmentPayload(fragment.ack_fragment, writer); diff --git a/net/quic/quic_framer_test.cc b/net/quic/quic_framer_test.cc index 61ae0a8..db8aa1c 100644 --- a/net/quic/quic_framer_test.cc +++ b/net/quic/quic_framer_test.cc @@ -1079,8 +1079,16 @@ TEST_F(QuicFramerTest, RstStreamFragment) { // offset 0x54, 0x76, 0x10, 0x32, 0xDC, 0xFE, 0x98, 0xBA, - // details + // error code 0x08, 0x07, 0x06, 0x05, + + // error details length + 0x0d, 0x00, + // error details + 'b', 'e', 'c', 'a', + 'u', 's', 'e', ' ', + 'I', ' ', 'c', 'a', + 'n', }; EXPECT_TRUE(framer_.ProcessPacket( @@ -1093,19 +1101,22 @@ TEST_F(QuicFramerTest, RstStreamFragment) { ASSERT_EQ(address_, visitor_.address_); EXPECT_EQ(GG_UINT64_C(0x01020304), visitor_.rst_stream_fragment_.stream_id); - EXPECT_EQ(0x05060708, visitor_.rst_stream_fragment_.details); + EXPECT_EQ(0x05060708, visitor_.rst_stream_fragment_.error_code); EXPECT_EQ(GG_UINT64_C(0xBA98FEDC32107654), visitor_.rst_stream_fragment_.offset); + EXPECT_EQ("because I can", visitor_.rst_stream_fragment_.error_details); // Now test framing boundaries - for (size_t i = kPacketHeaderSize + 3; i < kPacketHeaderSize + 18; ++i) { + for (size_t i = kPacketHeaderSize + 3; i < kPacketHeaderSize + 33; ++i) { string expected_error; if (i < kPacketHeaderSize + 6) { expected_error = "Unable to read stream_id."; } else if (i < kPacketHeaderSize + 14) { expected_error = "Unable to read offset in rst fragment."; } else if (i < kPacketHeaderSize + 18) { - expected_error = "Unable to read rst stream details."; + expected_error = "Unable to read rst stream error code."; + } else if (i < kPacketHeaderSize + 33) { + expected_error = "Unable to read rst stream error details."; } EXPECT_FALSE(framer_.ProcessPacket( address_, QuicEncryptedPacket(AsChars(packet), i, false))); @@ -1137,9 +1148,17 @@ TEST_F(QuicFramerTest, ConnectionCloseFragment) { 0x01, // fragment type (connection close fragment) 0x04, - // details + // error code 0x08, 0x07, 0x06, 0x05, + // error details length + 0x0d, 0x00, + // error details + 'b', 'e', 'c', 'a', + 'u', 's', 'e', ' ', + 'I', ' ', 'c', 'a', + 'n', + // Ack fragment. // largest received packet sequence number @@ -1190,7 +1209,8 @@ TEST_F(QuicFramerTest, ConnectionCloseFragment) { EXPECT_EQ(0u, visitor_.stream_fragments_.size()); - EXPECT_EQ(0x05060708, visitor_.connection_close_fragment_.details); + EXPECT_EQ(0x05060708, visitor_.connection_close_fragment_.error_code); + EXPECT_EQ("because I can", visitor_.connection_close_fragment_.error_details); ASSERT_EQ(1u, visitor_.ack_fragments_.size()); const QuicAckFragment& fragment = *visitor_.ack_fragments_[0]; @@ -1220,11 +1240,14 @@ TEST_F(QuicFramerTest, ConnectionCloseFragment) { fragment.congestion_info.inter_arrival.delta_time); // Now test framing boundaries - for (size_t i = kPacketHeaderSize + 3; i < kPacketHeaderSize + 6; ++i) { + for (size_t i = kPacketHeaderSize + 3; i < kPacketHeaderSize + 21; ++i) { string expected_error; if (i < kPacketHeaderSize + 6) { - expected_error = "Unable to read connection close details."; + expected_error = "Unable to read connection close error code."; + } else if (i < kPacketHeaderSize + 21) { + expected_error = "Unable to read connection close error details."; } + EXPECT_FALSE(framer_.ProcessPacket( address_, QuicEncryptedPacket(AsChars(packet), i, false))); EXPECT_EQ(expected_error, framer_.detailed_error()); @@ -1877,8 +1900,9 @@ TEST_F(QuicFramerTest, ConstructRstFragmentPacket) { QuicRstStreamFragment rst_fragment; rst_fragment.stream_id = 0x01020304; - rst_fragment.details = static_cast(0x05060708); + rst_fragment.error_code = static_cast(0x05060708); rst_fragment.offset = GG_UINT64_C(0xBA98FEDC32107654); + rst_fragment.error_details = "because I can"; unsigned char packet[] = { // guid @@ -1906,8 +1930,15 @@ TEST_F(QuicFramerTest, ConstructRstFragmentPacket) { // offset 0x54, 0x76, 0x10, 0x32, 0xDC, 0xFE, 0x98, 0xBA, - // details + // error code 0x08, 0x07, 0x06, 0x05, + // error details length + 0x0d, 0x00, + // error details + 'b', 'e', 'c', 'a', + 'u', 's', 'e', ' ', + 'I', ' ', 'c', 'a', + 'n', }; QuicFragment fragment(&rst_fragment); @@ -1935,7 +1966,8 @@ TEST_F(QuicFramerTest, ConstructCloseFragmentPacket) { header.fec_group = 0; QuicConnectionCloseFragment close_fragment; - close_fragment.details = static_cast(0x05060708); + close_fragment.error_code = static_cast(0x05060708); + close_fragment.error_details = "because I can"; QuicAckFragment* ack_fragment = &close_fragment.ack_fragment; ack_fragment->received_info.largest_received = GG_UINT64_C(0x0123456789ABC); @@ -1983,8 +2015,15 @@ TEST_F(QuicFramerTest, ConstructCloseFragmentPacket) { 0x01, // fragment type (connection close fragment) 0x04, - // details + // error code 0x08, 0x07, 0x06, 0x05, + // error details length + 0x0d, 0x00, + // error details + 'b', 'e', 'c', 'a', + 'u', 's', 'e', ' ', + 'I', ' ', 'c', 'a', + 'n', // Ack fragment. diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h index d096fff..a76817e 100644 --- a/net/quic/quic_protocol.h +++ b/net/quic/quic_protocol.h @@ -277,19 +277,21 @@ struct NET_EXPORT_PRIVATE QuicAckFragment { struct NET_EXPORT_PRIVATE QuicRstStreamFragment { QuicRstStreamFragment() {} QuicRstStreamFragment(QuicStreamId stream_id, uint64 offset, - QuicErrorCode details) - : stream_id(stream_id), offset(offset), details(details) { - DCHECK_LE(details, std::numeric_limits::max()); + QuicErrorCode error_code) + : stream_id(stream_id), offset(offset), error_code(error_code) { + DCHECK_LE(error_code, std::numeric_limits::max()); } QuicStreamId stream_id; uint64 offset; - QuicErrorCode details; + QuicErrorCode error_code; + std::string error_details; }; struct NET_EXPORT_PRIVATE QuicConnectionCloseFragment { - QuicErrorCode details; + QuicErrorCode error_code; QuicAckFragment ack_fragment; + std::string error_details; }; struct NET_EXPORT_PRIVATE QuicFragment { -- cgit v1.1