Handle unknown Mandatory To Implement Track Properties. Cancel any requests that have these and propagate the error downstream. Part of draft-18 update (Section 2.5.1). Not in production. PiperOrigin-RevId: 986212226
diff --git a/quiche/quic/moqt/moqt_error.cc b/quiche/quic/moqt/moqt_error.cc index a413f88..040d4f4 100644 --- a/quiche/quic/moqt/moqt_error.cc +++ b/quiche/quic/moqt/moqt_error.cc
@@ -38,6 +38,8 @@ return RequestErrorCode::kInvalidJoiningRequestId; case absl::StatusCode::kUnauthenticated: return RequestErrorCode::kExpiredAuthToken; + case absl::StatusCode::kFailedPrecondition: + return RequestErrorCode::kUnsupportedExtension; default: return RequestErrorCode::kInternalError; } @@ -62,6 +64,8 @@ return absl::StatusCode::kInvalidArgument; case RequestErrorCode::kExpiredAuthToken: return absl::StatusCode::kUnauthenticated; + case RequestErrorCode::kUnsupportedExtension: + return absl::StatusCode::kFailedPrecondition; default: return absl::StatusCode::kUnknown; } @@ -94,6 +98,12 @@ } } +MoqtRequestErrorInfo StatusToMoqtRequestError(absl::Status status) { + return MoqtRequestErrorInfo(StatusToRequestErrorCode(status), + /*retry_interval=*/std::nullopt, + std::string(status.message())); +} + webtransport::StreamErrorCode StatusToMoqtStreamError(absl::Status status) { switch (status.code()) { case absl::StatusCode::kInternal:
diff --git a/quiche/quic/moqt/moqt_error.h b/quiche/quic/moqt/moqt_error.h index 636ec97..b1b2fe7 100644 --- a/quiche/quic/moqt/moqt_error.h +++ b/quiche/quic/moqt/moqt_error.h
@@ -75,6 +75,7 @@ kNamespacePrefixUnknown = 0x21, kPrefixOverlap = 0x30, kInvalidJoiningRequestId = 0x32, + kUnsupportedExtension = 0x33, }; enum class QUICHE_EXPORT PublishDoneCode : uint64_t { @@ -100,6 +101,7 @@ absl::StatusCode RequestErrorCodeToStatusCode(RequestErrorCode error_code); absl::Status RequestErrorCodeToStatus(RequestErrorCode error_code, absl::string_view reason_phrase); +MoqtRequestErrorInfo StatusToMoqtRequestError(absl::Status status); absl::Status MoqtStreamErrorToStatus(webtransport::StreamErrorCode error_code, absl::string_view reason_phrase);
diff --git a/quiche/quic/moqt/moqt_fetch_stream.cc b/quiche/quic/moqt/moqt_fetch_stream.cc index f80724c..7533094 100644 --- a/quiche/quic/moqt/moqt_fetch_stream.cc +++ b/quiche/quic/moqt/moqt_fetch_stream.cc
@@ -140,6 +140,16 @@ if (response_callback_ == nullptr) { return absl::InvalidArgumentError("Multiple FETCH_OK on the same stream"); } + absl::Status mandatory_property_status = + message.properties.CheckForUnknownMandatoryProperty(); + if (!mandatory_property_status.ok()) { + FetchResponseCallback response_callback = std::move(response_callback_); + response_callback_ = nullptr; + Reset(kResetCodeCancelled); + std::move(response_callback)( + StatusToMoqtRequestError(mandatory_property_status)); + return absl::OkStatus(); + } QUIC_DLOG(INFO) << "Received the FETCH_OK for " << full_track_name(); if (relative_groups_.has_value() && (*relative_groups_ < message.end_location.group)) {
diff --git a/quiche/quic/moqt/moqt_fetch_stream_test.cc b/quiche/quic/moqt/moqt_fetch_stream_test.cc index bd2e970..55d140a 100644 --- a/quiche/quic/moqt/moqt_fetch_stream_test.cc +++ b/quiche/quic/moqt/moqt_fetch_stream_test.cc
@@ -248,6 +248,25 @@ EXPECT_FALSE(stream->InWindow(Location(3, 51))); } +TEST_F(MoqtFetchRequestStreamTest, + OnControlMessageFetchOkWithUnknownMandatoryProperty) { + std::unique_ptr<MoqtFetchRequestStream> stream = + CreateAndBindStandaloneStream(); + EXPECT_CALL(mock_stream_, ResetWithUserCode(kResetCodeCancelled)); + EXPECT_CALL(delete_callback_, Call(kRequestId)); + EXPECT_CALL(task_, OnStreamAndFetchClosed); + EXPECT_CALL(response_callback_, + Call(testing::VariantWith<MoqtRequestErrorInfo>( + MoqtRequestErrorInfo{RequestErrorCode::kUnsupportedExtension, + /*retry_interval=*/std::nullopt, + "Unknown mandatory property: 0x4000"}))); + MoqtFetchOk ok_message; + ok_message.end_location = Location(3, 50); + ok_message.end_of_track = true; + ok_message.properties.insert(kMinMandatoryTrackProperty, 1ULL); + QUICHE_EXPECT_OK(stream->OnControlMessage(ok_message)); +} + TEST_F(MoqtFetchRequestStreamTest, OnControlMessageDuplicateFetchOk) { std::unique_ptr<MoqtFetchRequestStream> stream = CreateAndBindStandaloneStream();
diff --git a/quiche/quic/moqt/moqt_key_value_pair.h b/quiche/quic/moqt/moqt_key_value_pair.h index c6f9b01..09d8949 100644 --- a/quiche/quic/moqt/moqt_key_value_pair.h +++ b/quiche/quic/moqt/moqt_key_value_pair.h
@@ -14,6 +14,7 @@ #include "absl/container/btree_map.h" #include "absl/status/status.h" +#include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "quiche/quic/core/quic_time.h" #include "quiche/quic/moqt/moqt_priority.h" @@ -50,6 +51,18 @@ bool operator==(const KeyValuePairList& other) const = default; KeyValuePairList& operator=(const KeyValuePairList& other) = default; + protected: + // Returns the first key in the range [min_key, max_key], or nullopt if + // there is no such key. + std::optional<uint64_t> first_key_in_range(uint64_t min_key, + uint64_t max_key) const { + auto it = map_.lower_bound(min_key); + if (it == map_.end() || it->first > max_key) { + return std::nullopt; + } + return it->first; + } + private: absl::btree_multimap<uint64_t, std::variant<uint64_t, std::string>> map_; }; @@ -259,6 +272,8 @@ inline constexpr MoqtDeliveryOrder kDefaultGroupOrder = MoqtDeliveryOrder::kAscending; inline constexpr bool kDefaultDynamicGroups = false; +inline constexpr uint64_t kMinMandatoryTrackProperty = 0x4000; +inline constexpr uint64_t kMaxMandatoryTrackProperty = 0x7FFF; class TrackProperties : public KeyValuePairList { public: TrackProperties() = default; @@ -284,6 +299,16 @@ // Returns false if the property list contains illegal values or illegally // duplicated properties. bool Validate() const; + // Returns OK if there are no unknown mandatory properties. Otherwise, returns + // an error. + absl::Status CheckForUnknownMandatoryProperty() const { + std::optional<uint64_t> key = first_key_in_range( + kMinMandatoryTrackProperty, kMaxMandatoryTrackProperty); + return !key.has_value() + ? absl::OkStatus() + : absl::FailedPreconditionError(absl::StrCat( + "Unknown mandatory property: 0x", absl::Hex(*key))); + } bool operator==(const TrackProperties& other) const = default; TrackProperties& operator=(const TrackProperties& other) = default;
diff --git a/quiche/quic/moqt/moqt_key_value_pair_test.cc b/quiche/quic/moqt/moqt_key_value_pair_test.cc index 0d776a2..9e21fd8 100644 --- a/quiche/quic/moqt/moqt_key_value_pair_test.cc +++ b/quiche/quic/moqt/moqt_key_value_pair_test.cc
@@ -362,4 +362,46 @@ EXPECT_FALSE(properties.Validate()); } +TEST_F(TrackPropertiesTest, CheckForUnknownMandatoryProperty) { + TrackProperties properties; + QUICHE_EXPECT_OK(properties.CheckForUnknownMandatoryProperty()); + + // Non-mandatory property types (< 0x4000). + properties.insert(static_cast<uint64_t>(PropertyType::kDeliveryTimeout), + 5ULL); + properties.insert(0x3FFE, 1ULL); + properties.insert(0x3FFF, "odd_optional"); + QUICHE_EXPECT_OK(properties.CheckForUnknownMandatoryProperty()); + + // Non-mandatory property types (> 0x7FFF). + properties.insert(0x8000, 2ULL); + properties.insert(0x8001, "above_range"); + properties.insert(0x10000, 3ULL); + QUICHE_EXPECT_OK(properties.CheckForUnknownMandatoryProperty()); + + // Min boundary: 0x4000. + TrackProperties min_mandatory; + min_mandatory.insert(kMinMandatoryTrackProperty, 0ULL); + EXPECT_TRUE( + IsFailedPrecondition(min_mandatory.CheckForUnknownMandatoryProperty())); + EXPECT_EQ(min_mandatory.CheckForUnknownMandatoryProperty().message(), + "Unknown mandatory property: 0x4000"); + + // Max boundary: 0x7FFF. + TrackProperties max_mandatory; + max_mandatory.insert(kMaxMandatoryTrackProperty, "mandatory_string"); + EXPECT_TRUE( + IsFailedPrecondition(max_mandatory.CheckForUnknownMandatoryProperty())); + EXPECT_EQ(max_mandatory.CheckForUnknownMandatoryProperty().message(), + "Unknown mandatory property: 0x7fff"); + + // In between. + TrackProperties mid_mandatory; + mid_mandatory.insert(0x5000, 42ULL); + EXPECT_TRUE( + IsFailedPrecondition(mid_mandatory.CheckForUnknownMandatoryProperty())); + EXPECT_EQ(mid_mandatory.CheckForUnknownMandatoryProperty().message(), + "Unknown mandatory property: 0x5000"); +} + } // namespace moqt::test
diff --git a/quiche/quic/moqt/moqt_publish_stream.cc b/quiche/quic/moqt/moqt_publish_stream.cc index 38bca29..17ca8a9 100644 --- a/quiche/quic/moqt/moqt_publish_stream.cc +++ b/quiche/quic/moqt/moqt_publish_stream.cc
@@ -126,6 +126,14 @@ // Two PUBLISH messages for the same stream. return absl::InvalidArgumentError("Multiple PUBLISH on the same stream"); } + absl::Status mandatory_property_status = + message.properties.CheckForUnknownMandatoryProperty(); + if (!mandatory_property_status.ok()) { + add_callback_ = nullptr; + remove_callback_ = nullptr; + return SendRequestError( + StatusToMoqtRequestError(mandatory_property_status)); + } subscriber_ = std::make_unique<LiveSubscriber>(message, nullptr, this); if (!std::move(add_callback_)(subscriber_.get())) { add_callback_ = nullptr;
diff --git a/quiche/quic/moqt/moqt_publish_stream_test.cc b/quiche/quic/moqt/moqt_publish_stream_test.cc index aa0c0e6..3a62d4d 100644 --- a/quiche/quic/moqt/moqt_publish_stream_test.cc +++ b/quiche/quic/moqt/moqt_publish_stream_test.cc
@@ -365,6 +365,20 @@ EXPECT_CALL(mock_subscribe_visitor_, OnPublishDone); } +TEST_F(MoqtPublishResponseStreamTest, + ReceivePublishWithUnknownMandatoryProperty) { + MoqtPublish publish = DefaultPublish(); + publish.properties.insert(kMinMandatoryTrackProperty, 1ULL); + EXPECT_CALL(incoming_publish_callback_mock_, Call).Times(0); + EXPECT_CALL(mock_add_callback_, Call).Times(0); + MoqtRequestError expected_error{RequestErrorCode::kUnsupportedExtension, + /*retry_interval=*/std::nullopt, + "Unknown mandatory property: 0x4000"}; + EXPECT_CALL(mock_stream_, Writev(SerializedControlMessage(expected_error), _)) + .WillOnce(Return(absl::OkStatus())); + QUICHE_EXPECT_OK(stream_->OnControlMessage(publish)); +} + TEST_F(MoqtPublishResponseStreamTest, ReceivePublishAndReject) { MoqtPublish publish = DefaultPublish(); // Callback returns nullptr (rejection).
diff --git a/quiche/quic/moqt/moqt_subscribe_stream.cc b/quiche/quic/moqt/moqt_subscribe_stream.cc index 6793bd1..ea322b7 100644 --- a/quiche/quic/moqt/moqt_subscribe_stream.cc +++ b/quiche/quic/moqt/moqt_subscribe_stream.cc
@@ -66,6 +66,28 @@ return absl::InvalidArgumentError( "Multiple SUBSCRIBE_OK on the same stream"); } + absl::Status mandatory_property_status = + message.properties.CheckForUnknownMandatoryProperty(); + if (!mandatory_property_status.ok()) { + add_callback_ = nullptr; + // Save everything before Reset() destroys track_. + bool error_allowed = track_->ErrorIsAllowed(); + SubscribeVisitor* visitor = track_->visitor(); + FullTrackName track_name = track_->full_track_name(); + Reset(kResetCodeCancelled); + if (!error_allowed) { + QUICHE_BUG(moqt_bug_object_before_subscribe_ok) + << "An object was delivered before SUBSCRIBE_OK provided the track " + "alias"; + return absl::OkStatus(); + } + if (visitor != nullptr) { + // It's too late to deliver REQUEST_ERROR if an object already arrived + visitor->OnReply(track_name, + StatusToMoqtRequestError(mandatory_property_status)); + } + return absl::OkStatus(); + } track_->set_track_alias(message.track_alias); if (!std::move(add_callback_)(track_.get())) { add_callback_ = nullptr;
diff --git a/quiche/quic/moqt/moqt_subscribe_stream_test.cc b/quiche/quic/moqt/moqt_subscribe_stream_test.cc index 1e10f1f..df00582 100644 --- a/quiche/quic/moqt/moqt_subscribe_stream_test.cc +++ b/quiche/quic/moqt/moqt_subscribe_stream_test.cc
@@ -99,6 +99,27 @@ EXPECT_CALL(mock_remove_callback_, Call); } +TEST_F(MoqtSubscribeRequestStreamTest, + ReceiveSubscribeOkWithUnknownMandatoryProperty) { + EXPECT_CALL(mock_stream_, + Writev(ControlMessageOfType(MoqtMessageType::kSubscribe), _)) + .WillOnce(Return(absl::OkStatus())); + stream_->BindStream(&mock_stream_); + EXPECT_CALL(mock_add_callback_, Call).Times(0); + EXPECT_CALL(mock_stream_, ResetWithUserCode(kResetCodeCancelled)); + EXPECT_CALL(mock_remove_callback_, Call); + EXPECT_CALL( + mock_subscribe_visitor_, + OnReply(track_name_, + testing::VariantWith<MoqtRequestErrorInfo>( + MoqtRequestErrorInfo{RequestErrorCode::kUnsupportedExtension, + /*retry_interval=*/std::nullopt, + "Unknown mandatory property: 0x4000"}))); + MoqtSubscribeOk subscribe_ok(kTrackAlias); + subscribe_ok.properties.insert(kMinMandatoryTrackProperty, 1ULL); + QUICHE_EXPECT_OK(stream_->OnControlMessage(subscribe_ok)); +} + TEST_F(MoqtSubscribeRequestStreamTest, ReceiveSubscribeOkAliasDuplicate) { EXPECT_CALL(mock_stream_, Writev(ControlMessageOfType(MoqtMessageType::kSubscribe), _))