From 5e95323b3ec51aade753cbc040b4e5477d33cdeb Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Wed, 10 Jun 2026 03:06:26 -0700 Subject: [PATCH 1/4] No request_id in GOAWAY if on request stream Summary: I'm allowing the `requestId` to not be set in the `Goaway`. Currently, the only caller of `MoQFrameWriter::writeGoaway` is `MoQFrameWriter::writeGoaway`, so this is a no-op, but it will come into use later on when we implement the sending of Goaways on request streams. ___ Differential Revision: D108016055 fbshipit-source-id: 0b20d761c5619c053f372c98c4b54552ed42fe1f --- moxygen/MoQFramer.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/moxygen/MoQFramer.cpp b/moxygen/MoQFramer.cpp index d2a48c34..18587867 100644 --- a/moxygen/MoQFramer.cpp +++ b/moxygen/MoQFramer.cpp @@ -5698,10 +5698,11 @@ WriteResult MoQFrameWriter::writeGoaway( writeFixedString(writeBuf, goaway.newSessionUri, size, error); if (getDraftMajorVersion(*version_) >= 18) { writeVarint(writeBuf, goaway.timeout, size, error); + // Per draft 18, Request ID is present only when GOAWAY is sent on the + // control stream. Callers signal request-stream GOAWAY by leaving + // requestID unset. if (goaway.requestID.has_value()) { writeVarint(writeBuf, goaway.requestID->value, size, error); - } else { - error = true; } } writeSize(sizePtr, size, error, *version_); From f4adb1fef2328677b03d61d66d0baa0cec9157ff Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Wed, 10 Jun 2026 04:17:37 -0700 Subject: [PATCH 2/4] Implement writing and parsing of Redirect struct Summary: See title ___ Differential Revision: D108019594 fbshipit-source-id: c19df93bc90cae52d166e21102302d7ffa429cf8 --- moxygen/MoQFramer.cpp | 47 ++++++++++ moxygen/MoQSession.cpp | 45 +++++++++- moxygen/MoQSession.h | 4 + moxygen/MoQTypes.h | 7 ++ moxygen/test/MoQFramerTest.cpp | 153 +++++++++++++++++++++++++++++++++ 5 files changed, 253 insertions(+), 3 deletions(-) diff --git a/moxygen/MoQFramer.cpp b/moxygen/MoQFramer.cpp index 18587867..c4fab5ab 100644 --- a/moxygen/MoQFramer.cpp +++ b/moxygen/MoQFramer.cpp @@ -3387,6 +3387,34 @@ folly::Expected MoQFrameParser::parseRequestError( } requestError.reasonPhrase = std::move(reasonPhrase.value()); + if (requestError.errorCode == RequestErrorCode::REDIRECT) { + if (getDraftMajorVersion(*version_) < 18) { + XLOG(DBG4) << "REDIRECT errorCode received on pre-v18 draft"; + return folly::makeUnexpected(ErrorCode::PROTOCOL_VIOLATION); + } + } + + if (getDraftMajorVersion(*version_) >= 18 && + requestError.errorCode == RequestErrorCode::REDIRECT) { + Redirect redirect; + + auto connectUri = parseFixedString(cursor, length); + if (!connectUri) { + XLOG(DBG4) << "parseRequestError: UNDERFLOW on Redirect Connect URI"; + return folly::makeUnexpected(connectUri.error()); + } + redirect.connectUri = std::move(connectUri.value()); + + auto fullTrackName = parseFullTrackName(cursor, length); + if (!fullTrackName) { + XLOG(DBG4) << "parseRequestError: error parsing Redirect Full Track Name"; + return folly::makeUnexpected(fullTrackName.error()); + } + redirect.fullTrackName = std::move(fullTrackName.value()); + + requestError.redirect = std::move(redirect); + } + // Check for leftover bytes if (length > 0) { return folly::makeUnexpected(ErrorCode::PROTOCOL_VIOLATION); @@ -6005,6 +6033,18 @@ WriteResult MoQFrameWriter::writeRequestError( << "Invalid frameType passed to writeRequestError: " << static_cast(frameType); + if (requestError.errorCode == RequestErrorCode::REDIRECT) { + if (getDraftMajorVersion(*version_) < 18) { + XLOG(ERR) << "REDIRECT errorCode is only valid for draft 18+, version=" + << *version_; + return folly::makeUnexpected(quic::TransportErrorCode::INTERNAL_ERROR); + } + if (!requestError.redirect) { + XLOG(ERR) << "REDIRECT errorCode without a Redirect struct"; + return folly::makeUnexpected(quic::TransportErrorCode::INTERNAL_ERROR); + } + } + size_t size = 0; bool error = false; if (getDraftMajorVersion(*version_) > 14) { @@ -6025,6 +6065,13 @@ WriteResult MoQFrameWriter::writeRequestError( } writeFixedString(writeBuf, requestError.reasonPhrase, size, error); + if (getDraftMajorVersion(*version_) >= 18 && + requestError.errorCode == RequestErrorCode::REDIRECT) { + const Redirect& redirect = *requestError.redirect; + writeFixedString(writeBuf, redirect.connectUri, size, error); + writeFullTrackName(writeBuf, redirect.fullTrackName, size, error); + } + writeSize(sizePtr, size, error, *version_); if (error) { return folly::makeUnexpected(quic::TransportErrorCode::INTERNAL_ERROR); diff --git a/moxygen/MoQSession.cpp b/moxygen/MoQSession.cpp index 7964041f..0776382d 100644 --- a/moxygen/MoQSession.cpp +++ b/moxygen/MoQSession.cpp @@ -2154,9 +2154,7 @@ MoQSession::PendingRequestState::setError( return type_; } case FrameType::TRACK_STATUS: { - storage_.trackStatus_.setValue( - folly::makeUnexpected(TrackStatusError( - {error.requestID, error.errorCode, error.reasonPhrase}))); + storage_.trackStatus_.setValue(folly::makeUnexpected(std::move(error))); return type_; } case FrameType::FETCH_ERROR: { @@ -3874,6 +3872,43 @@ void MoQSession::onRequestError(RequestError error, FrameType frameType) { frameType = pendingState->getErrorFrameType(); } + if (error.errorCode == RequestErrorCode::REDIRECT) { + using PRStateType = PendingRequestState::Type; + const auto pendingType = pendingState->type(); + const bool isNamespaceScoped = + (pendingType == PRStateType::PUBLISH_NAMESPACE || + pendingType == PRStateType::SUBSCRIBE_NAMESPACE); + const bool isRedirectable = + (pendingType == PRStateType::SUBSCRIBE_TRACK || + pendingType == PRStateType::FETCH || + pendingType == PRStateType::TRACK_STATUS || isNamespaceScoped); + if (!isRedirectable) { + XLOG(ERR) << "REDIRECT not permitted for pending request type=" + << static_cast(pendingType) << " id=" << error.requestID + << " sess=" << this; + close(SessionCloseErrorCode::PROTOCOL_VIOLATION); + return; + } + if (error.redirect) { + if (dir_ == MoQControlCodec::Direction::SERVER && + !error.redirect->connectUri.empty()) { + XLOG(ERR) << "Server received REDIRECT with non-empty Connect URI" + << " id=" << error.requestID << " sess=" << this; + close(SessionCloseErrorCode::PROTOCOL_VIOLATION); + return; + } + if (isNamespaceScoped && + !error.redirect->fullTrackName.trackName.empty()) { + XLOG(ERR) << "Namespace-scoped REDIRECT has non-empty Track Name" + << " id=" << error.requestID + << " pendingType=" << static_cast(pendingType) + << " sess=" << this; + close(SessionCloseErrorCode::PROTOCOL_VIOLATION); + return; + } + } + } + auto setErrorRes = pendingState->setError(error, frameType); if (setErrorRes.hasError()) { XLOG(ERR) << "setError failure id=" << error.requestID @@ -6539,6 +6574,10 @@ WriteResult MessageReply::ok(const RequestOk& okMsg) { } WriteResult MessageReply::error(const SubscribeTracksError& errorMsg) { + if (errorMsg.errorCode == RequestErrorCode::REDIRECT) { + XLOG(ERR) << "REDIRECT not permitted in SUBSCRIBE_TRACKS error"; + return folly::makeUnexpected(quic::TransportErrorCode::INTERNAL_ERROR); + } auto res = moqFrameWriter_.writeRequestError( replyContext_->writeBuf(), errorMsg, FrameType::REQUEST_ERROR); // After ERROR the publisher is done with this stream — FIN it. diff --git a/moxygen/MoQSession.h b/moxygen/MoQSession.h index 3a4a4e4a..c09cedfc 100644 --- a/moxygen/MoQSession.h +++ b/moxygen/MoQSession.h @@ -969,6 +969,10 @@ class MoQSession : public Subscriber, return getFrameType(false); } + Type type() const { + return type_; + } + virtual folly::Expected setError( RequestError error, FrameType frameType); diff --git a/moxygen/MoQTypes.h b/moxygen/MoQTypes.h index 12a183b8..a438c6fb 100644 --- a/moxygen/MoQTypes.h +++ b/moxygen/MoQTypes.h @@ -1371,6 +1371,11 @@ using SubscribeTracksOk = RequestOk; using PublishNamespaceOk = RequestOk; using SubscribeUpdateOk = RequestOk; +struct Redirect { + std::string connectUri; + FullTrackName fullTrackName; +}; + // Consolidated request error structure struct RequestError { RequestID requestID; @@ -1381,6 +1386,8 @@ struct RequestError { // If the value is 0, the request SHOULD NOT be retried. // A value of 1 indicates the request can be retried immediately. std::optional retryInterval = std::nullopt; + // Draft 18+: present only when errorCode == REDIRECT. + std::optional redirect = std::nullopt; }; // Type aliases for backward compatibility diff --git a/moxygen/test/MoQFramerTest.cpp b/moxygen/test/MoQFramerTest.cpp index 2afcf386..d27f93d2 100644 --- a/moxygen/test/MoQFramerTest.cpp +++ b/moxygen/test/MoQFramerTest.cpp @@ -4624,6 +4624,159 @@ TEST_F(MoQFramerV18Test, SubscribeTracksForwardFalseSerializedAsParameter) { EXPECT_EQ(parsed->forward, false); } +// Draft 18+ REQUEST_ERROR carries a Redirect structure when errorCode == +// REDIRECT. Verify round-trip of a fully-populated Redirect. +TEST_F(MoQFramerV18Test, RequestErrorRedirectRoundtrip) { + RequestError requestError; + requestError.requestID = RequestID(7); + requestError.errorCode = RequestErrorCode::REDIRECT; + requestError.reasonPhrase = "moved"; + requestError.retryInterval = std::chrono::milliseconds{1}; + Redirect redirect; + redirect.connectUri = "https://relay.example.com/moq"; + redirect.fullTrackName.trackNamespace = + TrackNamespace(std::vector{"example.com", "live"}); + redirect.fullTrackName.trackName = "alt-track"; + requestError.redirect = redirect; + + folly::IOBufQueue writeBuf{folly::IOBufQueue::cacheChainLength()}; + ASSERT_TRUE( + writer_ + .writeRequestError(writeBuf, requestError, FrameType::REQUEST_ERROR) + .hasValue()); + + auto serialized = writeBuf.move(); + folly::io::Cursor cursor(serialized.get()); + auto frameType = decodeMoQVarint(cursor); + ASSERT_TRUE(frameType.has_value()); + EXPECT_EQ(frameType->first, folly::to_underlying(FrameType::REQUEST_ERROR)); + + auto bodyLen = frameLength(cursor); + auto parsed = + parser_.parseRequestError(cursor, bodyLen, FrameType::REQUEST_ERROR); + ASSERT_TRUE(parsed.hasValue()); + + EXPECT_EQ(parsed->requestID, requestError.requestID); + EXPECT_EQ(parsed->errorCode, RequestErrorCode::REDIRECT); + EXPECT_EQ(parsed->reasonPhrase, requestError.reasonPhrase); + EXPECT_EQ(parsed->retryInterval, requestError.retryInterval); + ASSERT_TRUE(parsed->redirect.has_value()); + EXPECT_EQ(parsed->redirect->connectUri, redirect.connectUri); + EXPECT_EQ( + parsed->redirect->fullTrackName.trackNamespace, + redirect.fullTrackName.trackNamespace); + EXPECT_EQ( + parsed->redirect->fullTrackName.trackName, + redirect.fullTrackName.trackName); +} + +// When errorCode == REDIRECT but no Redirect is supplied, the writer +// must refuse — silently substituting an empty Redirect would hide a +// caller bug and produce a misleading on-wire message. Callers that +// genuinely want "reuse current session URI / original Full Track Name" +// must set `redirect` to a default-constructed Redirect explicitly. +TEST_F(MoQFramerV18Test, RequestErrorRedirectWithoutRedirectIsRejected) { + RequestError requestError; + requestError.requestID = RequestID(3); + requestError.errorCode = RequestErrorCode::REDIRECT; + requestError.reasonPhrase = ""; + requestError.retryInterval = std::chrono::milliseconds{0}; + // Intentionally leave requestError.redirect unset. + + folly::IOBufQueue writeBuf{folly::IOBufQueue::cacheChainLength()}; + EXPECT_FALSE( + writer_ + .writeRequestError(writeBuf, requestError, FrameType::REQUEST_ERROR) + .hasValue()); +} + +// Per draft-ietf-moq-transport-18 §10.6.2, REDIRECT is only permitted in +// response to SUBSCRIBE, FETCH, TRACK_STATUS, PUBLISH_NAMESPACE and +// SUBSCRIBE_NAMESPACE. The framer does not know that original request +// context for v18+ REQUEST_ERROR; session-layer validation handles it. +TEST_F(MoQFramerV18Test, RequestErrorRedirectDoesNotValidateRequestType) { + RequestError requestError; + requestError.requestID = RequestID(1); + requestError.errorCode = RequestErrorCode::REDIRECT; + requestError.reasonPhrase = ""; + requestError.retryInterval = std::chrono::milliseconds{0}; + requestError.redirect = Redirect{}; + + folly::IOBufQueue writeBuf{folly::IOBufQueue::cacheChainLength()}; + + EXPECT_TRUE( + writer_ + .writeRequestError(writeBuf, requestError, FrameType::PUBLISH_ERROR) + .hasValue()); + EXPECT_TRUE(writer_ + .writeRequestError( + writeBuf, requestError, FrameType::SUBSCRIBE_UPDATE) + .hasValue()); +} + +// The parser only decodes REQUEST_ERROR bytes. It cannot decide whether a +// REDIRECT is valid for the original request; session-layer enforcement handles +// that using pending request state. +TEST_F(MoQFramerV18Test, ParseRequestErrorDoesNotValidateRequestType) { + // Hand-craft a REQUEST_ERROR body with errorCode == REDIRECT and a + // valid Redirect payload. + RequestError requestError; + requestError.requestID = RequestID(42); + requestError.errorCode = RequestErrorCode::REDIRECT; + requestError.reasonPhrase = "moved"; + requestError.retryInterval = std::chrono::milliseconds{0}; + requestError.redirect = Redirect{}; + + folly::IOBufQueue writeBuf{folly::IOBufQueue::cacheChainLength()}; + ASSERT_TRUE( + writer_.writeRequestError(writeBuf, requestError, FrameType::FETCH_ERROR) + .hasValue()); + + auto serialized = writeBuf.move(); + folly::io::Cursor cursor(serialized.get()); + ASSERT_TRUE(decodeMoQVarint(cursor).has_value()); // frame type + auto bodyLen = frameLength(cursor); + + // Parsing the same bytes as if they were a PUBLISH_ERROR still succeeds: + // request-type redirect validation requires session context. + auto parsed = + parser_.parseRequestError(cursor, bodyLen, FrameType::PUBLISH_ERROR); + ASSERT_TRUE(parsed.hasValue()); + EXPECT_EQ(parsed->errorCode, RequestErrorCode::REDIRECT); + EXPECT_TRUE(parsed->redirect.has_value()); +} + +// When errorCode is anything other than REDIRECT, no Redirect bytes are +// written, and the parsed RequestError has no redirect. +TEST_F(MoQFramerV18Test, RequestErrorNonRedirectOmitsRedirect) { + RequestError requestError; + requestError.requestID = RequestID(9); + requestError.errorCode = RequestErrorCode::INTERNAL_ERROR; + requestError.reasonPhrase = "boom"; + requestError.retryInterval = std::chrono::milliseconds{0}; + // Set redirect anyway to confirm the writer ignores it for non-REDIRECT. + Redirect redirect; + redirect.connectUri = "ignored"; + requestError.redirect = redirect; + + folly::IOBufQueue writeBuf{folly::IOBufQueue::cacheChainLength()}; + ASSERT_TRUE( + writer_ + .writeRequestError(writeBuf, requestError, FrameType::REQUEST_ERROR) + .hasValue()); + + auto serialized = writeBuf.move(); + folly::io::Cursor cursor(serialized.get()); + auto frameType = decodeMoQVarint(cursor); + ASSERT_TRUE(frameType.has_value()); + + auto bodyLen = frameLength(cursor); + auto parsed = + parser_.parseRequestError(cursor, bodyLen, FrameType::REQUEST_ERROR); + ASSERT_TRUE(parsed.hasValue()); + EXPECT_FALSE(parsed->redirect.has_value()); +} + // Draft 18 added a Track Properties block at the end of REQUEST_OK. Verify // that a populated TRACK_STATUS_OK round-trips its Track Properties through // the wire and back into the TrackStatusOk struct. From 4055e805f96d1d8d853f74e5551ae984ea0e1563 Mon Sep 17 00:00:00 2001 From: Open Source Bot Date: Wed, 10 Jun 2026 09:32:18 -0700 Subject: [PATCH 3/4] Updating hashes Summary: GitHub commits: https://github.com/facebook/CacheLib/commit/2ea12ae68ffe71127a623750df58a9b1531dc51a https://github.com/facebook/fb303/commit/5ef2ba4a082e6f8f67127958d7d93852394afd43 https://github.com/facebook/fbthrift/commit/e49bd2c8e3858ae0fd1f6f07571371c5f092590d https://github.com/facebook/folly/commit/836dffb4ee21609b98b6ab0ba2f0fae43f9c0097 https://github.com/facebook/hermes/commit/7b429f65120c745594ce3efe6b26abd8a09fccf2 https://github.com/facebook/mvfst/commit/36f0ef484140540a63c710c9336672dce4e37153 https://github.com/facebook/proxygen/commit/61b53870bbb2194335ff81781243171733019d11 https://github.com/facebook/pyrefly/commit/f3332753b5e8ff957ac7fe19c0a611df6f0c05b4 https://github.com/facebook/wangle/commit/47656df19271f1323351f787a9f03e203ae4d9d2 https://github.com/facebookexperimental/edencommon/commit/635f590bd840b76ffbef748998dfc3bc9ef9daf9 https://github.com/facebookexperimental/rust-shed/commit/266d6909a7697e32a4d5b9717b2d89426caf2a5a https://github.com/facebookincubator/fizz/commit/2703f674e45ef4b8c647c1e2e0de08be1c2f07c0 https://github.com/meta-quest/vros-api/commit/3d33dc50e1f2009250eb1092c3da636e8a00dbce https://github.com/react/yoga/commit/f6206ec3538bf1e344c3b8094d394bbeba8531f1 Reviewed By: bigfootjon fbshipit-source-id: 824a42fef12873a287cff7eb72b8933092694314 --- build/deps/github_hashes/facebook/folly-rev.txt | 2 +- build/deps/github_hashes/facebook/mvfst-rev.txt | 2 +- build/deps/github_hashes/facebook/proxygen-rev.txt | 2 +- build/deps/github_hashes/facebook/wangle-rev.txt | 2 +- build/deps/github_hashes/facebookincubator/fizz-rev.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/build/deps/github_hashes/facebook/folly-rev.txt b/build/deps/github_hashes/facebook/folly-rev.txt index ee26efe7..5889f9ec 100644 --- a/build/deps/github_hashes/facebook/folly-rev.txt +++ b/build/deps/github_hashes/facebook/folly-rev.txt @@ -1 +1 @@ -Subproject commit ef07b7666e6a580a936987fc0f8727c1c51cc374 +Subproject commit 836dffb4ee21609b98b6ab0ba2f0fae43f9c0097 diff --git a/build/deps/github_hashes/facebook/mvfst-rev.txt b/build/deps/github_hashes/facebook/mvfst-rev.txt index 51b60012..c3488c04 100644 --- a/build/deps/github_hashes/facebook/mvfst-rev.txt +++ b/build/deps/github_hashes/facebook/mvfst-rev.txt @@ -1 +1 @@ -Subproject commit f98bd7649a1a604b2a8fa548e928ad046ec12ad4 +Subproject commit 36f0ef484140540a63c710c9336672dce4e37153 diff --git a/build/deps/github_hashes/facebook/proxygen-rev.txt b/build/deps/github_hashes/facebook/proxygen-rev.txt index af7e2516..832aa506 100644 --- a/build/deps/github_hashes/facebook/proxygen-rev.txt +++ b/build/deps/github_hashes/facebook/proxygen-rev.txt @@ -1 +1 @@ -Subproject commit 7558747901d08465619ed67f6dc896cc9c2117d1 +Subproject commit 61b53870bbb2194335ff81781243171733019d11 diff --git a/build/deps/github_hashes/facebook/wangle-rev.txt b/build/deps/github_hashes/facebook/wangle-rev.txt index 943daf78..d3b5bf9e 100644 --- a/build/deps/github_hashes/facebook/wangle-rev.txt +++ b/build/deps/github_hashes/facebook/wangle-rev.txt @@ -1 +1 @@ -Subproject commit dc6de26b8d5b34aa87ae1159f9e0c9a64c78386d +Subproject commit 47656df19271f1323351f787a9f03e203ae4d9d2 diff --git a/build/deps/github_hashes/facebookincubator/fizz-rev.txt b/build/deps/github_hashes/facebookincubator/fizz-rev.txt index b8b37ad1..6681272c 100644 --- a/build/deps/github_hashes/facebookincubator/fizz-rev.txt +++ b/build/deps/github_hashes/facebookincubator/fizz-rev.txt @@ -1 +1 @@ -Subproject commit 11216ba32d86478caedfcb73c6330f69052fecfd +Subproject commit 2703f674e45ef4b8c647c1e2e0de08be1c2f07c0 From 0aec257cdc25ef0041003919b045d673be36cac5 Mon Sep 17 00:00:00 2001 From: generatedunixname1899117597419293 Date: Wed, 10 Jun 2026 13:53:08 -0700 Subject: [PATCH 4/4] xplat/ti/experimental/moxygen/moqtest/MoQTestClient.cpp Reviewed By: franklinho Differential Revision: D107860314 fbshipit-source-id: 43c4ff5777411671d5453580510ecbea567cf929 --- moxygen/moqtest/MoQTestClient.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/moxygen/moqtest/MoQTestClient.cpp b/moxygen/moqtest/MoQTestClient.cpp index 9365c8c4..2ad8f921 100644 --- a/moxygen/moqtest/MoQTestClient.cpp +++ b/moxygen/moqtest/MoQTestClient.cpp @@ -189,7 +189,7 @@ folly::coro::Task MoQTestClient::fetch( receivingType_ = ReceivingType::FETCH; initializeExpecteds(params); - // Fetch to the reciever + // Fetch to the receiver auto res = co_await moqClient_->moqSession_->fetch(fetch, fetchReceiver_); moqClient_->moqSession_->drain(); if (!res.hasError()) { @@ -230,7 +230,7 @@ ObjectReceiverCallback::FlowControlState MoQTestClient::onObject( return ObjectReceiverCallback::FlowControlState::UNBLOCKED; } - // Adjust the expected data (If Still recieving data, leave unblocked) + // Adjust the expected data (If Still receiving data, leave unblocked) adjustExpected(params_, &objHeader); return ObjectReceiverCallback::FlowControlState::UNBLOCKED; }