Treat old (pre-draft02) PRIORITY_UPDATE frame as unknown frame. Protected by FLAGS_quic_reloadable_flag_quic_ignore_old_priority_update_frame. PiperOrigin-RevId: 370113693 Change-Id: I4c6f230aa58f17469391f2765384239cca72d09d
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc index 3544cfd..859d174 100644 --- a/quic/core/http/http_decoder.cc +++ b/quic/core/http/http_decoder.cc
@@ -36,8 +36,13 @@ current_push_id_length_(0), remaining_push_id_length_(0), error_(QUIC_NO_ERROR), - error_detail_("") { + error_detail_(""), + ignore_old_priority_update_( + GetQuicReloadableFlag(quic_ignore_old_priority_update_frame)) { QUICHE_DCHECK(visitor_); + if (ignore_old_priority_update_) { + QUIC_RELOADABLE_FLAG_COUNT(quic_ignore_old_priority_update_frame); + } } HttpDecoder::~HttpDecoder() {} @@ -257,7 +262,13 @@ case static_cast<uint64_t>(HttpFrameType::MAX_PUSH_ID): break; case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): - continue_processing = visitor_->OnPriorityUpdateFrameStart(header_length); + if (ignore_old_priority_update_) { + continue_processing = visitor_->OnUnknownFrameStart( + current_frame_type_, header_length, current_frame_length_); + } else { + continue_processing = + visitor_->OnPriorityUpdateFrameStart(header_length); + } break; case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM): continue_processing = visitor_->OnPriorityUpdateFrameStart(header_length); @@ -388,7 +399,11 @@ break; } case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): { - continue_processing = BufferOrParsePayload(reader); + if (ignore_old_priority_update_) { + continue_processing = HandleUnknownFramePayload(reader); + } else { + continue_processing = BufferOrParsePayload(reader); + } break; } case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM): { @@ -456,9 +471,13 @@ break; } case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): { - // If frame payload is not empty, FinishParsing() is skipped. - QUICHE_DCHECK_EQ(0u, current_frame_length_); - continue_processing = BufferOrParsePayload(reader); + if (ignore_old_priority_update_) { + continue_processing = visitor_->OnUnknownFrameEnd(); + } else { + // If frame payload is not empty, FinishParsing() is skipped. + QUICHE_DCHECK_EQ(0u, current_frame_length_); + continue_processing = BufferOrParsePayload(reader); + } break; } case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM): { @@ -606,11 +625,16 @@ return visitor_->OnMaxPushIdFrame(frame); } case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): { - PriorityUpdateFrame frame; - if (!ParsePriorityUpdateFrame(reader, &frame)) { + if (ignore_old_priority_update_) { + QUICHE_NOTREACHED(); return false; + } else { + PriorityUpdateFrame frame; + if (!ParsePriorityUpdateFrame(reader, &frame)) { + return false; + } + return visitor_->OnPriorityUpdateFrame(frame); } - return visitor_->OnPriorityUpdateFrame(frame); } case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM): { PriorityUpdateFrame frame;
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h index a351415..5580d44 100644 --- a/quic/core/http/http_decoder.h +++ b/quic/core/http/http_decoder.h
@@ -241,11 +241,13 @@ // Parses the payload of a PRIORITY_UPDATE frame (draft-01, type 0x0f) // from |reader| into |frame|. + // TODO(b/147306124): Remove. bool ParsePriorityUpdateFrame(QuicDataReader* reader, PriorityUpdateFrame* frame); // Parses the payload of a PRIORITY_UPDATE frame (draft-02, type 0xf0700) // from |reader| into |frame|. + // TODO(b/147306124): Rename to ParsePriorityUpdateFrame(). bool ParseNewPriorityUpdateFrame(QuicDataReader* reader, PriorityUpdateFrame* frame); @@ -291,6 +293,10 @@ std::array<char, sizeof(uint64_t)> type_buffer_; // Remaining unparsed push id data. std::array<char, sizeof(uint64_t)> push_id_buffer_; + + // Latched value of + // gfe2_reloadable_flag_quic_ignore_old_priority_update_frame. + const bool ignore_old_priority_update_; }; } // namespace quic
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc index aac020a..ab0e31f 100644 --- a/quic/core/http/http_decoder_test.cc +++ b/quic/core/http/http_decoder_test.cc
@@ -1003,7 +1003,11 @@ EXPECT_EQ("", decoder_.error_detail()); } -TEST_F(HttpDecoderTest, PriorityUpdateFrame) { +TEST_F(HttpDecoderTest, OldPriorityUpdateFrame) { + if (GetQuicReloadableFlag(quic_ignore_old_priority_update_frame)) { + return; + } + InSequence s; std::string input1 = absl::HexStringToBytes( "0f" // type (PRIORITY_UPDATE) @@ -1085,7 +1089,44 @@ EXPECT_EQ("", decoder_.error_detail()); } -TEST_F(HttpDecoderTest, NewPriorityUpdateFrame) { +TEST_F(HttpDecoderTest, ObsoletePriorityUpdateFrame) { + if (!GetQuicReloadableFlag(quic_ignore_old_priority_update_frame)) { + return; + } + + const QuicByteCount header_length = 2; + const QuicByteCount payload_length = 3; + InSequence s; + std::string input = absl::HexStringToBytes( + "0f" // type (obsolete PRIORITY_UPDATE) + "03" // length + "666f6f"); // payload "foo" + + // Process frame as a whole. + EXPECT_CALL(visitor_, + OnUnknownFrameStart(0x0f, header_length, payload_length)); + EXPECT_CALL(visitor_, OnUnknownFramePayload(Eq("foo"))); + EXPECT_CALL(visitor_, OnUnknownFrameEnd()).WillOnce(Return(false)); + + EXPECT_EQ(header_length + payload_length, + ProcessInputWithGarbageAppended(input)); + EXPECT_THAT(decoder_.error(), IsQuicNoError()); + EXPECT_EQ("", decoder_.error_detail()); + + // Process frame byte by byte. + EXPECT_CALL(visitor_, + OnUnknownFrameStart(0x0f, header_length, payload_length)); + EXPECT_CALL(visitor_, OnUnknownFramePayload(Eq("f"))); + EXPECT_CALL(visitor_, OnUnknownFramePayload(Eq("o"))); + EXPECT_CALL(visitor_, OnUnknownFramePayload(Eq("o"))); + EXPECT_CALL(visitor_, OnUnknownFrameEnd()); + + ProcessInputCharByChar(input); + EXPECT_THAT(decoder_.error(), IsQuicNoError()); + EXPECT_EQ("", decoder_.error_detail()); +} + +TEST_F(HttpDecoderTest, PriorityUpdateFrame) { InSequence s; std::string input1 = absl::HexStringToBytes( "800f0700" // type (PRIORITY_UPDATE) @@ -1166,6 +1207,10 @@ } TEST_F(HttpDecoderTest, CorruptPriorityUpdateFrame) { + if (GetQuicReloadableFlag(quic_ignore_old_priority_update_frame)) { + return; + } + std::string payload1 = absl::HexStringToBytes( "80" // prioritized element type: PUSH_STREAM "4005"); // prioritized element id
diff --git a/quic/core/http/http_frames.h b/quic/core/http/http_frames.h index 56e5348..b14ebe5 100644 --- a/quic/core/http/http_frames.h +++ b/quic/core/http/http_frames.h
@@ -29,6 +29,7 @@ GOAWAY = 0x7, MAX_PUSH_ID = 0xD, // https://tools.ietf.org/html/draft-ietf-httpbis-priority-01 + // TODO(b/147306124): Remove. PRIORITY_UPDATE = 0XF, // https://tools.ietf.org/html/draft-davidben-http-client-hint-reliability-02 ACCEPT_CH = 0x89,
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index 5de9c04..8025ea3 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -53,6 +53,7 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_willing_and_able_to_write2, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_group_path_response_and_challenge_sending_closer, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_h3_datagram, false) +QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_ignore_old_priority_update_frame, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_pass_path_response_to_validator, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_preempt_stream_data_with_handshake_packet, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_reject_unexpected_ietf_frame_types, true)