Unify ClientSetup/ServerSetup into Setup

Summary:
MOQT draft-15+ negotiates version via ALPN rather than in the setup
messages themselves, making the `supportedVersions` vector in
ClientSetup and `selectedVersion` in ServerSetup redundant for modern
drafts. Draft-14 (legacy/moq-00) is the only pre-ALPN version we
support.

This change unifies `ClientSetup` and `ServerSetup` into a single
`Setup` struct (aliases kept for compat), removing the version fields
from the structs and moving version handling into the framer/codec layer.

Key changes:
- Add `Setup` struct; alias `ClientSetup`/`ServerSetup` to it
- Framer: `writeClientSetup`/`writeServerSetup` take an explicit version
  param; write version on wire only in legacy mode (draft < 15), with
  XCHECK_EQ that version is draft-14
- Framer: `parseClientSetup` in legacy mode requires draft-14 in the
  client's version array (VERSION_NEGOTIATION_FAILED if absent);
  `parseServerSetup` in legacy mode validates server selected draft-14
- Session: `setup(Setup)` replaces `setup(ClientSetup)`; version
  initialized from ALPN or hardcoded to draft-14 for legacy
- Remove `shouldSendAuthorityParam()` and
  `shouldIncludeMoqtImplementationParam()` helpers; inline their logic
- mlog: unify `MOQTClientSetupMessage`/`MOQTServerSetupMessage` into
  `MOQTSetupMessage`; log version alongside setup params
- Tests: add framer helpers (`makeLegacyClientSetupFrame`,
  `makeLegacyServerSetupFrame`, `skipFrameHeader`) and version
  validation tests

Reviewed By: sharmafb

Differential Revision: D95678227

fbshipit-source-id: 48c4f15ff0ed14533a405b15f83727835373a1dc

Author: afrind

moxygen/MoQClientBase.cpp

