Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build/deps/github_hashes/facebook/folly-rev.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Subproject commit ef07b7666e6a580a936987fc0f8727c1c51cc374
Subproject commit 836dffb4ee21609b98b6ab0ba2f0fae43f9c0097
2 changes: 1 addition & 1 deletion build/deps/github_hashes/facebook/mvfst-rev.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Subproject commit f98bd7649a1a604b2a8fa548e928ad046ec12ad4
Subproject commit 36f0ef484140540a63c710c9336672dce4e37153
2 changes: 1 addition & 1 deletion build/deps/github_hashes/facebook/proxygen-rev.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Subproject commit 7558747901d08465619ed67f6dc896cc9c2117d1
Subproject commit 61b53870bbb2194335ff81781243171733019d11
2 changes: 1 addition & 1 deletion build/deps/github_hashes/facebook/wangle-rev.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Subproject commit dc6de26b8d5b34aa87ae1159f9e0c9a64c78386d
Subproject commit 47656df19271f1323351f787a9f03e203ae4d9d2
2 changes: 1 addition & 1 deletion build/deps/github_hashes/facebookincubator/fizz-rev.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Subproject commit 11216ba32d86478caedfcb73c6330f69052fecfd
Subproject commit 2703f674e45ef4b8c647c1e2e0de08be1c2f07c0
52 changes: 50 additions & 2 deletions moxygen/MoQFramer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3458,6 +3458,34 @@ folly::Expected<RequestError, ErrorCode> 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);
Expand Down Expand Up @@ -5793,10 +5821,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_);
Expand Down Expand Up @@ -6099,6 +6128,18 @@ WriteResult MoQFrameWriter::writeRequestError(
<< "Invalid frameType passed to writeRequestError: "
<< static_cast<int>(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) {
Expand All @@ -6119,6 +6160,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);
Expand Down
45 changes: 42 additions & 3 deletions moxygen/MoQSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -3876,6 +3874,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<int>(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<int>(pendingType)
<< " sess=" << this;
close(SessionCloseErrorCode::PROTOCOL_VIOLATION);
return;
}
}
}

auto setErrorRes = pendingState->setError(error, frameType);
if (setErrorRes.hasError()) {
XLOG(ERR) << "setError failure id=" << error.requestID
Expand Down Expand Up @@ -6541,6 +6576,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.
Expand Down
4 changes: 4 additions & 0 deletions moxygen/MoQSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,10 @@ class MoQSession : public Subscriber,
return getFrameType(false);
}

Type type() const {
return type_;
}

virtual folly::Expected<Type, folly::Unit> setError(
RequestError error,
FrameType frameType);
Expand Down
7 changes: 7 additions & 0 deletions moxygen/MoQTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,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;
Expand All @@ -1415,6 +1420,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<std::chrono::milliseconds> retryInterval = std::nullopt;
// Draft 18+: present only when errorCode == REDIRECT.
std::optional<Redirect> redirect = std::nullopt;
};

// Type aliases for backward compatibility
Expand Down
4 changes: 2 additions & 2 deletions moxygen/moqtest/MoQTestClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ folly::coro::Task<moxygen::TrackNamespace> 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()) {
Expand Down Expand Up @@ -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;
}
Expand Down
153 changes: 153 additions & 0 deletions moxygen/test/MoQFramerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4778,6 +4778,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<std::string>{"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.
Expand Down
Loading