Implement new PRIORITY_UPDATE frame. PRIORITY_UPDATE frame type and wire format has changed in https://tools.ietf.org/html/draft-ietf-httpbis-priority-02. Implement new frame for request streams, but not the one for push streams since that is currently unused. Protected by FLAGS_quic_reloadable_flag_quic_new_priority_update_frame. PiperOrigin-RevId: 341724034 Change-Id: I42e63bbb84c62f54abc28989e9eaa24685ef1596
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc index ab21d34..1a160e9 100644 --- a/quic/core/http/http_decoder.cc +++ b/quic/core/http/http_decoder.cc
@@ -14,6 +14,7 @@ #include "net/third_party/quiche/src/quic/core/quic_error_codes.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" #include "net/third_party/quiche/src/quic/platform/api/quic_logging.h" namespace quic { @@ -236,6 +237,14 @@ case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): continue_processing = visitor_->OnPriorityUpdateFrameStart(header_length); break; + case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM): + if (GetQuicReloadableFlag(quic_new_priority_update_frame)) { + QUIC_CODE_COUNT_N(quic_new_priority_update_frame, 2, 2); + continue_processing = + visitor_->OnPriorityUpdateFrameStart(header_length); + break; + } + ABSL_FALLTHROUGH_INTENDED; default: continue_processing = visitor_->OnUnknownFrameStart( current_frame_type_, header_length, current_frame_length_); @@ -364,6 +373,15 @@ BufferFramePayload(reader); break; } + case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM): { + if (GetQuicReloadableFlag(quic_new_priority_update_frame)) { + // TODO(bnc): Avoid buffering if the entire frame is present, and + // instead parse directly out of |reader|. + BufferFramePayload(reader); + break; + } + ABSL_FALLTHROUGH_INTENDED; + } default: { QuicByteCount bytes_to_read = std::min<QuicByteCount>( remaining_frame_length_, reader->BytesRemaining()); @@ -468,6 +486,20 @@ continue_processing = visitor_->OnPriorityUpdateFrame(frame); break; } + case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM): { + if (GetQuicReloadableFlag(quic_new_priority_update_frame)) { + // TODO(bnc): Avoid buffering if the entire frame is present, and + // instead parse directly out of |reader|. + PriorityUpdateFrame frame; + QuicDataReader reader(buffer_.data(), current_frame_length_); + if (!ParseNewPriorityUpdateFrame(&reader, &frame)) { + return false; + } + continue_processing = visitor_->OnPriorityUpdateFrame(frame); + break; + } + ABSL_FALLTHROUGH_INTENDED; + } default: { continue_processing = visitor_->OnUnknownFrameEnd(); break; @@ -603,6 +635,22 @@ return true; } +bool HttpDecoder::ParseNewPriorityUpdateFrame(QuicDataReader* reader, + PriorityUpdateFrame* frame) { + frame->prioritized_element_type = REQUEST_STREAM; + + if (!reader->ReadVarInt62(&frame->prioritized_element_id)) { + RaiseError(QUIC_HTTP_FRAME_ERROR, "Unable to read prioritized element id."); + return false; + } + + absl::string_view priority_field_value = reader->ReadRemainingPayload(); + frame->priority_field_value = + std::string(priority_field_value.data(), priority_field_value.size()); + + return true; +} + QuicByteCount HttpDecoder::MaxFrameLength(uint64_t frame_type) { switch (frame_type) { case static_cast<uint64_t>(HttpFrameType::CANCEL_PUSH): @@ -617,6 +665,9 @@ case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): // This limit is arbitrary. return 1024 * 1024; + case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM): + // This limit is arbitrary. + return 1024 * 1024; default: // Other frames require no data buffering, so it's safe to have no limit. return std::numeric_limits<QuicByteCount>::max();
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h index 77937f5..479f444 100644 --- a/quic/core/http/http_decoder.h +++ b/quic/core/http/http_decoder.h
@@ -197,10 +197,16 @@ // Parses the payload of a SETTINGS frame from |reader| into |frame|. bool ParseSettingsFrame(QuicDataReader* reader, SettingsFrame* frame); - // Parses the payload of a PRIORITY_UPDATE frame from |reader| into |frame|. + // Parses the payload of a PRIORITY_UPDATE frame (draft-01, type 0x0f) + // from |reader| into |frame|. bool ParsePriorityUpdateFrame(QuicDataReader* reader, PriorityUpdateFrame* frame); + // Parses the payload of a PRIORITY_UPDATE frame (draft-02, type 0xf0700) + // from |reader| into |frame|. + bool ParseNewPriorityUpdateFrame(QuicDataReader* reader, + PriorityUpdateFrame* frame); + // Returns the max frame size of a given |frame_type|. QuicByteCount MaxFrameLength(uint64_t frame_type);
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc index 7702b0d..d56626f 100644 --- a/quic/core/http/http_decoder_test.cc +++ b/quic/core/http/http_decoder_test.cc
@@ -14,6 +14,7 @@ #include "net/third_party/quiche/src/quic/core/http/http_frames.h" #include "net/third_party/quiche/src/quic/core/quic_data_writer.h" #include "net/third_party/quiche/src/quic/core/quic_versions.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" #include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h" #include "net/third_party/quiche/src/common/platform/api/quiche_str_cat.h" @@ -1000,7 +1001,7 @@ EXPECT_EQ("", decoder_.error_detail()); std::string input2 = absl::HexStringToBytes( - "0f" // type (PRIORIRTY) + "0f" // type (PRIORITY_UPDATE) "05" // length "80" // prioritized element type: PUSH_STREAM "05" // prioritized element id @@ -1040,6 +1041,90 @@ EXPECT_EQ("", decoder_.error_detail()); } +TEST_F(HttpDecoderTest, NewPriorityUpdateFrame) { + if (!GetQuicReloadableFlag(quic_new_priority_update_frame)) { + return; + } + + InSequence s; + std::string input1 = absl::HexStringToBytes( + "800f0700" // type (PRIORITY_UPDATE) + "01" // length + "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(5)).WillOnce(Return(false)); + absl::string_view remaining_input(input1); + QuicByteCount processed_bytes = + ProcessInputWithGarbageAppended(remaining_input); + EXPECT_EQ(5u, 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(5)); + 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(5)); + EXPECT_CALL(visitor_, OnPriorityUpdateFrame(priority_update1)); + ProcessInputCharByChar(input1); + EXPECT_THAT(decoder_.error(), IsQuicNoError()); + EXPECT_EQ("", decoder_.error_detail()); + + std::string input2 = absl::HexStringToBytes( + "800f0700" // type (PRIORITY_UPDATE) + "04" // length + "05" // prioritized element id + "666f6f"); // priority field value: "foo" + + PriorityUpdateFrame priority_update2; + priority_update2.prioritized_element_type = REQUEST_STREAM; + priority_update2.prioritized_element_id = 0x05; + priority_update2.priority_field_value = "foo"; + + // Visitor pauses processing. + EXPECT_CALL(visitor_, OnPriorityUpdateFrameStart(5)).WillOnce(Return(false)); + remaining_input = input2; + processed_bytes = ProcessInputWithGarbageAppended(remaining_input); + EXPECT_EQ(5u, 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(5)); + 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(5)); + EXPECT_CALL(visitor_, OnPriorityUpdateFrame(priority_update2)); + ProcessInputCharByChar(input2); + EXPECT_THAT(decoder_.error(), IsQuicNoError()); + EXPECT_EQ("", decoder_.error_detail()); +} + TEST_F(HttpDecoderTest, CorruptPriorityUpdateFrame) { std::string payload1 = absl::HexStringToBytes( "80" // prioritized element type: PUSH_STREAM @@ -1076,6 +1161,40 @@ } } +TEST_F(HttpDecoderTest, CorruptNewPriorityUpdateFrame) { + if (!GetQuicReloadableFlag(quic_new_priority_update_frame)) { + return; + } + + std::string payload = + absl::HexStringToBytes("4005"); // prioritized element id + struct { + size_t payload_length; + const char* const error_message; + } kTestData[] = { + {0, "Unable to read prioritized element id."}, + {1, "Unable to read prioritized element id."}, + }; + + for (const auto& test_data : kTestData) { + std::string input = + absl::HexStringToBytes("800f0700"); // type PRIORITY_UPDATE + input.push_back(test_data.payload_length); + size_t header_length = input.size(); + input.append(payload.data(), 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, DecodeSettings) { std::string input = absl::HexStringToBytes( "04" // type (SETTINGS)
diff --git a/quic/core/http/http_encoder.cc b/quic/core/http/http_encoder.cc index 7707705..4f86d45 100644 --- a/quic/core/http/http_encoder.cc +++ b/quic/core/http/http_encoder.cc
@@ -202,6 +202,37 @@ QuicByteCount HttpEncoder::SerializePriorityUpdateFrame( const PriorityUpdateFrame& priority_update, std::unique_ptr<char[]>* output) { + if (GetQuicReloadableFlag(quic_new_priority_update_frame)) { + QUIC_CODE_COUNT_N(quic_new_priority_update_frame, 1, 2); + + if (priority_update.prioritized_element_type != REQUEST_STREAM) { + QUIC_BUG << "PRIORITY_UPDATE for push streams not implemented"; + return 0; + } + + QuicByteCount payload_length = + QuicDataWriter::GetVarInt62Len(priority_update.prioritized_element_id) + + priority_update.priority_field_value.size(); + QuicByteCount total_length = GetTotalLength( + payload_length, HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM); + + output->reset(new char[total_length]); + QuicDataWriter writer(total_length, output->get()); + + if (WriteFrameHeader(payload_length, + HttpFrameType::PRIORITY_UPDATE_REQUEST_STREAM, + &writer) && + writer.WriteVarInt62(priority_update.prioritized_element_id) && + writer.WriteBytes(priority_update.priority_field_value.data(), + priority_update.priority_field_value.size())) { + return total_length; + } + + QUIC_DLOG(ERROR) << "Http encoder failed when attempting to serialize " + "PRIORITY_UPDATE frame."; + return 0; + } + QuicByteCount payload_length = kPriorityFirstByteLength + QuicDataWriter::GetVarInt62Len(priority_update.prioritized_element_id) +
diff --git a/quic/core/http/http_encoder_test.cc b/quic/core/http/http_encoder_test.cc index 50afdd3..2466326 100644 --- a/quic/core/http/http_encoder_test.cc +++ b/quic/core/http/http_encoder_test.cc
@@ -5,6 +5,7 @@ #include "net/third_party/quiche/src/quic/core/http/http_encoder.h" #include "absl/base/macros.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" #include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h" #include "net/third_party/quiche/src/common/test_tools/quiche_test_utils.h" @@ -133,6 +134,24 @@ } TEST(HttpEncoderTest, SerializePriorityUpdateFrame) { + if (GetQuicReloadableFlag(quic_new_priority_update_frame)) { + PriorityUpdateFrame priority_update1; + priority_update1.prioritized_element_type = REQUEST_STREAM; + priority_update1.prioritized_element_id = 0x03; + char output1[] = {0x80, 0x0f, 0x07, 0x00, // type (PRIORITY_UPDATE) + 0x01, // length + 0x03}; // prioritized element id + + std::unique_ptr<char[]> buffer; + uint64_t length = + HttpEncoder::SerializePriorityUpdateFrame(priority_update1, &buffer); + EXPECT_EQ(ABSL_ARRAYSIZE(output1), length); + quiche::test::CompareCharArraysWithHexError("PRIORITY_UPDATE", buffer.get(), + length, output1, + ABSL_ARRAYSIZE(output1)); + return; + } + PriorityUpdateFrame priority_update1; priority_update1.prioritized_element_type = REQUEST_STREAM; priority_update1.prioritized_element_id = 0x03;
diff --git a/quic/core/http/http_frames.h b/quic/core/http/http_frames.h index 00df2bc..ed7ad2c 100644 --- a/quic/core/http/http_frames.h +++ b/quic/core/http/http_frames.h
@@ -18,7 +18,7 @@ namespace quic { -enum class HttpFrameType : uint8_t { +enum class HttpFrameType { DATA = 0x0, HEADERS = 0x1, CANCEL_PUSH = 0X3, @@ -26,7 +26,10 @@ PUSH_PROMISE = 0x5, GOAWAY = 0x7, MAX_PUSH_ID = 0xD, + // https://tools.ietf.org/html/draft-ietf-httpbis-priority-01 PRIORITY_UPDATE = 0XF, + // https://tools.ietf.org/html/draft-ietf-httpbis-priority-02 + PRIORITY_UPDATE_REQUEST_STREAM = 0xF0700, }; // 7.2.1. DATA @@ -132,8 +135,11 @@ // https://httpwg.org/http-extensions/draft-ietf-httpbis-priority.html // -// The PRIORITY_UPDATE (type=0x0f) frame specifies the sender-advised priority -// of a stream +// The PRIORITY_UPDATE frame specifies the sender-advised priority of a stream. +// https://tools.ietf.org/html/draft-ietf-httpbis-priority-01 uses frame type +// 0x0f, both for request streams and push streams. +// https://tools.ietf.org/html/draft-ietf-httpbis-priority-02 uses frame types +// 0xf0700 for request streams and 0xf0701 for push streams (not implemented). // Length of a priority frame's first byte. const QuicByteCount kPriorityFirstByteLength = 1;
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index 4ad3d64..5590db2 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -59,6 +59,7 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_granular_qpack_error_codes, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_key_update_supported, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_let_connection_handle_pings, true) +QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_new_priority_update_frame, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_process_undecryptable_packets_after_async_decrypt_callback, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_record_received_min_ack_delay, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_reject_spdy_frames, true)