@@ -13,16 +13,6 @@
 #include <utility>
 
 namespace moxygen {
-/*static*/
-bool MoQClientBase::shouldSendAuthorityParam(
-    const std::vector<uint64_t>& supportedVersions) {
-  for (const auto& version : supportedVersions) {
-    if (getDraftMajorVersion(version) >= 14) {
-      return true;
-    }
-  }
-  return false;
-}
 
 folly::coro::Task<void> MoQClientBase::setupMoQSession(
     std::chrono::milliseconds connect_timeout,
@@ -91,7 +81,7 @@ folly::coro::Task<void> MoQClientBase::setupMoQSession(
   }
 }
 
-folly::coro::Task<ServerSetup> MoQClientBase::completeSetupMoQSession(
+folly::coro::Task<Setup> MoQClientBase::completeSetupMoQSession(
     proxygen::WebTransport* wt,
     const std::optional<std::string>& pathParam,
     std::shared_ptr<Publisher> publishHandler,
@@ -116,21 +106,21 @@ folly::coro::Task<ServerSetup> MoQClientBase::completeSetupMoQSession(
   moqSession_->start();
   ClientSetup clientSetup = getClientSetup(pathParam);
   if (logger_) {
-    logger_->logClientSetup(clientSetup);
+    logger_->logClientSetup(
+        clientSetup,
+        moqSession_->getNegotiatedVersion().value_or(kVersionDraft14));
   }
   return moqSession_->setup(clientSetup);
 }
 
-ClientSetup MoQClientBase::getClientSetup(
-    const std::optional<std::string>& path) {
+Setup MoQClientBase::getClientSetup(const std::optional<std::string>& path) {
   // Setup MoQSession parameters
   // TODO: maybe let the caller set max subscribes.  Any client that publishes
   // via relay needs to support subscribes.
   const uint32_t kDefaultMaxRequestID = 100;
   const uint32_t kMaxAuthTokenCacheSize = 1024;
 
-  const auto& legacyVersions = getSupportedLegacyVersions();
-  ClientSetup clientSetup{.supportedVersions = legacyVersions};
+  Setup clientSetup;
   clientSetup.params.insertParam(Parameter(
       folly::to_underlying(SetupKey::MAX_REQUEST_ID), kDefaultMaxRequestID));
   clientSetup.params.insertParam(Parameter(
@@ -142,18 +132,16 @@ ClientSetup MoQClientBase::getClientSetup(
         SetupParameter(folly::to_underlying(SetupKey::PATH), *path));
   }
 
-  if (shouldSendAuthorityParam(clientSetup.supportedVersions)) {
-    // Add AUTHORITY parameter for Direct QUIC with moqt:// scheme only
-    if (path.has_value() && url_.getScheme() == "moqt") {
-      // Extract authority from URI as per RFC 3986
-      std::string authority = url_.getHost();
-      if (url_.getPort() != 0 && url_.getPort() != 443) {
-        authority += ":" + std::to_string(url_.getPort());
-      }
-
-      clientSetup.params.insertParam(
-          SetupParameter(folly::to_underlying(SetupKey::AUTHORITY), authority));
+  // Add AUTHORITY parameter for Direct QUIC with moqt:// scheme only
+  if (path.has_value() && url_.getScheme() == "moqt") {
+    // Extract authority from URI as per RFC 3986
+    std::string authority = url_.getHost();
+    if (url_.getPort() != 0 && url_.getPort() != 443) {
+      authority += ":" + std::to_string(url_.getPort());
     }
+
+    clientSetup.params.insertParam(
+        SetupParameter(folly::to_underlying(SetupKey::AUTHORITY), authority));
   }
 
   return clientSetup;

moxygen/MoQClientBase.h

@@ -86,8 +86,6 @@ class MoQClientBase : public proxygen::WebTransportHandler {
   std::shared_ptr<MLogger> logger_ = nullptr;
 
  protected:
-  static bool shouldSendAuthorityParam(
-      const std::vector<uint64_t>& supportedVersions);
   virtual folly::coro::Task<std::shared_ptr<quic::QuicClientTransport>>
   connectQuic(
       folly::SocketAddress connectAddr,
@@ -101,12 +99,12 @@ class MoQClientBase : public proxygen::WebTransportHandler {
 
   static SessionFactory defaultSessionFactory();
 
-  folly::coro::Task<ServerSetup> completeSetupMoQSession(
+  folly::coro::Task<Setup> completeSetupMoQSession(
       proxygen::WebTransport* wt,
       const std::optional<std::string>& pathParam,
       std::shared_ptr<Publisher> publishHandler,
       std::shared_ptr<Subscriber> subscribeHandler);
-  ClientSetup getClientSetup(const std::optional<std::string>& path);
+  Setup getClientSetup(const std::optional<std::string>& path);
 
   void onSessionEnd(folly::Optional<uint32_t>) noexcept override;
   void onSessionDrain() noexcept override;

moxygen/MoQCodec.h

@@ -71,8 +71,8 @@ class MoQControlCodec : public MoQCodec {
     ~ControlCallback() override = default;
 
     virtual void onFrame(FrameType) {}
-    virtual void onClientSetup(ClientSetup) {}
-    virtual void onServerSetup(ServerSetup) {}
+    virtual void onClientSetup(Setup) {}
+    virtual void onServerSetup(Setup) {}
     virtual void onSubscribe(SubscribeRequest) {}
     virtual void onRequestUpdate(RequestUpdate) {}
     virtual void onSubscribeOk(SubscribeOk) {}

moxygen/MoQFramer.cpp

@@ -627,10 +627,10 @@ folly::Expected<folly::Unit, ErrorCode> parseParams(
   return folly::unit;
 }
 
-folly::Expected<ClientSetup, ErrorCode> MoQFrameParser::parseClientSetup(
+folly::Expected<Setup, ErrorCode> MoQFrameParser::parseClientSetup(
     folly::io::Cursor& cursor,
     size_t length) noexcept {
-  ClientSetup clientSetup;
+  Setup clientSetup;
   uint64_t serializationVersion = kVersionDraft14;
 
   // Only parse version array when version is not initialized, i.e. alpn did not
@@ -642,20 +642,22 @@ folly::Expected<ClientSetup, ErrorCode> MoQFrameParser::parseClientSetup(
       return folly::makeUnexpected(ErrorCode::PARSE_UNDERFLOW);
     }
     length -= numVersions->second;
+    bool foundDraft14 = false;
     for (auto i = 0ul; i < numVersions->first; i++) {
       auto version = quic::follyutils::decodeQuicInteger(cursor, length);
       if (!version) {
         XLOG(DBG4) << "parseClientSetup: UNDERFLOW on version";
         return folly::makeUnexpected(ErrorCode::PARSE_UNDERFLOW);
       }
       length -= version->second;
-      if (!isSupportedVersion(version->first)) {
-        XLOG(WARN) << "Peer advertised unsupported version " << version->first
-                   << ", supported versions are: "
-                   << getSupportedVersionsString();
-        continue;
+      if (getDraftMajorVersion(version->first) == 14) {
+        foundDraft14 = true;
       }
-      clientSetup.supportedVersions.push_back(version->first);
+    }
+    if (!foundDraft14) {
+      XLOG(ERR) << "Draft-14 not found in ClientSetup version array"
+                   " (legacy mode only supports draft-14)";
+      return folly::makeUnexpected(ErrorCode::VERSION_NEGOTIATION_FAILED);
     }
   } else {
     XLOG(DBG3)
@@ -688,10 +690,11 @@ folly::Expected<ClientSetup, ErrorCode> MoQFrameParser::parseClientSetup(
   return clientSetup;
 }
 
-folly::Expected<ServerSetup, ErrorCode> MoQFrameParser::parseServerSetup(
+folly::Expected<Setup, ErrorCode> MoQFrameParser::parseServerSetup(
     folly::io::Cursor& cursor,
     size_t length) noexcept {
-  ServerSetup serverSetup;
+  Setup serverSetup;
+  uint64_t serializationVersion = kVersionDraft14;
 
   // Only parse version when version is not initialized, i.e. alpn did not
   // happen, or when version is initialized but is < 15 (in tests)
@@ -702,16 +705,17 @@ folly::Expected<ServerSetup, ErrorCode> MoQFrameParser::parseServerSetup(
       return folly::makeUnexpected(ErrorCode::PARSE_UNDERFLOW);
     }
     length -= version->second;
-    if (!isSupportedVersion(version->first)) {
-      XLOG(WARN) << "Peer advertised unsupported version " << version->first
-                 << ", supported versions are: "
-                 << getSupportedVersionsString();
-      return folly::makeUnexpected(ErrorCode::PROTOCOL_VIOLATION);
+    if (getDraftMajorVersion(version->first) != 14) {
+      XLOG(ERR) << "Server selected version draft-"
+                << getDraftMajorVersion(version->first)
+                << " but we only offer draft-14 in legacy mode";
+      return folly::makeUnexpected(ErrorCode::VERSION_NEGOTIATION_FAILED);
     }
-    serverSetup.selectedVersion = version->first;
+    serializationVersion = version->first;
   } else {
     XLOG(DBG3)
         << "Skipped parsing version from wire for alpn ServerSetup message";
+    serializationVersion = *version_;
   }
 
   auto numParams = quic::follyutils::decodeQuicInteger(cursor, length);
@@ -724,7 +728,7 @@ folly::Expected<ServerSetup, ErrorCode> MoQFrameParser::parseServerSetup(
   auto res = parseParams(
       cursor,
       length,
-      version_ ? *version_ : serverSetup.selectedVersion,
+      serializationVersion,
       numParams->first,
       serverSetup.params,
       requestSpecificParams,
@@ -3500,7 +3504,7 @@ bool includeSetupParam(uint64_t version, SetupKey key) {
 
 WriteResult writeClientSetup(
     folly::IOBufQueue& writeBuf,
-    const ClientSetup& clientSetup,
+    const Setup& clientSetup,
     uint64_t version) noexcept {
   size_t size = 0;
   bool error = false;
@@ -3509,17 +3513,12 @@ WriteResult writeClientSetup(
   auto sizePtr = writeFrameHeader(writeBuf, frameType, error);
 
   if (getDraftMajorVersion(version) < 15) {
-    // Check that all versions are supported
-    for (auto ver : clientSetup.supportedVersions) {
-      XCHECK(isSupportedVersion(ver))
-          << "Version " << ver << " is not supported. Supported versions are: "
-          << getSupportedVersionsString();
-    }
-    // Only write version array in non-alpn mode
-    writeVarint(writeBuf, clientSetup.supportedVersions.size(), size, error);
-    for (auto ver : clientSetup.supportedVersions) {
-      writeVarint(writeBuf, ver, size, error);
-    }
+    XCHECK_EQ(getDraftMajorVersion(version), 14u)
+        << "Legacy mode only supports draft-14, got draft-"
+        << getDraftMajorVersion(version);
+    // Only write version array in non-alpn mode, hardcode draft-14
+    writeVarint(writeBuf, 1, size, error);
+    writeVarint(writeBuf, kVersionDraft14, size, error);
   } else {
     XLOG(DBG3)
         << "Skipped writing versions to wire for alpn ClientSetup message";
@@ -3563,7 +3562,7 @@ WriteResult writeClientSetup(
 
 WriteResult writeServerSetup(
     folly::IOBufQueue& writeBuf,
-    const ServerSetup& serverSetup,
+    const Setup& serverSetup,
     uint64_t version) noexcept {
   size_t size = 0;
   bool error = false;
@@ -3573,20 +3572,19 @@ WriteResult writeServerSetup(
 
   // Only write selected version in non-alpn mode
   if (getDraftMajorVersion(version) < 15) {
-    XCHECK(isSupportedVersion(serverSetup.selectedVersion))
-        << "Supported version " << serverSetup.selectedVersion
-        << ") is not supported. Supported versions are: "
-        << getSupportedVersionsString();
-    writeVarint(writeBuf, serverSetup.selectedVersion, size, error);
+    XCHECK_EQ(getDraftMajorVersion(version), 14u)
+        << "Legacy mode only supports draft-14, got draft-"
+        << getDraftMajorVersion(version);
+    writeVarint(writeBuf, kVersionDraft14, size, error);
   } else {
     XLOG(DBG3)
-        << "Skipped writing version to wire for alpn ClientSetup message";
+        << "Skipped writing version to wire for alpn ServerSetup message";
   }
 
   // Collect params that should be included
   std::vector<Parameter> filteredParams;
   for (const auto& param : serverSetup.params) {
-    if (includeSetupParam(serverSetup.selectedVersion, SetupKey(param.key))) {
+    if (includeSetupParam(version, SetupKey(param.key))) {
       filteredParams.push_back(param);
     }
   }

moxygen/MoQFramer.h

@@ -111,11 +111,11 @@ class MoQFrameParser {
     T value;
     size_t bytesConsumed;
   };
-  folly::Expected<ClientSetup, ErrorCode> parseClientSetup(
+  folly::Expected<Setup, ErrorCode> parseClientSetup(
       folly::io::Cursor& cursor,
       size_t length) noexcept;
 
-  folly::Expected<ServerSetup, ErrorCode> parseServerSetup(
+  folly::Expected<Setup, ErrorCode> parseServerSetup(
       folly::io::Cursor& cursor,
       size_t length) noexcept;
 
@@ -435,12 +435,12 @@ TrackRequestParameter getAuthParam(
 
 WriteResult writeClientSetup(
     folly::IOBufQueue& writeBuf,
-    const ClientSetup& clientSetup,
+    const Setup& clientSetup,
     uint64_t version) noexcept;
 
 WriteResult writeServerSetup(
     folly::IOBufQueue& writeBuf,
-    const ServerSetup& serverSetup,
+    const Setup& serverSetup,
     uint64_t version) noexcept;
 
 // writeClientSetup and writeServerSetup are the only two functions that

moxygen/MoQServerBase.cpp

@@ -46,54 +46,26 @@ folly::coro::Task<void> MoQServerBase::handleClientSession(
   terminateClientSession(std::move(clientSession));
 }
 
-folly::Try<ServerSetup> MoQServerBase::onClientSetup(
-    ClientSetup setup,
+folly::Try<Setup> MoQServerBase::onClientSetup(
+    Setup setup,
     const std::shared_ptr<MoQSession>& session) {
   XLOG(DBG1) << "MoQServerBase::ClientSetup";
 
-  uint64_t negotiatedVersion = 0;
-
-  // Check if version was negotiated via ALPN first (takes precedence)
+  // Version is either negotiated via ALPN or defaults to draft-14
   auto sessionVersion = session->getNegotiatedVersion();
   if (sessionVersion) {
-    // ALPN mode: use the ALPN-negotiated version
-    negotiatedVersion = *sessionVersion;
     XLOG(DBG1)
         << "MoQServerBase::ClientSetup: Using ALPN-negotiated version: moqt-"
-        << getDraftMajorVersion(negotiatedVersion);
-  } else if (!setup.supportedVersions.empty()) {
-    // Legacy mode: negotiate from version array in CLIENT_SETUP
-    // Iterate over supported versions and set the highest version within the
-    // range
-    uint64_t highestVersion = 0;
-    for (const auto& version : setup.supportedVersions) {
-      if (getDraftMajorVersion(version) >= 15) {
-        XLOG(WARN) << "MoQServerBase::ClientSetup: Skiping version " << version
-                   << " (which needs alpn negotiation), to attempt fallback.";
-        continue;
-      }
-      if (isSupportedVersion(version)) {
-        highestVersion = std::max(highestVersion, version);
-      }
-    }
-    if (highestVersion == 0) {
-      std::string errorMessage = folly::to<std::string>(
-          "The only supported versions in client_setup are ",
-          getSupportedVersionsString());
-      return folly::Try<ServerSetup>(std::runtime_error(errorMessage));
-    }
-    negotiatedVersion = highestVersion;
+        << getDraftMajorVersion(*sessionVersion);
   } else {
-    // No version available from either ALPN or CLIENT_SETUP
-    return folly::Try<ServerSetup>(
-        std::runtime_error("No version negotiated via ALPN or CLIENT_SETUP"));
+    XLOG(DBG1) << "MoQServerBase::ClientSetup: No ALPN, using draft-14";
   }
 
   // TODO: Make the default MAX_REQUEST_ID configurable and
   // take in the value from ClientSetup
   static constexpr size_t kDefaultMaxRequestID = 100;
   static constexpr size_t kMaxAuthTokenCacheSize = 1024;
-  ServerSetup serverSetup{.selectedVersion = negotiatedVersion};
+  Setup serverSetup;
   serverSetup.params.insertParam(
       Parameter{
           folly::to_underlying(SetupKey::MAX_REQUEST_ID),
@@ -105,15 +77,16 @@ folly::Try<ServerSetup> MoQServerBase::onClientSetup(
 
   // Log Server Setup
   if (auto logger = session->getLogger()) {
-    logger->logServerSetup(serverSetup);
+    logger->logServerSetup(
+        serverSetup, sessionVersion.value_or(kVersionDraft14));
   }
 
-  return folly::Try<ServerSetup>(serverSetup);
+  return folly::Try<Setup>(serverSetup);
 }
 
 folly::Expected<folly::Unit, SessionCloseErrorCode>
 MoQServerBase::validateAuthority(
-    const ClientSetup& setup,
+    const Setup& setup,
     uint64_t negotiatedVersion,
     std::shared_ptr<MoQSession>) {
   if (getDraftMajorVersion(negotiatedVersion) >= 14) {

moxygen/MoQServerBase.h

@@ -59,12 +59,12 @@ class MoQServerBase : public MoQSession::ServerSetupCallback {
   void setMLoggerFactory(std::shared_ptr<MLoggerFactory> factory);
 
   // ServerSetupCallback overrides
-  folly::Try<ServerSetup> onClientSetup(
-      ClientSetup clientSetup,
+  folly::Try<Setup> onClientSetup(
+      Setup clientSetup,
       const std::shared_ptr<MoQSession>& session) override;
 
   folly::Expected<folly::Unit, SessionCloseErrorCode> validateAuthority(
-      const ClientSetup& clientSetup,
+      const Setup& clientSetup,
       uint64_t negotiatedVersion,
       std::shared_ptr<MoQSession> session) override;