Use structured headers parser for PRIORITY_UPDATE frames. Use structured headers parser to parse HTTP/3 PRIORITY_UPDATE frame Priority Field Value field. Also, do not reset the stream when receiving urgency parameter with out-of-range value or unexpected type, in accordance with RFC9218. Protected by FLAGS_quic_reloadable_flag_quic_priority_update_structured_headers_parser. PiperOrigin-RevId: 491332082
diff --git a/quiche/quic/core/http/quic_receive_control_stream.cc b/quiche/quic/core/http/quic_receive_control_stream.cc index 3c1f3e7..a35a925 100644 --- a/quiche/quic/core/http/quic_receive_control_stream.cc +++ b/quiche/quic/core/http/quic_receive_control_stream.cc
@@ -16,6 +16,7 @@ #include "quiche/quic/platform/api/quic_flag_utils.h" #include "quiche/quic/platform/api/quic_flags.h" #include "quiche/common/quiche_text_utils.h" +#include "quiche/common/structured_headers.h" namespace quic { @@ -135,7 +136,23 @@ spdy_session()->debug_visitor()->OnPriorityUpdateFrameReceived(frame); } - // TODO(b/147306124): Use a proper structured headers parser instead. + if (GetQuicReloadableFlag(quic_priority_update_structured_headers_parser)) { + QUIC_RELOADABLE_FLAG_COUNT(quic_priority_update_structured_headers_parser); + const ParsePriorityFieldValueResult result = + ParsePriorityFieldValue(frame.priority_field_value); + + if (!result.success) { + stream_delegate()->OnStreamError( + QUIC_INVALID_PRIORITY_UPDATE, + "Invalid PRIORITY_UPDATE frame payload."); + return false; + } + + const QuicStreamId stream_id = frame.prioritized_element_id; + return spdy_session_->OnPriorityUpdateForRequestStream(stream_id, + result.urgency); + } + for (absl::string_view key_value : absl::StrSplit(frame.priority_field_value, ',')) { std::vector<absl::string_view> key_and_value = @@ -152,6 +169,8 @@ absl::string_view value = key_and_value[1]; int urgency; + // This violates RFC9218 Section 4: "priority parameters with out-of-range + // values, or values of unexpected types MUST be ignored". if (!absl::SimpleAtoi(value, &urgency) || urgency < 0 || urgency > 7) { stream_delegate()->OnStreamError( QUIC_INVALID_PRIORITY_UPDATE, @@ -206,6 +225,45 @@ return true; } +QuicReceiveControlStream::ParsePriorityFieldValueResult +QuicReceiveControlStream::ParsePriorityFieldValue( + absl::string_view priority_field_value) { + // Default values + int urgency = 3; + bool incremental = false; + + absl::optional<quiche::structured_headers::Dictionary> parsed_dictionary = + quiche::structured_headers::ParseDictionary(priority_field_value); + if (!parsed_dictionary.has_value()) { + return {false, urgency, incremental}; + } + + for (const auto& [name, value] : *parsed_dictionary) { + if (value.member_is_inner_list) { + continue; + } + + const std::vector<quiche::structured_headers::ParameterizedItem>& member = + value.member; + if (member.size() != 1) { + continue; + } + + const quiche::structured_headers::Item item = member[0].item; + if (name == "u" && item.is_integer()) { + int parsed_urgency = item.GetInteger(); + // Ignore out-of-range values. + if (parsed_urgency >= 0 && parsed_urgency <= 7) { + urgency = parsed_urgency; + } + } else if (name == "i" && item.is_boolean()) { + incremental = item.GetBoolean(); + } + } + + return {true, urgency, incremental}; +} + bool QuicReceiveControlStream::OnUnknownFrameEnd() { // Ignore unknown frame types. return true;
diff --git a/quiche/quic/core/http/quic_receive_control_stream.h b/quiche/quic/core/http/quic_receive_control_stream.h index 71d0bf2..e5eeb01 100644 --- a/quiche/quic/core/http/quic_receive_control_stream.h +++ b/quiche/quic/core/http/quic_receive_control_stream.h
@@ -20,6 +20,13 @@ : public QuicStream, public HttpDecoder::Visitor { public: + // Return type of ParsePriorityFieldValue(). + struct ParsePriorityFieldValueResult { + bool success; + int urgency; + bool incremental; + }; + explicit QuicReceiveControlStream(PendingStream* pending, QuicSpdySession* spdy_session); QuicReceiveControlStream(const QuicReceiveControlStream&) = delete; @@ -60,6 +67,10 @@ QuicSpdySession* spdy_session() { return spdy_session_; } + // Parses the Priority Field Value field of a PRIORITY_UPDATE frame. + static ParsePriorityFieldValueResult ParsePriorityFieldValue( + absl::string_view priority_field_value); + private: // Called when a frame of allowed type is received. Returns true if the frame // is allowed in this position. Returns false and resets the stream
diff --git a/quiche/quic/core/http/quic_receive_control_stream_test.cc b/quiche/quic/core/http/quic_receive_control_stream_test.cc index af7e5d4..9181ba6 100644 --- a/quiche/quic/core/http/quic_receive_control_stream_test.cc +++ b/quiche/quic/core/http/quic_receive_control_stream_test.cc
@@ -456,6 +456,71 @@ /* offset = */ 1, unknown_frame)); } +TEST(ParsePriorityFieldValueTest, ParsePriorityFieldValue) { + // Default values + QuicReceiveControlStream::ParsePriorityFieldValueResult result = + QuicReceiveControlStream::ParsePriorityFieldValue(""); + EXPECT_TRUE(result.success); + EXPECT_EQ(3, result.urgency); + EXPECT_FALSE(result.incremental); + + result = QuicReceiveControlStream::ParsePriorityFieldValue("i=?1"); + EXPECT_TRUE(result.success); + EXPECT_EQ(3, result.urgency); + EXPECT_TRUE(result.incremental); + + result = QuicReceiveControlStream::ParsePriorityFieldValue("u=5"); + EXPECT_TRUE(result.success); + EXPECT_EQ(5, result.urgency); + EXPECT_FALSE(result.incremental); + + result = QuicReceiveControlStream::ParsePriorityFieldValue("u=5, i"); + EXPECT_TRUE(result.success); + EXPECT_EQ(5, result.urgency); + EXPECT_TRUE(result.incremental); + + result = QuicReceiveControlStream::ParsePriorityFieldValue("i, u=1"); + EXPECT_TRUE(result.success); + EXPECT_EQ(1, result.urgency); + EXPECT_TRUE(result.incremental); + + // Duplicate values are allowed. + result = + QuicReceiveControlStream::ParsePriorityFieldValue("u=5, i=?1, i=?0, u=2"); + EXPECT_TRUE(result.success); + EXPECT_EQ(2, result.urgency); + EXPECT_FALSE(result.incremental); + + // Unknown parameters MUST be ignored. + result = QuicReceiveControlStream::ParsePriorityFieldValue("a=42, u=4, i=?0"); + EXPECT_TRUE(result.success); + EXPECT_EQ(4, result.urgency); + EXPECT_FALSE(result.incremental); + + // Out-of-range values MUST be ignored. + result = QuicReceiveControlStream::ParsePriorityFieldValue("u=-2, i"); + EXPECT_TRUE(result.success); + EXPECT_EQ(3, result.urgency); + EXPECT_TRUE(result.incremental); + + // Values of unexpected types MUST be ignored. + result = + QuicReceiveControlStream::ParsePriorityFieldValue("u=4.2, i=\"foo\""); + EXPECT_TRUE(result.success); + EXPECT_EQ(3, result.urgency); + EXPECT_FALSE(result.incremental); + + // Values of the right type but different names are ignored. + result = QuicReceiveControlStream::ParsePriorityFieldValue("a=4, b=?1"); + EXPECT_TRUE(result.success); + EXPECT_EQ(3, result.urgency); + EXPECT_FALSE(result.incremental); + + // Cannot be parsed as structured headers. + result = QuicReceiveControlStream::ParsePriorityFieldValue("000"); + EXPECT_FALSE(result.success); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quiche/quic/core/http/quic_spdy_session.cc b/quiche/quic/core/http/quic_spdy_session.cc index 66e5b8e..e9effda 100644 --- a/quiche/quic/core/http/quic_spdy_session.cc +++ b/quiche/quic/core/http/quic_spdy_session.cc
@@ -377,10 +377,7 @@ } void OnPriorityUpdate(SpdyStreamId /*prioritized_stream_id*/, - absl::string_view /*priority_field_value*/) override { - // TODO(b/171470299): Parse and call - // QuicSpdySession::OnPriorityUpdateForRequestStream(). - } + absl::string_view /*priority_field_value*/) override {} bool OnUnknownFrame(SpdyStreamId /*stream_id*/, uint8_t /*frame_type*/) override {
diff --git a/quiche/quic/core/http/quic_spdy_session_test.cc b/quiche/quic/core/http/quic_spdy_session_test.cc index aa7ba10..81a8999 100644 --- a/quiche/quic/core/http/quic_spdy_session_test.cc +++ b/quiche/quic/core/http/quic_spdy_session_test.cc
@@ -2244,6 +2244,108 @@ EXPECT_EQ(2u, stream2->precedence().spdy3_priority()); } +TEST_P(QuicSpdySessionTestServer, OnInvalidPriorityUpdateFrame) { + if (!VersionUsesHttp3(transport_version())) { + return; + } + + StrictMock<MockHttp3DebugVisitor> debug_visitor; + session_.set_debug_visitor(&debug_visitor); + + // Create control stream. + QuicStreamId receive_control_stream_id = + GetNthClientInitiatedUnidirectionalStreamId(transport_version(), 3); + char type[] = {kControlStream}; + absl::string_view stream_type(type, 1); + QuicStreamOffset offset = 0; + QuicStreamFrame data1(receive_control_stream_id, false, offset, stream_type); + offset += stream_type.length(); + EXPECT_CALL(debug_visitor, + OnPeerControlStreamCreated(receive_control_stream_id)); + session_.OnStreamFrame(data1); + EXPECT_EQ(receive_control_stream_id, + QuicSpdySessionPeer::GetReceiveControlStream(&session_)->id()); + + // Send SETTINGS frame. + std::string serialized_settings = HttpEncoder::SerializeSettingsFrame({}); + QuicStreamFrame data2(receive_control_stream_id, false, offset, + serialized_settings); + offset += serialized_settings.length(); + EXPECT_CALL(debug_visitor, OnSettingsFrameReceived(_)); + session_.OnStreamFrame(data2); + + // PRIORITY_UPDATE frame with Priority Field Value that is not valid + // Structured Headers. + const QuicStreamId stream_id = GetNthClientInitiatedBidirectionalId(0); + PriorityUpdateFrame priority_update{stream_id, "00"}; + + EXPECT_CALL(debug_visitor, OnPriorityUpdateFrameReceived(priority_update)); + if (GetQuicReloadableFlag(quic_priority_update_structured_headers_parser)) { + EXPECT_CALL(*connection_, + CloseConnection(QUIC_INVALID_PRIORITY_UPDATE, + "Invalid PRIORITY_UPDATE frame payload.", _)); + } else { + EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0); + } + + std::string serialized_priority_update = + HttpEncoder::SerializePriorityUpdateFrame(priority_update); + QuicStreamFrame data3(receive_control_stream_id, + /* fin = */ false, offset, serialized_priority_update); + session_.OnStreamFrame(data3); +} + +TEST_P(QuicSpdySessionTestServer, OnPriorityUpdateFrameOutOfBoundsUrgency) { + if (!VersionUsesHttp3(transport_version())) { + return; + } + + StrictMock<MockHttp3DebugVisitor> debug_visitor; + session_.set_debug_visitor(&debug_visitor); + + // Create control stream. + QuicStreamId receive_control_stream_id = + GetNthClientInitiatedUnidirectionalStreamId(transport_version(), 3); + char type[] = {kControlStream}; + absl::string_view stream_type(type, 1); + QuicStreamOffset offset = 0; + QuicStreamFrame data1(receive_control_stream_id, false, offset, stream_type); + offset += stream_type.length(); + EXPECT_CALL(debug_visitor, + OnPeerControlStreamCreated(receive_control_stream_id)); + session_.OnStreamFrame(data1); + EXPECT_EQ(receive_control_stream_id, + QuicSpdySessionPeer::GetReceiveControlStream(&session_)->id()); + + // Send SETTINGS frame. + std::string serialized_settings = HttpEncoder::SerializeSettingsFrame({}); + QuicStreamFrame data2(receive_control_stream_id, false, offset, + serialized_settings); + offset += serialized_settings.length(); + EXPECT_CALL(debug_visitor, OnSettingsFrameReceived(_)); + session_.OnStreamFrame(data2); + + // PRIORITY_UPDATE frame with urgency not in [0,7]. + const QuicStreamId stream_id = GetNthClientInitiatedBidirectionalId(0); + PriorityUpdateFrame priority_update{stream_id, "u=9"}; + + EXPECT_CALL(debug_visitor, OnPriorityUpdateFrameReceived(priority_update)); + if (GetQuicReloadableFlag(quic_priority_update_structured_headers_parser)) { + EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0); + } else { + EXPECT_CALL(*connection_, + CloseConnection( + QUIC_INVALID_PRIORITY_UPDATE, + "Invalid value for PRIORITY_UPDATE urgency parameter.", _)); + } + + std::string serialized_priority_update = + HttpEncoder::SerializePriorityUpdateFrame(priority_update); + QuicStreamFrame data3(receive_control_stream_id, + /* fin = */ false, offset, serialized_priority_update); + session_.OnStreamFrame(data3); +} + TEST_P(QuicSpdySessionTestServer, SimplePendingStreamType) { if (!VersionUsesHttp3(transport_version())) { return;
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index d88f628..0b148a1 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -87,6 +87,8 @@ QUIC_FLAG(quic_reloadable_flag_quic_connection_migration_use_new_cid_v2, true) // If true, use next_connection_id_sequence_number to validate retired cid number. QUIC_FLAG(quic_reloadable_flag_quic_check_retire_cid_with_next_cid_sequence_number, true) +// If true, use quiche/common/structured_headers in QuicReceiveControlStream::OnPriorityUpdateFrame(). +QUIC_FLAG(quic_reloadable_flag_quic_priority_update_structured_headers_parser, false) // If true, uses conservative cwnd gain and pacing gain when cwnd gets bootstrapped. QUIC_FLAG(quic_reloadable_flag_quic_conservative_cwnd_and_pacing_gains, false) // Instead of assuming an incoming connection ID length for short headers, ask each time.