Deprecate --gfe2_reloadable_flag_quic_ignore_old_priority_update_frame. PiperOrigin-RevId: 379531754
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc index 6d9290a..5fd6b65 100644 --- a/quic/core/http/http_decoder.cc +++ b/quic/core/http/http_decoder.cc
@@ -37,13 +37,8 @@ remaining_push_id_length_(0), error_(QUIC_NO_ERROR), error_detail_(""), - ignore_old_priority_update_( - GetQuicReloadableFlag(quic_ignore_old_priority_update_frame)), error_on_http3_push_(GetQuicReloadableFlag(quic_error_on_http3_push)) { QUICHE_DCHECK(visitor_); - if (ignore_old_priority_update_) { - QUIC_RELOADABLE_FLAG_COUNT(quic_ignore_old_priority_update_frame); - } if (error_on_http3_push_) { QUIC_RELOADABLE_FLAG_COUNT(quic_error_on_http3_push); } @@ -287,15 +282,6 @@ break; case static_cast<uint64_t>(HttpFrameType::MAX_PUSH_ID): break; - case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): - 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); break; @@ -432,14 +418,6 @@ continue_processing = BufferOrParsePayload(reader); break; } - case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): { - 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): { continue_processing = BufferOrParsePayload(reader); break; @@ -512,16 +490,6 @@ continue_processing = BufferOrParsePayload(reader); break; } - case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): { - 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): { // If frame payload is not empty, FinishParsing() is skipped. QUICHE_DCHECK_EQ(0u, current_frame_length_); @@ -670,18 +638,6 @@ } return visitor_->OnMaxPushIdFrame(frame); } - case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): { - if (ignore_old_priority_update_) { - QUICHE_NOTREACHED(); - return false; - } else { - PriorityUpdateFrame frame; - if (!ParsePriorityUpdateFrame(reader, &frame)) { - return false; - } - return visitor_->OnPriorityUpdateFrame(frame); - } - } case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM): { PriorityUpdateFrame frame; if (!ParseNewPriorityUpdateFrame(reader, &frame)) {
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h index 3a970fb..4d415b6 100644 --- a/quic/core/http/http_decoder.h +++ b/quic/core/http/http_decoder.h
@@ -300,10 +300,6 @@ 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_; - - // Latched value of // gfe2_reloadable_flag_quic_error_on_http3_push. const bool error_on_http3_push_; };
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc index 3f7fb02..bae0dc0 100644 --- a/quic/core/http/http_decoder_test.cc +++ b/quic/core/http/http_decoder_test.cc
@@ -1118,97 +1118,8 @@ EXPECT_EQ("", decoder_.error_detail()); } -TEST_F(HttpDecoderTest, OldPriorityUpdateFrame) { - if (GetQuicReloadableFlag(quic_ignore_old_priority_update_frame)) { - return; - } - - InSequence s; - std::string input1 = absl::HexStringToBytes( - "0f" // type (PRIORITY_UPDATE) - "02" // length - "00" // prioritized element type: REQUEST_STREAM - "03"); // prioritized element id - - PriorityUpdateFrame priority_update1; - priority_update1.prioritized_element_type = REQUEST_STREAM; - priority_update1.prioritized_element_id = 0x03; - - // Visitor pauses processing. - EXPECT_CALL(visitor_, OnPriorityUpdateFrameStart(2)).WillOnce(Return(false)); - absl::string_view remaining_input(input1); - QuicByteCount processed_bytes = - ProcessInputWithGarbageAppended(remaining_input); - EXPECT_EQ(2u, processed_bytes); - remaining_input = remaining_input.substr(processed_bytes); - - EXPECT_CALL(visitor_, OnPriorityUpdateFrame(priority_update1)) - .WillOnce(Return(false)); - processed_bytes = ProcessInputWithGarbageAppended(remaining_input); - EXPECT_EQ(remaining_input.size(), processed_bytes); - EXPECT_THAT(decoder_.error(), IsQuicNoError()); - EXPECT_EQ("", decoder_.error_detail()); - - // Process the full frame. - EXPECT_CALL(visitor_, OnPriorityUpdateFrameStart(2)); - EXPECT_CALL(visitor_, OnPriorityUpdateFrame(priority_update1)); - EXPECT_EQ(input1.size(), ProcessInput(input1)); - EXPECT_THAT(decoder_.error(), IsQuicNoError()); - EXPECT_EQ("", decoder_.error_detail()); - - // Process the frame incrementally. - EXPECT_CALL(visitor_, OnPriorityUpdateFrameStart(2)); - EXPECT_CALL(visitor_, OnPriorityUpdateFrame(priority_update1)); - ProcessInputCharByChar(input1); - EXPECT_THAT(decoder_.error(), IsQuicNoError()); - EXPECT_EQ("", decoder_.error_detail()); - - std::string input2 = absl::HexStringToBytes( - "0f" // type (PRIORITY_UPDATE) - "05" // length - "80" // prioritized element type: PUSH_STREAM - "05" // prioritized element id - "666f6f"); // priority field value: "foo" - - PriorityUpdateFrame priority_update2; - priority_update2.prioritized_element_type = PUSH_STREAM; - priority_update2.prioritized_element_id = 0x05; - priority_update2.priority_field_value = "foo"; - - // Visitor pauses processing. - EXPECT_CALL(visitor_, OnPriorityUpdateFrameStart(2)).WillOnce(Return(false)); - remaining_input = input2; - processed_bytes = ProcessInputWithGarbageAppended(remaining_input); - EXPECT_EQ(2u, processed_bytes); - remaining_input = remaining_input.substr(processed_bytes); - - EXPECT_CALL(visitor_, OnPriorityUpdateFrame(priority_update2)) - .WillOnce(Return(false)); - processed_bytes = ProcessInputWithGarbageAppended(remaining_input); - EXPECT_EQ(remaining_input.size(), processed_bytes); - EXPECT_THAT(decoder_.error(), IsQuicNoError()); - EXPECT_EQ("", decoder_.error_detail()); - - // Process the full frame. - EXPECT_CALL(visitor_, OnPriorityUpdateFrameStart(2)); - EXPECT_CALL(visitor_, OnPriorityUpdateFrame(priority_update2)); - EXPECT_EQ(input2.size(), ProcessInput(input2)); - EXPECT_THAT(decoder_.error(), IsQuicNoError()); - EXPECT_EQ("", decoder_.error_detail()); - - // Process the frame incrementally. - EXPECT_CALL(visitor_, OnPriorityUpdateFrameStart(2)); - EXPECT_CALL(visitor_, OnPriorityUpdateFrame(priority_update2)); - ProcessInputCharByChar(input2); - EXPECT_THAT(decoder_.error(), IsQuicNoError()); - EXPECT_EQ("", decoder_.error_detail()); -} - +// Old PRIORITY_UPDATE frame is parsed as unknown frame. 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; @@ -1322,46 +1233,6 @@ } 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 - std::string payload2 = - absl::HexStringToBytes("42"); // invalid prioritized element type - struct { - const char* const payload; - size_t payload_length; - const char* const error_message; - } kTestData[] = { - {payload1.data(), 0, "Unable to read prioritized element type."}, - {payload1.data(), 1, "Unable to read prioritized element id."}, - {payload1.data(), 2, "Unable to read prioritized element id."}, - {payload2.data(), 1, "Invalid prioritized element type."}, - }; - - for (const auto& test_data : kTestData) { - std::string input; - input.push_back(15u); // type PRIORITY_UPDATE - input.push_back(test_data.payload_length); - size_t header_length = input.size(); - input.append(test_data.payload, test_data.payload_length); - - HttpDecoder decoder(&visitor_); - EXPECT_CALL(visitor_, OnPriorityUpdateFrameStart(header_length)); - EXPECT_CALL(visitor_, OnError(&decoder)); - - QuicByteCount processed_bytes = - decoder.ProcessInput(input.data(), input.size()); - EXPECT_EQ(input.size(), processed_bytes); - EXPECT_THAT(decoder.error(), IsError(QUIC_HTTP_FRAME_ERROR)); - EXPECT_EQ(test_data.error_message, decoder.error_detail()); - } -} - -TEST_F(HttpDecoderTest, CorruptNewPriorityUpdateFrame) { std::string payload = absl::HexStringToBytes("4005"); // prioritized element id struct {
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index 02e19b4..40dd86b 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -99,8 +99,6 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_donot_reset_ideal_next_packet_send_time, false) // If true, time_wait_list can support multiple connection IDs. QUIC_FLAG(FLAGS_quic_restart_flag_quic_time_wait_list_support_multiple_cid_v2, true) -// If true, treat old (pre-draft02) PRIORITY_UPDATE frame as unknown frame. -QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_ignore_old_priority_update_frame, true) // If true, upon receiving path challenge, send path response and reverse path challenge in the same function. QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_group_path_response_and_challenge_sending_closer, true) // If true, use BBRv2 as the default congestion controller. Takes precedence over --quic_default_to_bbr.