Use "kInvalidPath" and "kKeyValueFormattingError" when processing MoQT Setup parameters. PiperOrigin-RevId: 754872808
diff --git a/quiche/quic/moqt/moqt_framer.cc b/quiche/quic/moqt/moqt_framer.cc index c1f84bd..b6cf1b3 100644 --- a/quiche/quic/moqt/moqt_framer.cc +++ b/quiche/quic/moqt/moqt_framer.cc
@@ -384,8 +384,9 @@ const MoqtClientSetup& message) { KeyValuePairList parameters; SessionParametersToKeyValuePairList(message.parameters, parameters); - if (!ValidateSetupParameters(parameters, using_webtrans_, - quic::Perspective::IS_SERVER)) { + if (ValidateSetupParameters(parameters, using_webtrans_, + quic::Perspective::IS_SERVER) != + MoqtError::kNoError) { QUICHE_BUG(QUICHE_BUG_invalid_parameters) << "Serializing invalid MoQT parameters"; return quiche::QuicheBuffer(); @@ -401,8 +402,9 @@ const MoqtServerSetup& message) { KeyValuePairList parameters; SessionParametersToKeyValuePairList(message.parameters, parameters); - if (!ValidateSetupParameters(parameters, using_webtrans_, - quic::Perspective::IS_CLIENT)) { + if (ValidateSetupParameters(parameters, using_webtrans_, + quic::Perspective::IS_CLIENT) != + MoqtError::kNoError) { QUICHE_BUG(QUICHE_BUG_invalid_parameters) << "Serializing invalid MoQT parameters"; return quiche::QuicheBuffer();
diff --git a/quiche/quic/moqt/moqt_messages.cc b/quiche/quic/moqt/moqt_messages.cc index cc57440..967a816 100644 --- a/quiche/quic/moqt/moqt_messages.cc +++ b/quiche/quic/moqt/moqt_messages.cc
@@ -127,29 +127,30 @@ return MoqtFilterType::kLatestObject; } -bool ValidateSetupParameters(const KeyValuePairList& parameters, bool webtrans, - quic::Perspective perspective) { +MoqtError ValidateSetupParameters(const KeyValuePairList& parameters, + bool webtrans, + quic::Perspective perspective) { if (parameters.count(SetupParameter::kPath) > 1 || parameters.count(SetupParameter::kMaxRequestId) > 1 || parameters.count(SetupParameter::kMaxAuthTokenCacheSize) > 1 || parameters.count(SetupParameter::kSupportObjectAcks) > 1) { - return false; + return MoqtError::kKeyValueFormattingError; } if ((webtrans || perspective == quic::Perspective::IS_CLIENT) == parameters.contains(SetupParameter::kPath)) { // Only non-webtrans servers should receive kPath. - return false; + return MoqtError::kInvalidPath; } if (!parameters.contains(SetupParameter::kSupportObjectAcks)) { - return true; + return MoqtError::kNoError; } std::vector<uint64_t> support_object_acks = parameters.GetIntegers(SetupParameter::kSupportObjectAcks); QUICHE_DCHECK(support_object_acks.size() == 1); if (support_object_acks.front() > 1) { - return false; + return MoqtError::kKeyValueFormattingError; } - return true; + return MoqtError::kNoError; } const std::array<MoqtMessageType, 5> kAllowsAuthorization = {
diff --git a/quiche/quic/moqt/moqt_messages.h b/quiche/quic/moqt/moqt_messages.h index 9c01103..51b5861 100644 --- a/quiche/quic/moqt/moqt_messages.h +++ b/quiche/quic/moqt/moqt_messages.h
@@ -737,11 +737,10 @@ quic::QuicTimeDelta delta_from_deadline = quic::QuicTimeDelta::Zero(); }; -// Returns false if duplicates are present for a known parameter where the spec -// forbids duplicates. |perspective| is the consumer of the message, not the -// sender. -bool ValidateSetupParameters(const KeyValuePairList& parameters, bool webtrans, - quic::Perspective perspective); +// Returns an error if the parameters are malformed or otherwise violate the +// spec. |perspective| is the consumer of the message, not the sender. +MoqtError ValidateSetupParameters(const KeyValuePairList& parameters, + bool webtrans, quic::Perspective perspective); // Returns false if the parameters contain a protocol violation, or a // parameter cannot be in |message type|. Does not validate the internal // structure of Authorization Token values.
diff --git a/quiche/quic/moqt/moqt_parser.cc b/quiche/quic/moqt/moqt_parser.cc index 27d9c81..8b3ba02 100644 --- a/quiche/quic/moqt/moqt_parser.cc +++ b/quiche/quic/moqt/moqt_parser.cc
@@ -356,9 +356,10 @@ if (!ParseKeyValuePairList(reader, parameters)) { return 0; } - if (!ValidateSetupParameters(parameters, uses_web_transport_, - quic::Perspective::IS_SERVER)) { - ParseError("Client SETUP contains invalid parameters"); + MoqtError error = ValidateSetupParameters(parameters, uses_web_transport_, + quic::Perspective::IS_SERVER); + if (error != MoqtError::kNoError) { + ParseError(error, "Client SETUP contains invalid parameters"); return 0; } KeyValuePairListToMoqtSessionParameters(parameters, setup.parameters); @@ -380,9 +381,10 @@ if (!ParseKeyValuePairList(reader, parameters)) { return 0; } - if (!ValidateSetupParameters(parameters, uses_web_transport_, - quic::Perspective::IS_CLIENT)) { - ParseError("Server SETUP contains invalid parameters"); + MoqtError error = ValidateSetupParameters(parameters, uses_web_transport_, + quic::Perspective::IS_CLIENT); + if (error != MoqtError::kNoError) { + ParseError(error, "Server SETUP contains invalid parameters"); return 0; } KeyValuePairListToMoqtSessionParameters(parameters, setup.parameters);
diff --git a/quiche/quic/moqt/moqt_parser_test.cc b/quiche/quic/moqt/moqt_parser_test.cc index bfe3c99..84f4f60 100644 --- a/quiche/quic/moqt/moqt_parser_test.cc +++ b/quiche/quic/moqt/moqt_parser_test.cc
@@ -574,7 +574,7 @@ EXPECT_EQ(visitor_.messages_received_, 0); EXPECT_EQ(visitor_.parsing_error_, "Client SETUP contains invalid parameters"); - EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kProtocolViolation); + EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kKeyValueFormattingError); } TEST_F(MoqtMessageSpecificTest, SetupPathFromServer) { @@ -591,7 +591,7 @@ EXPECT_EQ(visitor_.messages_received_, 0); EXPECT_EQ(visitor_.parsing_error_, "Server SETUP contains invalid parameters"); - EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kProtocolViolation); + EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kInvalidPath); } TEST_F(MoqtMessageSpecificTest, SetupPathAppearsTwice) { @@ -608,7 +608,7 @@ EXPECT_EQ(visitor_.messages_received_, 0); EXPECT_EQ(visitor_.parsing_error_, "Client SETUP contains invalid parameters"); - EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kProtocolViolation); + EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kKeyValueFormattingError); } TEST_F(MoqtMessageSpecificTest, SetupPathOverWebtrans) { @@ -624,7 +624,7 @@ EXPECT_EQ(visitor_.messages_received_, 0); EXPECT_EQ(visitor_.parsing_error_, "Client SETUP contains invalid parameters"); - EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kProtocolViolation); + EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kInvalidPath); } TEST_F(MoqtMessageSpecificTest, SetupPathMissing) { @@ -639,7 +639,7 @@ EXPECT_EQ(visitor_.messages_received_, 0); EXPECT_EQ(visitor_.parsing_error_, "Client SETUP contains invalid parameters"); - EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kProtocolViolation); + EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kInvalidPath); } TEST_F(MoqtMessageSpecificTest, ServerSetupMaxSubscribeIdAppearsTwice) { @@ -657,7 +657,7 @@ EXPECT_EQ(visitor_.messages_received_, 0); EXPECT_EQ(visitor_.parsing_error_, "Client SETUP contains invalid parameters"); - EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kProtocolViolation); + EXPECT_EQ(visitor_.parsing_error_code_, MoqtError::kKeyValueFormattingError); } TEST_F(MoqtMessageSpecificTest, UnknownParameterTwiceIsOk) {