Skip to content

Commit 56a167f

Browse files
afrindfacebook-github-bot
authored andcommitted
Add Request ID to new messages (facebookexperimental#53)
Summary: There's a marginally gross hack here where I keep a single map of "FullTrackName" to RequestID for legacy verions. For TrackStatus its the real FTN, for ANNOUNCE and SUB_ANNOUNCES it's a fake FTN with eg (NS, "announce"). It could break if someone has a track name or namespace tuple called "announce" or "subannounce". Reviewed By: sharmafb Differential Revision: D74147358
1 parent acb80cb commit 56a167f

15 files changed

+365
-113
lines changed

moxygen/MoQFramer.cpp

Lines changed: 111 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -967,6 +967,17 @@ folly::Expected<Announce, ErrorCode> MoQFrameParser::parseAnnounce(
967967
folly::io::Cursor& cursor,
968968
size_t length) const noexcept {
969969
Announce announce;
970+
if (getDraftMajorVersion(*version_) >= 11) {
971+
auto requestID = quic::decodeQuicInteger(cursor, length);
972+
if (!requestID) {
973+
return folly::makeUnexpected(ErrorCode::PARSE_UNDERFLOW);
974+
}
975+
length -= requestID->second;
976+
announce.requestID = requestID->first;
977+
} else {
978+
announce.requestID = 0;
979+
}
980+
970981
auto res = parseFixedTuple(cursor, length);
971982
if (!res) {
972983
return folly::makeUnexpected(res.error());
@@ -992,11 +1003,21 @@ folly::Expected<AnnounceOk, ErrorCode> MoQFrameParser::parseAnnounceOk(
9921003
folly::io::Cursor& cursor,
9931004
size_t length) const noexcept {
9941005
AnnounceOk announceOk;
995-
auto res = parseFixedTuple(cursor, length);
996-
if (!res) {
997-
return folly::makeUnexpected(res.error());
1006+
if (getDraftMajorVersion(*version_) >= 11) {
1007+
auto requestID = quic::decodeQuicInteger(cursor, length);
1008+
if (!requestID) {
1009+
return folly::makeUnexpected(ErrorCode::PARSE_UNDERFLOW);
1010+
}
1011+
length -= requestID->second;
1012+
announceOk.requestID = requestID->first;
1013+
} else {
1014+
announceOk.requestID = 0;
1015+
auto res = parseFixedTuple(cursor, length);
1016+
if (!res) {
1017+
return folly::makeUnexpected(res.error());
1018+
}
1019+
announceOk.trackNamespace = TrackNamespace(std::move(res.value()));
9981020
}
999-
announceOk.trackNamespace = TrackNamespace(std::move(res.value()));
10001021
if (length > 0) {
10011022
return folly::makeUnexpected(ErrorCode::PROTOCOL_VIOLATION);
10021023
}
@@ -1007,11 +1028,21 @@ folly::Expected<AnnounceError, ErrorCode> MoQFrameParser::parseAnnounceError(
10071028
folly::io::Cursor& cursor,
10081029
size_t length) const noexcept {
10091030
AnnounceError announceError;
1010-
auto res = parseFixedTuple(cursor, length);
1011-
if (!res) {
1012-
return folly::makeUnexpected(res.error());
1031+
if (getDraftMajorVersion(*version_) >= 11) {
1032+
auto requestID = quic::decodeQuicInteger(cursor, length);
1033+
if (!requestID) {
1034+
return folly::makeUnexpected(ErrorCode::PARSE_UNDERFLOW);
1035+
}
1036+
length -= requestID->second;
1037+
announceError.requestID = requestID->first;
1038+
} else {
1039+
announceError.requestID = 0;
1040+
auto res = parseFixedTuple(cursor, length);
1041+
if (!res) {
1042+
return folly::makeUnexpected(res.error());
1043+
}
1044+
announceError.trackNamespace = TrackNamespace(std::move(res.value()));
10131045
}
1014-
announceError.trackNamespace = TrackNamespace(std::move(res.value()));
10151046

10161047
auto errorCode = quic::decodeQuicInteger(cursor, length);
10171048
if (!errorCode) {
@@ -1080,6 +1111,16 @@ MoQFrameParser::parseTrackStatusRequest(
10801111
<< "version_ needs to be set to parse TrackStatusRequest";
10811112

10821113
TrackStatusRequest trackStatusRequest;
1114+
if (getDraftMajorVersion(*version_) >= 11) {
1115+
auto requestID = quic::decodeQuicInteger(cursor, length);
1116+
if (!requestID) {
1117+
return folly::makeUnexpected(ErrorCode::PARSE_UNDERFLOW);
1118+
}
1119+
length -= requestID->second;
1120+
trackStatusRequest.requestID = requestID->first;
1121+
} else {
1122+
trackStatusRequest.requestID = 0;
1123+
}
10831124
auto res = parseFullTrackName(cursor, length);
10841125
if (!res) {
10851126
return folly::makeUnexpected(res.error());
@@ -1110,11 +1151,21 @@ folly::Expected<TrackStatus, ErrorCode> MoQFrameParser::parseTrackStatus(
11101151
CHECK(version_.hasValue()) << "version_ needs to be set to parse TrackStatus";
11111152

11121153
TrackStatus trackStatus;
1113-
auto res = parseFullTrackName(cursor, length);
1114-
if (!res) {
1115-
return folly::makeUnexpected(res.error());
1154+
if (getDraftMajorVersion(*version_) >= 11) {
1155+
auto requestID = quic::decodeQuicInteger(cursor, length);
1156+
if (!requestID) {
1157+
return folly::makeUnexpected(ErrorCode::PARSE_UNDERFLOW);
1158+
}
1159+
length -= requestID->second;
1160+
trackStatus.requestID = requestID->first;
1161+
} else {
1162+
trackStatus.requestID = 0;
1163+
auto res = parseFullTrackName(cursor, length);
1164+
if (!res) {
1165+
return folly::makeUnexpected(res.error());
1166+
}
1167+
trackStatus.fullTrackName = res.value();
11161168
}
1117-
trackStatus.fullTrackName = res.value();
11181169
auto statusCode = quic::decodeQuicInteger(cursor, length);
11191170
if (!statusCode) {
11201171
return folly::makeUnexpected(ErrorCode::PARSE_UNDERFLOW);
@@ -1390,7 +1441,7 @@ MoQFrameParser::parseSubscribeAnnounces(
13901441
return folly::makeUnexpected(res.error());
13911442
}
13921443
return SubscribeAnnounces(
1393-
{std::move(res->trackNamespace), std::move(res->params)});
1444+
{res->requestID, std::move(res->trackNamespace), std::move(res->params)});
13941445
}
13951446

13961447
folly::Expected<SubscribeAnnouncesOk, ErrorCode>
@@ -1401,7 +1452,7 @@ MoQFrameParser::parseSubscribeAnnouncesOk(
14011452
if (!res) {
14021453
return folly::makeUnexpected(res.error());
14031454
}
1404-
return SubscribeAnnouncesOk({std::move(res->trackNamespace)});
1455+
return SubscribeAnnouncesOk({res->requestID, std::move(res->trackNamespace)});
14051456
}
14061457

14071458
folly::Expected<SubscribeAnnouncesError, ErrorCode>
@@ -1413,7 +1464,8 @@ MoQFrameParser::parseSubscribeAnnouncesError(
14131464
return folly::makeUnexpected(res.error());
14141465
}
14151466
return SubscribeAnnouncesError(
1416-
{std::move(res->trackNamespace),
1467+
{res->requestID,
1468+
std::move(res->trackNamespace),
14171469
SubscribeAnnouncesErrorCode(folly::to_underlying(res->errorCode)),
14181470
std::move(res->reasonPhrase)});
14191471
}
@@ -1422,11 +1474,17 @@ folly::Expected<UnsubscribeAnnounces, ErrorCode>
14221474
MoQFrameParser::parseUnsubscribeAnnounces(
14231475
folly::io::Cursor& cursor,
14241476
size_t length) const noexcept {
1425-
auto res = parseAnnounceOk(cursor, length);
1477+
UnsubscribeAnnounces unsubscribeAnnounces;
1478+
auto res = parseFixedTuple(cursor, length);
14261479
if (!res) {
14271480
return folly::makeUnexpected(res.error());
14281481
}
1429-
return UnsubscribeAnnounces({std::move(res->trackNamespace)});
1482+
unsubscribeAnnounces.trackNamespacePrefix =
1483+
TrackNamespace(std::move(res.value()));
1484+
if (length > 0) {
1485+
return folly::makeUnexpected(ErrorCode::PROTOCOL_VIOLATION);
1486+
}
1487+
return unsubscribeAnnounces;
14301488
}
14311489

14321490
folly::Expected<FullTrackName, ErrorCode> MoQFrameParser::parseFullTrackName(
@@ -2304,6 +2362,9 @@ WriteResult MoQFrameWriter::writeAnnounce(
23042362
size_t size = 0;
23052363
bool error = false;
23062364
auto sizePtr = writeFrameHeader(writeBuf, FrameType::ANNOUNCE, error);
2365+
if (getDraftMajorVersion(*version_) >= 11) {
2366+
writeVarint(writeBuf, announce.requestID.value, size, error);
2367+
}
23072368
writeTrackNamespace(writeBuf, announce.trackNamespace, size, error);
23082369
writeTrackRequestParams(writeBuf, announce.params, size, error);
23092370
writeSize(sizePtr, size, error, *version_);
@@ -2320,7 +2381,11 @@ WriteResult MoQFrameWriter::writeAnnounceOk(
23202381
size_t size = 0;
23212382
bool error = false;
23222383
auto sizePtr = writeFrameHeader(writeBuf, FrameType::ANNOUNCE_OK, error);
2323-
writeTrackNamespace(writeBuf, announceOk.trackNamespace, size, error);
2384+
if (getDraftMajorVersion(*version_) >= 11) {
2385+
writeVarint(writeBuf, announceOk.requestID.value, size, error);
2386+
} else {
2387+
writeTrackNamespace(writeBuf, announceOk.trackNamespace, size, error);
2388+
}
23242389
writeSize(sizePtr, size, error, *version_);
23252390
if (error) {
23262391
return folly::makeUnexpected(quic::TransportErrorCode::INTERNAL_ERROR);
@@ -2336,7 +2401,11 @@ WriteResult MoQFrameWriter::writeAnnounceError(
23362401
size_t size = 0;
23372402
bool error = false;
23382403
auto sizePtr = writeFrameHeader(writeBuf, FrameType::ANNOUNCE_ERROR, error);
2339-
writeTrackNamespace(writeBuf, announceError.trackNamespace, size, error);
2404+
if (getDraftMajorVersion(*version_) >= 11) {
2405+
writeVarint(writeBuf, announceError.requestID.value, size, error);
2406+
} else {
2407+
writeTrackNamespace(writeBuf, announceError.trackNamespace, size, error);
2408+
}
23402409
writeVarint(
23412410
writeBuf, folly::to_underlying(announceError.errorCode), size, error);
23422411
writeFixedString(writeBuf, announceError.reasonPhrase, size, error);
@@ -2391,6 +2460,9 @@ WriteResult MoQFrameWriter::writeTrackStatusRequest(
23912460
bool error = false;
23922461
auto sizePtr =
23932462
writeFrameHeader(writeBuf, FrameType::TRACK_STATUS_REQUEST, error);
2463+
if (getDraftMajorVersion(*version_) >= 11) {
2464+
writeVarint(writeBuf, trackStatusRequest.requestID.value, size, error);
2465+
}
23942466
writeFullTrackName(writeBuf, trackStatusRequest.fullTrackName, size, error);
23952467
if (getDraftMajorVersion(*version_) >= 11) {
23962468
writeTrackRequestParams(writeBuf, trackStatusRequest.params, size, error);
@@ -2410,7 +2482,11 @@ WriteResult MoQFrameWriter::writeTrackStatus(
24102482
size_t size = 0;
24112483
bool error = false;
24122484
auto sizePtr = writeFrameHeader(writeBuf, FrameType::TRACK_STATUS, error);
2413-
writeFullTrackName(writeBuf, trackStatus.fullTrackName, size, error);
2485+
if (getDraftMajorVersion(*version_) >= 11) {
2486+
writeVarint(writeBuf, trackStatus.requestID.value, size, error);
2487+
} else {
2488+
writeFullTrackName(writeBuf, trackStatus.fullTrackName, size, error);
2489+
}
24142490
writeVarint(
24152491
writeBuf, folly::to_underlying(trackStatus.statusCode), size, error);
24162492
if (trackStatus.statusCode == TrackStatusCode::IN_PROGRESS) {
@@ -2455,6 +2531,9 @@ WriteResult MoQFrameWriter::writeSubscribeAnnounces(
24552531
bool error = false;
24562532
auto sizePtr =
24572533
writeFrameHeader(writeBuf, FrameType::SUBSCRIBE_ANNOUNCES, error);
2534+
if (getDraftMajorVersion(*version_) >= 11) {
2535+
writeVarint(writeBuf, subscribeAnnounces.requestID.value, size, error);
2536+
}
24582537
writeTrackNamespace(
24592538
writeBuf, subscribeAnnounces.trackNamespacePrefix, size, error);
24602539
writeTrackRequestParams(writeBuf, subscribeAnnounces.params, size, error);
@@ -2474,8 +2553,12 @@ WriteResult MoQFrameWriter::writeSubscribeAnnouncesOk(
24742553
bool error = false;
24752554
auto sizePtr =
24762555
writeFrameHeader(writeBuf, FrameType::SUBSCRIBE_ANNOUNCES_OK, error);
2477-
writeTrackNamespace(
2478-
writeBuf, subscribeAnnouncesOk.trackNamespacePrefix, size, error);
2556+
if (getDraftMajorVersion(*version_) >= 11) {
2557+
writeVarint(writeBuf, subscribeAnnouncesOk.requestID.value, size, error);
2558+
} else {
2559+
writeTrackNamespace(
2560+
writeBuf, subscribeAnnouncesOk.trackNamespacePrefix, size, error);
2561+
}
24792562
writeSize(sizePtr, size, error, *version_);
24802563
if (error) {
24812564
return folly::makeUnexpected(quic::TransportErrorCode::INTERNAL_ERROR);
@@ -2492,8 +2575,12 @@ WriteResult MoQFrameWriter::writeSubscribeAnnouncesError(
24922575
bool error = false;
24932576
auto sizePtr =
24942577
writeFrameHeader(writeBuf, FrameType::SUBSCRIBE_ANNOUNCES_ERROR, error);
2495-
writeTrackNamespace(
2496-
writeBuf, subscribeAnnouncesError.trackNamespacePrefix, size, error);
2578+
if (getDraftMajorVersion(*version_) >= 11) {
2579+
writeVarint(writeBuf, subscribeAnnouncesError.requestID.value, size, error);
2580+
} else {
2581+
writeTrackNamespace(
2582+
writeBuf, subscribeAnnouncesError.trackNamespacePrefix, size, error);
2583+
}
24972584
writeVarint(
24982585
writeBuf,
24992586
folly::to_underlying(subscribeAnnouncesError.errorCode),

moxygen/MoQFramer.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -672,15 +672,18 @@ struct SubscribeDone {
672672
};
673673

674674
struct Announce {
675+
RequestID requestID;
675676
TrackNamespace trackNamespace;
676677
std::vector<TrackRequestParameter> params;
677678
};
678679

679680
struct AnnounceOk {
681+
RequestID requestID;
680682
TrackNamespace trackNamespace;
681683
};
682684

683685
struct AnnounceError {
686+
RequestID requestID;
684687
TrackNamespace trackNamespace;
685688
AnnounceErrorCode errorCode;
686689
std::string reasonPhrase;
@@ -697,11 +700,13 @@ struct AnnounceCancel {
697700
};
698701

699702
struct TrackStatusRequest {
703+
RequestID requestID;
700704
FullTrackName fullTrackName;
701705
std::vector<TrackRequestParameter> params; // draft-11 and later
702706
};
703707

704708
struct TrackStatus {
709+
RequestID requestID;
705710
FullTrackName fullTrackName;
706711
TrackStatusCode statusCode;
707712
folly::Optional<AbsoluteLocation> latestGroupAndObject;
@@ -827,15 +832,18 @@ struct FetchError {
827832
};
828833

829834
struct SubscribeAnnounces {
835+
RequestID requestID;
830836
TrackNamespace trackNamespacePrefix;
831837
std::vector<TrackRequestParameter> params;
832838
};
833839

834840
struct SubscribeAnnouncesOk {
841+
RequestID requestID;
835842
TrackNamespace trackNamespacePrefix;
836843
};
837844

838845
struct SubscribeAnnouncesError {
846+
RequestID requestID;
839847
TrackNamespace trackNamespacePrefix;
840848
SubscribeAnnouncesErrorCode errorCode;
841849
std::string reasonPhrase;

0 commit comments

Comments
 (0)