Remove MAX_REQUEST_ID and REQUESTS_BLOCKED. PiperOrigin-RevId: 976062263
diff --git a/quiche/quic/moqt/moqt_error.h b/quiche/quic/moqt/moqt_error.h index 30b1a58..befb362 100644 --- a/quiche/quic/moqt/moqt_error.h +++ b/quiche/quic/moqt/moqt_error.h
@@ -26,7 +26,6 @@ kInvalidRequestId = 0x4, kDuplicateTrackAlias = 0x5, kKeyValueFormattingError = 0x6, - kTooManyRequests = 0x7, kInvalidPath = 0x8, kMalformedPath = 0x9, kGoawayTimeout = 0x10,
diff --git a/quiche/quic/moqt/moqt_framer.cc b/quiche/quic/moqt/moqt_framer.cc index fa3930a..7e76bcc 100644 --- a/quiche/quic/moqt/moqt_framer.cc +++ b/quiche/quic/moqt/moqt_framer.cc
@@ -279,10 +279,6 @@ KeyValuePairList SetupParameters::ToKeyValuePairList() const { KeyValuePairList out; - if (max_request_id.has_value()) { - out.insert(static_cast<uint64_t>(SetupParameter::kMaxRequestId), - *max_request_id); - } if (max_auth_token_cache_size.has_value()) { out.insert(static_cast<uint64_t>(SetupParameter::kMaxAuthTokenCacheSize), *max_auth_token_cache_size); @@ -609,12 +605,6 @@ WireKeyValuePairList(message.parameters.ToKeyValuePairList())); } -quiche::QuicheBuffer MoqtFramer::SerializeMaxRequestId( - const MoqtMaxRequestId& message) { - return SerializeControlMessage(MoqtMessageType::kMaxRequestId, - WireMoqVarInt(message.max_request_id)); -} - quiche::QuicheBuffer MoqtFramer::SerializeFetch(const MoqtFetch& message) { if (std::holds_alternative<StandaloneFetch>(message.fetch)) { const StandaloneFetch& standalone_fetch = @@ -676,12 +666,6 @@ WireMoqVarInt(message.request_id)); } -quiche::QuicheBuffer MoqtFramer::SerializeRequestsBlocked( - const MoqtRequestsBlocked& message) { - return SerializeControlMessage(MoqtMessageType::kRequestsBlocked, - WireMoqVarInt(message.max_request_id)); -} - quiche::QuicheBuffer MoqtFramer::SerializePublish(const MoqtPublish& message) { return SerializeControlMessage( MoqtMessageType::kPublish, WireMoqVarInt(message.request_id),
diff --git a/quiche/quic/moqt/moqt_framer.h b/quiche/quic/moqt/moqt_framer.h index c3beed3..c11479b 100644 --- a/quiche/quic/moqt/moqt_framer.h +++ b/quiche/quic/moqt/moqt_framer.h
@@ -66,12 +66,9 @@ const MoqtSubscribeNamespace& message); quiche::QuicheBuffer SerializeSubscribeTracks( const MoqtSubscribeTracks& message); - quiche::QuicheBuffer SerializeMaxRequestId(const MoqtMaxRequestId& message); quiche::QuicheBuffer SerializeFetch(const MoqtFetch& message); quiche::QuicheBuffer SerializeFetchCancel(const MoqtFetchCancel& message); quiche::QuicheBuffer SerializeFetchOk(const MoqtFetchOk& message); - quiche::QuicheBuffer SerializeRequestsBlocked( - const MoqtRequestsBlocked& message); quiche::QuicheBuffer SerializePublish(const MoqtPublish& message); quiche::QuicheBuffer SerializeObjectAck(const MoqtObjectAck& message);
diff --git a/quiche/quic/moqt/moqt_framer_test.cc b/quiche/quic/moqt/moqt_framer_test.cc index 90463dc..7082ef7 100644 --- a/quiche/quic/moqt/moqt_framer_test.cc +++ b/quiche/quic/moqt/moqt_framer_test.cc
@@ -54,11 +54,9 @@ MoqtMessageType::kGoAway, MoqtMessageType::kSubscribeNamespace, MoqtMessageType::kSubscribeTracks, - MoqtMessageType::kMaxRequestId, MoqtMessageType::kFetch, MoqtMessageType::kFetchCancel, MoqtMessageType::kFetchOk, - MoqtMessageType::kRequestsBlocked, MoqtMessageType::kPublish, MoqtMessageType::kObjectAck, MoqtMessageType::kSetup, @@ -182,10 +180,6 @@ auto data = std::get<MoqtSubscribeTracks>(structured_data); return framer_.SerializeSubscribeTracks(data); } - case moqt::MoqtMessageType::kMaxRequestId: { - auto data = std::get<MoqtMaxRequestId>(structured_data); - return framer_.SerializeMaxRequestId(data); - } case moqt::MoqtMessageType::kFetch: { auto data = std::get<MoqtFetch>(structured_data); return framer_.SerializeFetch(data); @@ -198,10 +192,6 @@ auto data = std::get<MoqtFetchOk>(structured_data); return framer_.SerializeFetchOk(data); } - case moqt::MoqtMessageType::kRequestsBlocked: { - auto data = std::get<MoqtRequestsBlocked>(structured_data); - return framer_.SerializeRequestsBlocked(data); - } case moqt::MoqtMessageType::kPublish: { auto data = std::get<MoqtPublish>(structured_data); return framer_.SerializePublish(data);
diff --git a/quiche/quic/moqt/moqt_key_value_pair.h b/quiche/quic/moqt/moqt_key_value_pair.h index c40b50f..90a9fdc 100644 --- a/quiche/quic/moqt/moqt_key_value_pair.h +++ b/quiche/quic/moqt/moqt_key_value_pair.h
@@ -143,13 +143,11 @@ }; // Setup parameters. -inline constexpr uint64_t kDefaultMaxRequestId = 0; // TODO(martinduke): Implement an auth token cache. inline constexpr uint64_t kDefaultMaxAuthTokenCacheSize = 0; inline constexpr bool kDefaultSupportObjectAcks = false; enum class QUICHE_EXPORT SetupParameter : uint64_t { kPath = 0x1, - kMaxRequestId = 0x2, kAuthorizationToken = 0x3, kMaxAuthTokenCacheSize = 0x4, kAuthority = 0x5, @@ -163,13 +161,10 @@ struct QUICHE_EXPORT SetupParameters { SetupParameters() = default; // Constructors for tests. - SetupParameters(absl::string_view path, absl::string_view authority, - uint64_t max_request_id) - : path(path), max_request_id(max_request_id), authority(authority) {} - SetupParameters(uint64_t max_request_id) : max_request_id(max_request_id) {} + SetupParameters(absl::string_view path, absl::string_view authority) + : path(path), authority(authority) {} std::optional<std::string> path; - std::optional<uint64_t> max_request_id; // TODO(martinduke): Turn authorization_token into structured data. std::vector<AuthToken> authorization_tokens; std::optional<uint64_t> max_auth_token_cache_size;
diff --git a/quiche/quic/moqt/moqt_messages.cc b/quiche/quic/moqt/moqt_messages.cc index ab5e89d..ee6e6ca 100644 --- a/quiche/quic/moqt/moqt_messages.cc +++ b/quiche/quic/moqt/moqt_messages.cc
@@ -99,8 +99,6 @@ return "SUBSCRIBE_NAMESPACE"; case MoqtMessageType::kSubscribeTracks: return "SUBSCRIBE_TRACKS"; - case MoqtMessageType::kMaxRequestId: - return "MAX_REQUEST_ID"; case MoqtMessageType::kPublish: return "PUBLISH"; case MoqtMessageType::kFetch: @@ -109,8 +107,6 @@ return "FETCH_CANCEL"; case MoqtMessageType::kFetchOk: return "FETCH_OK"; - case MoqtMessageType::kRequestsBlocked: - return "REQUESTS_BLOCKED"; case MoqtMessageType::kObjectAck: return "OBJECT_ACK"; }
diff --git a/quiche/quic/moqt/moqt_messages.h b/quiche/quic/moqt/moqt_messages.h index 2d35300..e3ff734 100644 --- a/quiche/quic/moqt/moqt_messages.h +++ b/quiche/quic/moqt/moqt_messages.h
@@ -210,11 +210,9 @@ kTrackStatus = 0x0d, kNamespaceDone = 0x0e, kGoAway = 0x10, - kMaxRequestId = 0x15, kFetch = 0x16, kFetchCancel = 0x17, kFetchOk = 0x18, - kRequestsBlocked = 0x1a, kPublish = 0x1d, kSubscribeNamespace = 0x50, kSubscribeTracks = 0x51, @@ -437,10 +435,6 @@ TrackNamespace track_namespace_suffix; }; -struct QUICHE_EXPORT MoqtMaxRequestId { - uint64_t max_request_id; -}; - enum class QUICHE_EXPORT FetchType : uint64_t { kStandalone = 0x1, kRelativeJoining = 0x2, @@ -503,10 +497,6 @@ uint64_t request_id; }; -struct QUICHE_EXPORT MoqtRequestsBlocked { - uint64_t max_request_id; -}; - struct QUICHE_EXPORT MoqtPublish { uint64_t request_id; FullTrackName full_track_name;
diff --git a/quiche/quic/moqt/moqt_parser.cc b/quiche/quic/moqt/moqt_parser.cc index ab6babe..bf0eadf 100644 --- a/quiche/quic/moqt/moqt_parser.cc +++ b/quiche/quic/moqt/moqt_parser.cc
@@ -275,13 +275,6 @@ [&](uint64_t key, std::variant<uint64_t, absl::string_view> value) { last_key = key; switch (static_cast<SetupParameter>(key)) { - case SetupParameter::kMaxRequestId: - if (max_request_id.has_value()) { - status = absl::InvalidArgumentError("Duplicate Setup Parameter"); - return false; - } - max_request_id = std::get<uint64_t>(value); - break; case SetupParameter::kMaxAuthTokenCacheSize: if (max_auth_token_cache_size.has_value()) { status = absl::InvalidArgumentError("Duplicate Setup Parameter"); @@ -838,17 +831,6 @@ return subscribe_tracks; } -absl::StatusOr<MoqtMaxRequestId> MoqtControlMessageParser::ProcessMaxRequestId( - absl::string_view data) const { - quic::QuicDataReader reader(data); - MoqtMaxRequestId max_request_id; - if (!reader.ReadMoqVarInt(&max_request_id.max_request_id)) { - return absl::InvalidArgumentError("Max request ID missing"); - } - QUICHE_RETURN_IF_ERROR(CheckForTrailingData(reader)); - return max_request_id; -} - absl::StatusOr<MoqtFetch> MoqtControlMessageParser::ProcessFetch( absl::string_view data) const { quic::QuicDataReader reader(data); @@ -956,17 +938,6 @@ return fetch_cancel; } -absl::StatusOr<MoqtRequestsBlocked> -MoqtControlMessageParser::ProcessRequestsBlocked(absl::string_view data) const { - quic::QuicDataReader reader(data); - MoqtRequestsBlocked requests_blocked; - if (!reader.ReadMoqVarInt(&requests_blocked.max_request_id)) { - return absl::InvalidArgumentError("Max request ID missing"); - } - QUICHE_RETURN_IF_ERROR(CheckForTrailingData(reader)); - return requests_blocked; -} - absl::StatusOr<MoqtPublish> MoqtControlMessageParser::ProcessPublish( absl::string_view data) const { quic::QuicDataReader reader(data);
diff --git a/quiche/quic/moqt/moqt_parser.h b/quiche/quic/moqt/moqt_parser.h index 0a2e46e..b36815c 100644 --- a/quiche/quic/moqt/moqt_parser.h +++ b/quiche/quic/moqt/moqt_parser.h
@@ -171,14 +171,10 @@ absl::string_view data) const; absl::StatusOr<MoqtSubscribeTracks> ProcessSubscribeTracks( absl::string_view data) const; - absl::StatusOr<MoqtMaxRequestId> ProcessMaxRequestId( - absl::string_view data) const; absl::StatusOr<MoqtFetch> ProcessFetch(absl::string_view data) const; absl::StatusOr<MoqtFetchCancel> ProcessFetchCancel( absl::string_view data) const; absl::StatusOr<MoqtFetchOk> ProcessFetchOk(absl::string_view data) const; - absl::StatusOr<MoqtRequestsBlocked> ProcessRequestsBlocked( - absl::string_view data) const; absl::StatusOr<MoqtPublish> ProcessPublish(absl::string_view data) const; absl::StatusOr<MoqtObjectAck> ProcessObjectAck(absl::string_view data) const; @@ -226,16 +222,12 @@ return parse(&MoqtControlMessageParser::ProcessSubscribeNamespace); case MoqtMessageType::kSubscribeTracks: return parse(&MoqtControlMessageParser::ProcessSubscribeTracks); - case MoqtMessageType::kMaxRequestId: - return parse(&MoqtControlMessageParser::ProcessMaxRequestId); case MoqtMessageType::kFetch: return parse(&MoqtControlMessageParser::ProcessFetch); case MoqtMessageType::kFetchCancel: return parse(&MoqtControlMessageParser::ProcessFetchCancel); case MoqtMessageType::kFetchOk: return parse(&MoqtControlMessageParser::ProcessFetchOk); - case MoqtMessageType::kRequestsBlocked: - return parse(&MoqtControlMessageParser::ProcessRequestsBlocked); case MoqtMessageType::kPublish: return parse(&MoqtControlMessageParser::ProcessPublish); case MoqtMessageType::kObjectAck:
diff --git a/quiche/quic/moqt/moqt_parser_test.cc b/quiche/quic/moqt/moqt_parser_test.cc index c393367..8b29cb6 100644 --- a/quiche/quic/moqt/moqt_parser_test.cc +++ b/quiche/quic/moqt/moqt_parser_test.cc
@@ -60,11 +60,9 @@ MoqtMessageType::kGoAway, MoqtMessageType::kSubscribeNamespace, MoqtMessageType::kSubscribeTracks, - MoqtMessageType::kMaxRequestId, MoqtMessageType::kFetch, MoqtMessageType::kFetchCancel, MoqtMessageType::kFetchOk, - MoqtMessageType::kRequestsBlocked, MoqtMessageType::kPublish, MoqtMessageType::kObjectAck, MoqtMessageType::kSetup, @@ -609,13 +607,13 @@ EXPECT_FALSE(data_visitor.parsing_error().has_value()); } -TEST_F(MoqtMessageSpecificTest, ClientSetupMaxRequestIdAppearsTwice) { +TEST_F(MoqtMessageSpecificTest, ClientSetupMaxAuthTokenCacheSizeAppearsTwice) { char setup[] = { 0xaf, 0x00, 0x00, 0x0a, 0x03, // 3 params 0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo" - 0x01, 0x32, // max_request_id = 50 - 0x00, 0x32, // max_request_id = 50 + 0x03, 0x32, // max_auth_token_cache_size = 50 + 0x00, 0x32, // max_auth_token_cache_size = 50 }; absl::StatusOr<std::vector<AnyMoqtControlMessage>> parsed = ParseAllMessages(absl::string_view(setup, sizeof(setup))); @@ -625,10 +623,9 @@ TEST_F(MoqtMessageSpecificTest, ServerSetupAuthorizationTokenTagRegister) { char setup[] = { - 0xaf, 0x00, 0x00, 0x0b, - 0x02, // 2 params - 0x02, 0x32, // max_request_id = 50 - 0x01, 0x06, 0x01, 0x10, 0x00, 0x62, 0x61, 0x72, // REGISTER 0x01 + 0xaf, 0x00, 0x00, 0x09, + 0x01, // 1 param + 0x03, 0x06, 0x01, 0x10, 0x00, 0x62, 0x61, 0x72, // REGISTER 0x01 }; absl::StatusOr<std::vector<AnyMoqtControlMessage>> parsed = ParseAllMessages(absl::string_view(setup, sizeof(setup)), @@ -714,19 +711,6 @@ MoqtError::kInvalidPath); } -TEST_F(MoqtMessageSpecificTest, ServerSetupMaxRequestIdAppearsTwice) { - char setup[] = { - 0xaf, 0x00, 0x00, 0x05, 0x02, // 2 params - 0x02, 0x32, // max_request_id = 50 - 0x00, 0x32, // max_request_id = 50 - }; - absl::StatusOr<std::vector<AnyMoqtControlMessage>> parsed = ParseAllMessages( - absl::string_view(setup, sizeof(setup)), kDefaultMoqtVersion, kRawQuic, - quic::Perspective::IS_CLIENT); - EXPECT_EQ(ExtractMoqtErrorForStatus(parsed.status()), - MoqtError::kProtocolViolation); -} - TEST_F(MoqtMessageSpecificTest, ClientSetupMalformedPath) { char setup[] = { 0xaf, 0x00, 0x00, 0x06,
diff --git a/quiche/quic/moqt/moqt_session.cc b/quiche/quic/moqt/moqt_session.cc index 00565a0..e812048 100644 --- a/quiche/quic/moqt/moqt_session.cc +++ b/quiche/quic/moqt/moqt_session.cc
@@ -92,7 +92,6 @@ callbacks_(std::move(callbacks)), framer_(parameters.using_webtrans, parameters.perspective), publisher_(DefaultPublisher::GetInstance()), - local_max_request_id_(parameters.max_request_id), alarm_factory_(std::move(alarm_factory)), weak_ptr_factory_(this), weak_ptr_factory_for_publishers_(this), @@ -280,20 +279,6 @@ << "Tried to send SUBSCRIBE_NAMESPACE after GOAWAY"; return nullptr; } - if (next_request_id_ >= peer_max_request_id_) { - if (!last_requests_blocked_sent_.has_value() || - peer_max_request_id_ > *last_requests_blocked_sent_) { - MoqtRequestsBlocked requests_blocked; - requests_blocked.max_request_id = peer_max_request_id_; - SendControlMessage(framer_.SerializeRequestsBlocked(requests_blocked)); - last_requests_blocked_sent_ = peer_max_request_id_; - } - QUIC_DLOG(INFO) << ENDPOINT << "Tried to send SUBSCRIBE_NAMESPACE with ID " - << next_request_id_ - << " which is greater than the maximum ID " - << peer_max_request_id_; - return nullptr; - } if (!outgoing_subscribe_namespace_.SubscribeNamespace(prefix)) { return nullptr; } @@ -471,20 +456,6 @@ SubscribeVisitor* absl_nonnull visitor, const MessageParameters& parameters) { QUICHE_DCHECK(name.IsValid()); - if (next_request_id_ >= peer_max_request_id_) { - if (!last_requests_blocked_sent_.has_value() || - peer_max_request_id_ > *last_requests_blocked_sent_) { - MoqtRequestsBlocked requests_blocked; - requests_blocked.max_request_id = peer_max_request_id_; - SendControlMessage(framer_.SerializeRequestsBlocked(requests_blocked)); - last_requests_blocked_sent_ = peer_max_request_id_; - } - QUIC_DLOG(INFO) << ENDPOINT << "Tried to send SUBSCRIBE with ID " - << next_request_id_ - << " which is greater than the maximum ID " - << peer_max_request_id_; - return false; - } if (subscribe_by_name_.contains(name)) { QUIC_DLOG(INFO) << ENDPOINT << "Tried to send SUBSCRIBE for track " << name << " which is already subscribed"; @@ -644,13 +615,6 @@ uint64_t end_group, std::optional<uint64_t> end_object, MessageParameters parameters) { QUICHE_DCHECK(name.IsValid()); - if (next_request_id_ >= peer_max_request_id_) { - QUIC_DLOG(INFO) << ENDPOINT << "Tried to send FETCH with ID " - << next_request_id_ - << " which is greater than the maximum ID " - << peer_max_request_id_; - return false; - } if (received_goaway_ || sent_goaway_) { QUIC_DLOG(INFO) << ENDPOINT << "Tried to send FETCH after GOAWAY"; return false; @@ -699,13 +663,6 @@ uint64_t num_previous_groups, MessageParameters parameters) { QUICHE_DCHECK(name.IsValid()); - if ((next_request_id_ + 2) >= peer_max_request_id_) { - QUIC_DLOG(INFO) << ENDPOINT << "Tried to send JOINING_FETCH with ID " - << (next_request_id_ + 2) - << " which is greater than the maximum ID " - << peer_max_request_id_; - return false; - } MessageParameters subscribe_parameters = parameters; subscribe_parameters.subscription_filter.emplace( MoqtFilterType::kLargestObject); @@ -869,19 +826,7 @@ } } -void MoqtSession::GrantMoreRequests(uint64_t num_requests) { - local_max_request_id_ += (num_requests * 2); - MoqtMaxRequestId message; - message.max_request_id = local_max_request_id_; - SendControlMessage(framer_.SerializeMaxRequestId(message)); -} - bool MoqtSession::ValidateRequestId(uint64_t request_id) { - if (request_id >= local_max_request_id_) { - QUIC_DLOG(INFO) << ENDPOINT << "Received request with too large ID"; - Error(MoqtError::kTooManyRequests, "Received request with too large ID"); - return false; - } if ((request_id % 2 == 0) != (parameters_.perspective == Perspective::IS_SERVER)) { QUICHE_DLOG(INFO) << ENDPOINT << "Request ID evenness incorrect"; @@ -1283,8 +1228,6 @@ peer_setup_received_ = true; peer_supports_object_ack_ = message.parameters.support_object_acks.value_or( kDefaultSupportObjectAcks); - peer_max_request_id_ = - message.parameters.max_request_id.value_or(kDefaultMaxRequestId); QUIC_DLOG(INFO) << ENDPOINT << "Received the SETUP message"; // TODO: handle path. if (callbacks_.session_established_callback != nullptr) { @@ -1369,18 +1312,6 @@ return absl::OkStatus(); } -absl::Status MoqtSession::OnControlMessage(const MoqtMaxRequestId& message) { - if (message.max_request_id < peer_max_request_id_) { - QUIC_DLOG(INFO) << ENDPOINT - << "Peer sent MAX_REQUEST_ID message with " - "lower value than previous"; - return absl::InvalidArgumentError( - "MAX_REQUEST_ID has lower value than previous"); - } - peer_max_request_id_ = message.max_request_id; - return absl::OkStatus(); -} - absl::Status MoqtSession::OnControlMessage(const MoqtFetch& message) { if (!ValidateRequestId(message.request_id)) { return absl::OkStatus(); @@ -1565,11 +1496,6 @@ return absl::OkStatus(); } -absl::Status MoqtSession::OnControlMessage(const MoqtRequestsBlocked& message) { - // TODO(martinduke): Derive logic for granting more subscribes. - return absl::OkStatus(); -} - void MoqtSession::OnMalformedTrack(ObjectSubscriber* track) { if (!track->is_fetch()) { auto* subscribe = absl::down_cast<LiveSubscriber*>(track); @@ -1654,9 +1580,6 @@ out.path = path; out.authority = authority; } - if (max_request_id != kDefaultMaxRequestId) { - out.max_request_id = max_request_id; - } if (max_auth_token_cache_size != kDefaultMaxAuthTokenCacheSize) { out.max_auth_token_cache_size = max_auth_token_cache_size; }
diff --git a/quiche/quic/moqt/moqt_session.h b/quiche/quic/moqt/moqt_session.h index 4593bff..bdf0e8d 100644 --- a/quiche/quic/moqt/moqt_session.h +++ b/quiche/quic/moqt/moqt_session.h
@@ -222,8 +222,6 @@ CleanUpState(); } - void GrantMoreRequests(uint64_t num_requests); - void UseAlternateDeliveryTimeout() { alternate_delivery_timeout_ = true; } private: @@ -437,13 +435,11 @@ absl::Status OnControlMessage(const MoqtRequestError& message); absl::Status OnControlMessage(const MoqtRequestUpdate& message); absl::Status OnControlMessage(const MoqtGoAway& /*message*/); - absl::Status OnControlMessage(const MoqtMaxRequestId& message); absl::Status OnControlMessage(const MoqtFetch& message); absl::Status OnControlMessage(const MoqtFetchCancel& /*message*/) { return absl::OkStatus(); } absl::Status OnControlMessage(const MoqtFetchOk& message); - absl::Status OnControlMessage(const MoqtRequestsBlocked& message); // TODO(vasilvv): remove this once all requests are moved into individual // streams. @@ -495,9 +491,6 @@ // The next subscribe ID that the local endpoint can send. uint64_t next_request_id_ = 0; - // The local endpoint can send subscribe IDs less than this value. - uint64_t peer_max_request_id_ = 0; - std::optional<uint64_t> last_requests_blocked_sent_; // All open incoming subscriptions, indexed by track name, used to check for // duplicates. @@ -533,10 +526,6 @@ SessionNamespaceTree incoming_subscribe_namespace_; SessionNamespaceTree outgoing_subscribe_namespace_; - // The maximum request ID sent to the peer. Peer-generated IDs must be less - // than this value. - uint64_t local_max_request_id_ = 0; - std::unique_ptr<quic::QuicAlarmFactory> alarm_factory_; // Kill the session if the peer doesn't promptly close out the session after // a GOAWAY.
diff --git a/quiche/quic/moqt/moqt_session_interface.h b/quiche/quic/moqt/moqt_session_interface.h index c2f41f5..1f8d52e 100644 --- a/quiche/quic/moqt/moqt_session_interface.h +++ b/quiche/quic/moqt/moqt_session_interface.h
@@ -35,7 +35,6 @@ inline constexpr absl::string_view kImplementationName = "Google QUICHE MOQT draft 16"; -inline constexpr uint64_t kDefaultInitialMaxRequestId = 100; struct QUICHE_EXPORT MoqtSessionParameters { // TODO: support multiple versions. MoqtSessionParameters() = default; @@ -49,15 +48,6 @@ using_webtrans(false), path(std::move(path)), authority(std::move(authority)) {} - MoqtSessionParameters(quic::Perspective perspective, std::string path, - std::string authority, uint64_t max_request_id) - : perspective(perspective), - using_webtrans(true), - path(std::move(path)), - max_request_id(max_request_id), - authority(std::move(authority)) {} - MoqtSessionParameters(quic::Perspective perspective, uint64_t max_request_id) - : perspective(perspective), max_request_id(max_request_id) {} bool operator==(const MoqtSessionParameters& other) const = default; std::string version = std::string(kDefaultMoqtVersion); @@ -65,7 +55,6 @@ quic::Perspective perspective = quic::Perspective::IS_SERVER; bool using_webtrans = true; std::string path; - uint64_t max_request_id = kDefaultInitialMaxRequestId; uint64_t max_auth_token_cache_size = kDefaultMaxAuthTokenCacheSize; bool support_object_acks = false; // TODO(martinduke): Turn authorization_token into structured data.
diff --git a/quiche/quic/moqt/moqt_session_test.cc b/quiche/quic/moqt/moqt_session_test.cc index 61a52ae..047a4e2 100644 --- a/quiche/quic/moqt/moqt_session_test.cc +++ b/quiche/quic/moqt/moqt_session_test.cc
@@ -141,8 +141,6 @@ std::make_unique<quic::test::TestAlarmFactory>(), session_callbacks_.AsSessionCallbacks()) { session_.set_publisher(&publisher_); - MoqtSessionPeer::set_peer_max_request_id(&session_, - kDefaultInitialMaxRequestId); MoqtSessionPeer::set_peer_setup_received(&session_, true); ON_CALL(mock_session_, GetStreamById) .WillByDefault(Return(&mock_bidi_stream_)); @@ -855,30 +853,6 @@ bidi_wrapper_2->ReceiveMessage(request); } -TEST_F(MoqtSessionTest, TooManySubscribes) { - MoqtSessionPeer::set_next_request_id(&session_, - kDefaultInitialMaxRequestId - 1); - PrepareRequestStream(bidi_wrapper_); - EXPECT_CALL(mock_bidi_stream_, - Writev(ControlMessageOfType(MoqtMessageType::kSubscribe), _)); - MessageParameters parameters(SubscribeForTest()); - parameters.subscription_filter.emplace(MoqtFilterType::kLargestObject); - EXPECT_TRUE(session_.Subscribe(FullTrackName("foo", "bar"), - &remote_track_visitor_, parameters)); - webtransport::test::MockStream control_stream; - std::unique_ptr<MoqtBidiStreamTestWrapper> control_wrapper = - MoqtSessionPeer::CreateControlStream(&session_, &control_stream); - EXPECT_CALL( - control_stream, - Writev(ControlMessageOfType(MoqtMessageType::kRequestsBlocked), _)) - .Times(1); - EXPECT_FALSE(session_.Subscribe(FullTrackName("foo2", "bar2"), - &remote_track_visitor_, parameters)); - // Second time does not send requests_blocked. - EXPECT_FALSE(session_.Subscribe(FullTrackName("foo2", "bar2"), - &remote_track_visitor_, parameters)); -} - TEST_F(MoqtSessionTest, SubscribeDuplicateTrackName) { PrepareRequestStream(bidi_wrapper_); EXPECT_CALL(mock_bidi_stream_, @@ -998,59 +972,6 @@ +[](std::variant<MessageParameters, MoqtRequestErrorInfo>) {})); } -TEST_F(MoqtSessionTest, MaxRequestIdChangesResponse) { - MoqtSessionPeer::set_next_request_id(&session_, kDefaultInitialMaxRequestId); - webtransport::test::MockStream control_stream; - std::unique_ptr<MoqtBidiStreamTestWrapper> control_wrapper = - MoqtSessionPeer::CreateControlStream(&session_, &control_stream); - EXPECT_CALL( - control_stream, - Writev(ControlMessageOfType(MoqtMessageType::kRequestsBlocked), _)); - MessageParameters parameters(SubscribeForTest()); - parameters.subscription_filter.emplace(MoqtFilterType::kLargestObject); - EXPECT_FALSE(session_.Subscribe(FullTrackName("foo", "bar"), - &remote_track_visitor_, parameters)); - MoqtMaxRequestId max_request_id = { - /*max_request_id=*/kDefaultInitialMaxRequestId + 1, - }; - control_wrapper->ReceiveMessage(max_request_id); - - PrepareRequestStream(bidi_wrapper_); - EXPECT_CALL(mock_bidi_stream_, - Writev(ControlMessageOfType(MoqtMessageType::kSubscribe), _)); - EXPECT_TRUE(session_.Subscribe(FullTrackName("foo", "bar"), - &remote_track_visitor_, parameters)); -} - -TEST_F(MoqtSessionTest, LowerMaxRequestIdIsAnError) { - MoqtMaxRequestId max_request_id = { - /*max_request_id=*/kDefaultInitialMaxRequestId - 1, - }; - bidi_wrapper_ = - MoqtSessionPeer::CreateControlStream(&session_, &mock_bidi_stream_); - EXPECT_CALL(mock_session_, - CloseSession(static_cast<uint64_t>(MoqtError::kProtocolViolation), - "MAX_REQUEST_ID has lower value than previous")) - .Times(1); - bidi_wrapper_->ReceiveMessage(max_request_id); -} - -TEST_F(MoqtSessionTest, GrantMoreRequests) { - webtransport::test::MockStream control_stream; - std::unique_ptr<MoqtBidiStreamTestWrapper> control_wrapper_ = - MoqtSessionPeer::CreateControlStream(&session_, &control_stream); - EXPECT_CALL(control_stream, - Writev(ControlMessageOfType(MoqtMessageType::kMaxRequestId), _)); - session_.GrantMoreRequests(1); - // Peer subscribes to (0, 0) - bidi_wrapper_ = std::make_unique<MoqtBidiStreamTestWrapper>( - ResponseStream(kSubscribeByte)); - MoqtSubscribe request = DefaultSubscribe(); - request.request_id = kDefaultInitialMaxRequestId + 1; - MockTrackPublisher* track = CreateTrackPublisher(); - ReceiveSubscribeSynchronousOk(track, request, bidi_wrapper_.get()); -} - TEST_F(MoqtSessionTest, SubscribeWithError) { PrepareRequestStream(bidi_wrapper_); EXPECT_CALL(mock_bidi_stream_,
diff --git a/quiche/quic/moqt/test_tools/moqt_framer_utils.cc b/quiche/quic/moqt/test_tools/moqt_framer_utils.cc index aa45615..f134c23 100644 --- a/quiche/quic/moqt/test_tools/moqt_framer_utils.cc +++ b/quiche/quic/moqt/test_tools/moqt_framer_utils.cc
@@ -64,9 +64,6 @@ quiche::QuicheBuffer operator()(const MoqtSubscribeTracks& message) { return framer.SerializeSubscribeTracks(message); } - quiche::QuicheBuffer operator()(const MoqtMaxRequestId& message) { - return framer.SerializeMaxRequestId(message); - } quiche::QuicheBuffer operator()(const MoqtFetch& message) { return framer.SerializeFetch(message); } @@ -76,9 +73,6 @@ quiche::QuicheBuffer operator()(const MoqtFetchOk& message) { return framer.SerializeFetchOk(message); } - quiche::QuicheBuffer operator()(const MoqtRequestsBlocked& message) { - return framer.SerializeRequestsBlocked(message); - } quiche::QuicheBuffer operator()(const MoqtPublish& message) { return framer.SerializePublish(message); }
diff --git a/quiche/quic/moqt/test_tools/moqt_framer_utils.h b/quiche/quic/moqt/test_tools/moqt_framer_utils.h index c0d9ee3..ac05e16 100644 --- a/quiche/quic/moqt/test_tools/moqt_framer_utils.h +++ b/quiche/quic/moqt/test_tools/moqt_framer_utils.h
@@ -24,9 +24,9 @@ std::variant<MoqtSetup, MoqtRequestOk, MoqtRequestError, MoqtSubscribe, MoqtSubscribeOk, MoqtPublishDone, MoqtRequestUpdate, MoqtPublishNamespace, MoqtTrackStatus, MoqtGoAway, - MoqtSubscribeNamespace, MoqtSubscribeTracks, MoqtMaxRequestId, - MoqtFetch, MoqtFetchCancel, MoqtFetchOk, MoqtRequestsBlocked, - MoqtPublish, MoqtNamespace, MoqtNamespaceDone, MoqtObjectAck>; + MoqtSubscribeNamespace, MoqtSubscribeTracks, MoqtFetch, + MoqtFetchCancel, MoqtFetchOk, MoqtPublish, MoqtNamespace, + MoqtNamespaceDone, MoqtObjectAck>; std::string SerializeGenericMessage(const AnyMoqtControlMessage& frame, bool use_webtrans = false);
diff --git a/quiche/quic/moqt/test_tools/moqt_session_peer.h b/quiche/quic/moqt/test_tools/moqt_session_peer.h index db84f18..024a6cb 100644 --- a/quiche/quic/moqt/test_tools/moqt_session_peer.h +++ b/quiche/quic/moqt/test_tools/moqt_session_peer.h
@@ -161,10 +161,6 @@ session->next_request_id_ = id; } - static void set_peer_max_request_id(MoqtSession* session, uint64_t id) { - session->peer_max_request_id_ = id; - } - static void set_peer_setup_received(MoqtSession* session, bool value) { session->peer_setup_received_ = value; }
diff --git a/quiche/quic/moqt/test_tools/moqt_test_message.h b/quiche/quic/moqt/test_tools/moqt_test_message.h index 732a817..45106fe 100644 --- a/quiche/quic/moqt/test_tools/moqt_test_message.h +++ b/quiche/quic/moqt/test_tools/moqt_test_message.h
@@ -117,9 +117,8 @@ MoqtSubscribe, MoqtSubscribeOk, MoqtPublishDone, MoqtRequestUpdate, MoqtPublishNamespace, MoqtTrackStatus, MoqtGoAway, MoqtSubscribeNamespace, MoqtSubscribeTracks, - MoqtMaxRequestId, MoqtFetch, MoqtFetchCancel, MoqtFetchOk, - MoqtRequestsBlocked, MoqtPublish, MoqtNamespace, - MoqtNamespaceDone, MoqtObjectAck>; + MoqtFetch, MoqtFetchCancel, MoqtFetchOk, MoqtPublish, + MoqtNamespace, MoqtNamespaceDone, MoqtObjectAck>; // The total actual size of the message. size_t total_message_size() const { return wire_image_size_; } @@ -609,14 +608,11 @@ client_setup_.parameters.path = std::nullopt; client_setup_.parameters.authority = std::nullopt; raw_packet_[3] -= 17; // adjust payload length - raw_packet_[4] = 0x02; // only two parameters - // Move MaxRequestId up in the packet. - memmove(raw_packet_ + 5, raw_packet_ + 11, 2); + raw_packet_[4] = 0x01; // only one parameter // Move MoqtImplementation up in the packet. - memmove(raw_packet_ + 7, raw_packet_ + 24, + memmove(raw_packet_ + 5, raw_packet_ + 22, kTestImplementationString.length() + 2); - raw_packet_[5] = 0x02; // Diff from 0. - raw_packet_[7] = 0x05; // Diff from 2. + raw_packet_[5] = 0x07; // Diff from 0. SetWireImage(raw_packet_, sizeof(raw_packet_) - 17); } else { SetWireImage(raw_packet_, sizeof(raw_packet_)); @@ -634,9 +630,9 @@ void ExpandVarints() override { if (client_setup_.parameters.path.has_value()) { - ExpandVarintsImpl("vvv----vvvv---------vv---------------------------"); + ExpandVarintsImpl("vvv----vv---------vv---------------------------"); } else { - ExpandVarintsImpl("vvvvv---------------------------"); + ExpandVarintsImpl("vvv---------------------------"); } } @@ -649,19 +645,18 @@ // string parameters in order. Unfortunately, this means that // kMoqtImplementation goes last even though it is always present, while // kPath and KAuthority aren't. - uint8_t raw_packet_[54] = { - 0xaf, 0x00, 0x00, 0x32, // type, length - 0x04, // 4 parameters + uint8_t raw_packet_[52] = { + 0xaf, 0x00, 0x00, 0x30, // type, length + 0x03, // 3 parameters 0x01, 0x04, 0x70, 0x61, 0x74, 0x68, // path = "path" - 0x01, 0x32, // max_request_id = 50 - 0x03, 0x09, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x74, + 0x04, 0x09, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x74, 0x79, // authority = "authority" // moqt_implementation: 0x02, 0x1c, 0x4d, 0x6f, 0x71, 0x20, 0x54, 0x65, 0x73, 0x74, 0x20, 0x49, 0x6d, 0x70, 0x6c, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x20, 0x54, 0x79, 0x70, 0x65}; MoqtSetup client_setup_ = { - SetupParameters("path", "authority", 50), + SetupParameters("path", "authority"), }; }; @@ -688,16 +683,15 @@ } private: - uint8_t raw_packet_[37] = {0xaf, 0x00, 0x00, 0x21, // type, length - 0x02, // two parameters - 0x02, 0x32, // max_subscribe_id = 50 + uint8_t raw_packet_[35] = {0xaf, 0x00, 0x00, 0x1f, // type, length + 0x01, // one parameter // moqt_implementation: - 0x05, 0x1c, 0x4d, 0x6f, 0x71, 0x20, 0x54, 0x65, + 0x07, 0x1c, 0x4d, 0x6f, 0x71, 0x20, 0x54, 0x65, 0x73, 0x74, 0x20, 0x49, 0x6d, 0x70, 0x6c, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x20, 0x54, 0x79, 0x70, 0x65}; MoqtSetup server_setup_ = { - SetupParameters(50), + SetupParameters(), }; }; @@ -1269,40 +1263,6 @@ }; }; -class QUICHE_NO_EXPORT MaxRequestIdMessage : public TestMessageBase { - public: - MaxRequestIdMessage() : TestMessageBase() { - SetWireImage(raw_packet_, sizeof(raw_packet_)); - } - - bool EqualFieldValues(const MessageStructuredData& values) const override { - auto cast = std::get<MoqtMaxRequestId>(values); - if (cast.max_request_id != max_request_id_.max_request_id) { - QUIC_LOG(INFO) << "MAX_REQUEST_ID mismatch"; - return false; - } - return true; - } - - void ExpandVarints() override { ExpandVarintsImpl("v"); } - - MessageStructuredData structured_data() const override { - return TestMessageBase::MessageStructuredData(max_request_id_); - } - - private: - uint8_t raw_packet_[4] = { - 0x15, - 0x00, - 0x01, - 0x0b, - }; - - MoqtMaxRequestId max_request_id_ = { - /*max_request_id =*/11, - }; -}; - class QUICHE_NO_EXPORT FetchMessage : public TestMessageBase { public: FetchMessage() : TestMessageBase() { @@ -1588,39 +1548,6 @@ }; }; -class QUICHE_NO_EXPORT RequestsBlockedMessage : public TestMessageBase { - public: - RequestsBlockedMessage() : TestMessageBase() { - SetWireImage(raw_packet_, sizeof(raw_packet_)); - } - bool EqualFieldValues(const MessageStructuredData& values) const override { - auto cast = std::get<MoqtRequestsBlocked>(values); - if (cast.max_request_id != requests_blocked_.max_request_id) { - QUIC_LOG(INFO) << "SUBSCRIBES_BLOCKED max_subscribe_id mismatch"; - return false; - } - return true; - } - - void ExpandVarints() override { ExpandVarintsImpl("v"); } - - MessageStructuredData structured_data() const override { - return TestMessageBase::MessageStructuredData(requests_blocked_); - } - - private: - uint8_t raw_packet_[4] = { - 0x1a, - 0x00, - 0x01, - 0x0b, // max_request_id = 11 - }; - - MoqtRequestsBlocked requests_blocked_ = { - /*max_request_id=*/11, - }; -}; - class QUICHE_NO_EXPORT PublishMessage : public TestMessageBase { public: PublishMessage() : TestMessageBase() { @@ -1762,16 +1689,12 @@ return std::make_unique<SubscribeNamespaceMessage>(); case MoqtMessageType::kSubscribeTracks: return std::make_unique<SubscribeTracksMessage>(); - case MoqtMessageType::kMaxRequestId: - return std::make_unique<MaxRequestIdMessage>(); case MoqtMessageType::kFetch: return std::make_unique<FetchMessage>(); case MoqtMessageType::kFetchCancel: return std::make_unique<FetchCancelMessage>(); case MoqtMessageType::kFetchOk: return std::make_unique<FetchOkMessage>(); - case MoqtMessageType::kRequestsBlocked: - return std::make_unique<RequestsBlockedMessage>(); case MoqtMessageType::kPublish: return std::make_unique<PublishMessage>(); case MoqtMessageType::kObjectAck:
diff --git a/quiche/quic/moqt/tools/moqt_end_to_end_test.cc b/quiche/quic/moqt/tools/moqt_end_to_end_test.cc index f415176..36cc538 100644 --- a/quiche/quic/moqt/tools/moqt_end_to_end_test.cc +++ b/quiche/quic/moqt/tools/moqt_end_to_end_test.cc
@@ -149,7 +149,7 @@ quic::QuicSocketAddress custom_server_address(host, custom_server.port()); MoqtSessionParameters client_parameters; - client_parameters.max_request_id = 200; + client_parameters.max_auth_token_cache_size = 200; MoqtClient client(custom_server_address, quic::QuicServerId("test.example.com", 443), quic::test::crypto_test_utils::ProofVerifierForTesting(), @@ -166,7 +166,8 @@ }); EXPECT_TRUE(success); ASSERT_NE(client.session(), nullptr); - EXPECT_EQ(MoqtSessionPeer::GetParameters(client.session()).max_request_id, + EXPECT_EQ(MoqtSessionPeer::GetParameters(client.session()) + .max_auth_token_cache_size, 200); ASSERT_NE(server_session, nullptr); EXPECT_TRUE(