Skip to content

Commit 83b540f

Browse files
martindukecopybara-github
martinduke
authored and
copybara-github
committed
Implement MOQT draft-16 version negotiation.
Breaks interop with all hosts, as no other version-16 features are yet supported. Integration tests will fail until WebTransport APIs are in place. PiperOrigin-RevId: 853922338
1 parent c589b34 commit 83b540f

13 files changed

+117
-134
lines changed

‎quiche/quic/moqt/moqt_framer.cc‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -401,8 +401,6 @@ quiche::QuicheBuffer MoqtFramer::SerializeClientSetup(
401401
}
402402
return SerializeControlMessage(
403403
MoqtMessageType::kClientSetup,
404-
WireVarInt62(message.supported_versions.size()),
405-
WireSpan<WireVarInt62, MoqtVersion>(message.supported_versions),
406404
WireKeyValuePairList(parameters));
407405
}
408406

@@ -418,7 +416,6 @@ quiche::QuicheBuffer MoqtFramer::SerializeServerSetup(
418416
return quiche::QuicheBuffer();
419417
}
420418
return SerializeControlMessage(MoqtMessageType::kServerSetup,
421-
WireVarInt62(message.selected_version),
422419
WireKeyValuePairList(parameters));
423420
}
424421

‎quiche/quic/moqt/moqt_integration_test.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,9 @@ TEST_F(MoqtIntegrationTest, Handshake) {
142142
}
143143

144144
TEST_F(MoqtIntegrationTest, VersionMismatch) {
145-
client_ = std::make_unique<MoqtClientEndpoint>(
146-
&test_harness_.simulator(), "Client", "Server",
147-
MoqtVersion::kUnrecognizedVersionForTests);
145+
client_ = std::make_unique<MoqtClientEndpoint>(&test_harness_.simulator(),
146+
"Client", "Server",
147+
kUnrecognizedVersionForTests);
148148
server_ = std::make_unique<MoqtServerEndpoint>(
149149
&test_harness_.simulator(), "Server", "Client", kDefaultMoqtVersion);
150150
SetupCallbacks();

‎quiche/quic/moqt/moqt_messages.h‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,12 @@ inline constexpr quic::ParsedQuicVersionVector GetMoqtSupportedQuicVersions() {
3838
return quic::ParsedQuicVersionVector{quic::ParsedQuicVersion::RFCv1()};
3939
}
4040

41-
enum class MoqtVersion : uint64_t {
42-
kDraft14 = 0xff00000e,
43-
kUnrecognizedVersionForTests = 0xfe0000ff,
44-
};
41+
inline constexpr absl::string_view kDraft16 = "moqt-16";
42+
inline constexpr absl::string_view kDefaultMoqtVersion = kDraft16;
43+
inline constexpr absl::string_view kImplementationName =
44+
"Google QUICHE MOQT draft 16";
45+
inline constexpr absl::string_view kUnrecognizedVersionForTests = "moqt-15";
4546

46-
inline constexpr MoqtVersion kDefaultMoqtVersion = MoqtVersion::kDraft14;
47-
inline constexpr absl::string_view kVersionString =
48-
"Google QUICHE MOQT draft 14";
4947
inline constexpr uint64_t kDefaultInitialMaxRequestId = 100;
5048
// TODO(martinduke): Implement an auth token cache.
5149
inline constexpr uint64_t kDefaultMaxAuthTokenCacheSize = 0;
@@ -97,7 +95,7 @@ struct QUICHE_EXPORT MoqtSessionParameters {
9795
: perspective(perspective), max_request_id(max_request_id) {}
9896
bool operator==(const MoqtSessionParameters& other) const = default;
9997

100-
MoqtVersion version = kDefaultMoqtVersion;
98+
std::string version = std::string(kDefaultMoqtVersion);
10199
bool deliver_partial_objects = false;
102100
quic::Perspective perspective = quic::Perspective::IS_SERVER;
103101
bool using_webtrans = true;
@@ -535,12 +533,10 @@ class KeyValuePairList {
535533

536534
// TODO(martinduke): Collapse both Setup messages into MoqtSessionParameters.
537535
struct QUICHE_EXPORT MoqtClientSetup {
538-
std::vector<MoqtVersion> supported_versions;
539536
MoqtSessionParameters parameters;
540537
};
541538

542539
struct QUICHE_EXPORT MoqtServerSetup {
543-
MoqtVersion selected_version;
544540
MoqtSessionParameters parameters;
545541
};
546542

‎quiche/quic/moqt/moqt_parser.cc‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -325,17 +325,6 @@ size_t MoqtControlParser::ProcessClientSetup(quic::QuicDataReader& reader) {
325325
MoqtClientSetup setup;
326326
setup.parameters.using_webtrans = uses_web_transport_;
327327
setup.parameters.perspective = quic::Perspective::IS_CLIENT;
328-
uint64_t number_of_supported_versions;
329-
if (!reader.ReadVarInt62(&number_of_supported_versions)) {
330-
return 0;
331-
}
332-
uint64_t version;
333-
for (uint64_t i = 0; i < number_of_supported_versions; ++i) {
334-
if (!reader.ReadVarInt62(&version)) {
335-
return 0;
336-
}
337-
setup.supported_versions.push_back(static_cast<MoqtVersion>(version));
338-
}
339328
KeyValuePairList parameters;
340329
if (!ParseKeyValuePairList(reader, parameters)) {
341330
return 0;
@@ -358,11 +347,6 @@ size_t MoqtControlParser::ProcessServerSetup(quic::QuicDataReader& reader) {
358347
MoqtServerSetup setup;
359348
setup.parameters.using_webtrans = uses_web_transport_;
360349
setup.parameters.perspective = quic::Perspective::IS_SERVER;
361-
uint64_t version;
362-
if (!reader.ReadVarInt62(&version)) {
363-
return 0;
364-
}
365-
setup.selected_version = static_cast<MoqtVersion>(version);
366350
KeyValuePairList parameters;
367351
if (!ParseKeyValuePairList(reader, parameters)) {
368352
return 0;

‎quiche/quic/moqt/moqt_parser_test.cc‎

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -491,11 +491,11 @@ TEST_F(MoqtMessageSpecificTest, ClientSetupMaxRequestIdAppearsTwice) {
491491
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
492492
MoqtControlParser parser(kRawQuic, &stream, visitor_);
493493
char setup[] = {
494-
0x20, 0x00, 0x0d, 0x02, 0x01, 0x02, // versions
495-
0x03, // 3 params
496-
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
497-
0x01, 0x32, // max_request_id = 50
498-
0x00, 0x32, // max_request_id = 50
494+
0x20, 0x00, 0x0a,
495+
0x03, // 3 params
496+
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
497+
0x01, 0x32, // max_request_id = 50
498+
0x00, 0x32, // max_request_id = 50
499499
};
500500
stream.Receive(absl::string_view(setup, sizeof(setup)), false);
501501
parser.ReadAndDispatchMessages();
@@ -509,7 +509,7 @@ TEST_F(MoqtMessageSpecificTest, ClientSetupAuthorizationTokenTagRegister) {
509509
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
510510
MoqtControlParser parser(kRawQuic, &stream, visitor_);
511511
char setup[] = {
512-
0x20, 0x00, 0x13, 0x02, 0x01, 0x02, // versions
512+
0x20, 0x00, 0x10,
513513
0x03, // 3 params
514514
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
515515
0x01, 0x32, // max_request_id = 50
@@ -525,8 +525,7 @@ TEST_F(MoqtMessageSpecificTest, SetupPathFromServer) {
525525
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
526526
MoqtControlParser parser(kRawQuic, &stream, visitor_);
527527
char setup[] = {
528-
0x21, 0x00, 0x07,
529-
0x01, // version = 1
528+
0x21, 0x00, 0x06,
530529
0x01, // 1 param
531530
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
532531
};
@@ -542,8 +541,7 @@ TEST_F(MoqtMessageSpecificTest, SetupAuthorityFromServer) {
542541
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
543542
MoqtControlParser parser(kRawQuic, &stream, visitor_);
544543
char setup[] = {
545-
0x21, 0x00, 0x07,
546-
0x01, // version = 1
544+
0x21, 0x00, 0x06,
547545
0x01, // 1 param
548546
0x05, 0x03, 0x66, 0x6f, 0x6f, // authority = "foo"
549547
};
@@ -559,10 +557,10 @@ TEST_F(MoqtMessageSpecificTest, SetupPathAppearsTwice) {
559557
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
560558
MoqtControlParser parser(kRawQuic, &stream, visitor_);
561559
char setup[] = {
562-
0x20, 0x00, 0x0e, 0x02, 0x01, 0x02, // versions = 1, 2
563-
0x02, // 2 params
564-
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
565-
0x00, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
560+
0x20, 0x00, 0x0b,
561+
0x02, // 2 params
562+
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
563+
0x00, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
566564
};
567565
stream.Receive(absl::string_view(setup, sizeof(setup)), false);
568566
parser.ReadAndDispatchMessages();
@@ -576,9 +574,9 @@ TEST_F(MoqtMessageSpecificTest, SetupPathOverWebtrans) {
576574
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
577575
MoqtControlParser parser(kWebTrans, &stream, visitor_);
578576
char setup[] = {
579-
0x20, 0x00, 0x09, 0x02, 0x01, 0x02, // versions = 1, 2
580-
0x01, // 1 param
581-
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
577+
0x20, 0x00, 0x06,
578+
0x01, // 1 param
579+
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
582580
};
583581
stream.Receive(absl::string_view(setup, sizeof(setup)), false);
584582
parser.ReadAndDispatchMessages();
@@ -592,9 +590,9 @@ TEST_F(MoqtMessageSpecificTest, SetupAuthorityOverWebtrans) {
592590
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
593591
MoqtControlParser parser(kWebTrans, &stream, visitor_);
594592
char setup[] = {
595-
0x20, 0x00, 0x09, 0x02, 0x01, 0x02, // versions = 1, 2
596-
0x01, // 1 param
597-
0x05, 0x03, 0x66, 0x6f, 0x6f, // authority = "foo"
593+
0x20, 0x00, 0x06,
594+
0x01, // 1 param
595+
0x05, 0x03, 0x66, 0x6f, 0x6f, // authority = "foo"
598596
};
599597
stream.Receive(absl::string_view(setup, sizeof(setup)), false);
600598
parser.ReadAndDispatchMessages();
@@ -608,8 +606,10 @@ TEST_F(MoqtMessageSpecificTest, SetupPathMissing) {
608606
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
609607
MoqtControlParser parser(kRawQuic, &stream, visitor_);
610608
char setup[] = {
611-
0x20, 0x00, 0x04, 0x02, 0x01, 0x02, // versions = 1, 2
612-
0x00, // no param
609+
0x20,
610+
0x00,
611+
0x01,
612+
0x00, // no param
613613
};
614614
stream.Receive(absl::string_view(setup, sizeof(setup)), false);
615615
parser.ReadAndDispatchMessages();
@@ -623,11 +623,11 @@ TEST_F(MoqtMessageSpecificTest, ServerSetupMaxRequestIdAppearsTwice) {
623623
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
624624
MoqtControlParser parser(kRawQuic, &stream, visitor_);
625625
char setup[] = {
626-
0x20, 0x00, 0x0d, 0x02, 0x01, 0x02, // versions = 1, 2
627-
0x03, // 4 params
628-
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
629-
0x01, 0x32, // max_request_id = 50
630-
0x00, 0x32, // max_request_id = 50
626+
0x20, 0x00, 0x0a,
627+
0x03, // 3 params
628+
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
629+
0x01, 0x32, // max_request_id = 50
630+
0x00, 0x32, // max_request_id = 50
631631
};
632632
stream.Receive(absl::string_view(setup, sizeof(setup)), false);
633633
parser.ReadAndDispatchMessages();
@@ -641,9 +641,9 @@ TEST_F(MoqtMessageSpecificTest, ServerSetupMalformedPath) {
641641
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
642642
MoqtControlParser parser(kRawQuic, &stream, visitor_);
643643
char setup[] = {
644-
0x20, 0x00, 0x09, 0x02, 0x01, 0x02, // versions = 1, 2
645-
0x01, // 1 param
646-
0x01, 0x03, 0x66, 0x5c, 0x6f, // path = "f\o"
644+
0x20, 0x00, 0x06,
645+
0x01, // 1 param
646+
0x01, 0x03, 0x66, 0x5c, 0x6f, // path = "f\o"
647647
};
648648
stream.Receive(absl::string_view(setup, sizeof(setup)), false);
649649
parser.ReadAndDispatchMessages();
@@ -656,10 +656,10 @@ TEST_F(MoqtMessageSpecificTest, ServerSetupMalformedAuthority) {
656656
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
657657
MoqtControlParser parser(kRawQuic, &stream, visitor_);
658658
char setup[] = {
659-
0x20, 0x00, 0x0e, 0x02, 0x01, 0x02, // versions = 1, 2
660-
0x02, // 2 params
661-
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
662-
0x04, 0x03, 0x66, 0x5c, 0x6f, // authority = "f\o"
659+
0x20, 0x00, 0x0b,
660+
0x02, // 2 params
661+
0x01, 0x03, 0x66, 0x6f, 0x6f, // path = "foo"
662+
0x04, 0x03, 0x66, 0x5c, 0x6f, // authority = "f\o"
663663
};
664664
stream.Receive(absl::string_view(setup, sizeof(setup)), false);
665665
parser.ReadAndDispatchMessages();

‎quiche/quic/moqt/moqt_session.cc‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include <vector>
1616

1717

18-
#include "absl/algorithm/container.h"
1918
#include "absl/base/nullability.h"
2019
#include "absl/container/btree_map.h"
2120
#include "absl/container/flat_hash_map.h"
@@ -121,7 +120,7 @@ MoqtSession::MoqtSession(webtransport::Session* session,
121120
next_incoming_request_id_ = 1;
122121
}
123122
QUICHE_DCHECK(parameters_.moqt_implementation.empty());
124-
parameters_.moqt_implementation = kVersionString;
123+
parameters_.moqt_implementation = kImplementationName;
125124
}
126125

127126
MoqtSession::ControlStream* MoqtSession::GetControlStream() {
@@ -147,10 +146,15 @@ void MoqtSession::SendControlMessage(quiche::QuicheBuffer message) {
147146

148147
void MoqtSession::OnSessionReady() {
149148
QUICHE_DLOG(INFO) << ENDPOINT << "Underlying session ready";
149+
std::optional<std::string> version = session_->GetNegotiatedSubprotocol();
150+
if (version != parameters_.version) {
151+
Error(MoqtError::kVersionNegotiationFailed,
152+
"MOQT peer chose wrong subprotocol");
153+
return;
154+
}
150155
if (parameters_.perspective == Perspective::IS_SERVER) {
151156
return;
152157
}
153-
154158
webtransport::Stream* control_stream =
155159
session_->OpenOutgoingBidirectionalStream();
156160
if (control_stream == nullptr) {
@@ -161,7 +165,6 @@ void MoqtSession::OnSessionReady() {
161165
std::make_unique<ControlStream>(this, control_stream));
162166
control_stream_ = control_stream->GetStreamId();
163167
MoqtClientSetup setup = MoqtClientSetup{
164-
.supported_versions = std::vector<MoqtVersion>{parameters_.version},
165168
.parameters = parameters_,
166169
};
167170
SendControlMessage(framer_.SerializeClientSetup(setup));
@@ -990,20 +993,11 @@ void MoqtSession::ControlStream::OnClientSetupMessage(
990993
"Received CLIENT_SETUP from server");
991994
return;
992995
}
993-
if (absl::c_find(message.supported_versions, session_->parameters_.version) ==
994-
message.supported_versions.end()) {
995-
// TODO(martinduke): Is this the right error code? See issue #346.
996-
session_->Error(MoqtError::kVersionNegotiationFailed,
997-
absl::StrCat("Version mismatch: expected 0x",
998-
absl::Hex(session_->parameters_.version)));
999-
return;
1000-
}
1001996
session_->peer_supports_object_ack_ = message.parameters.support_object_acks;
1002997
QUICHE_DLOG(INFO) << ENDPOINT << "Received the SETUP message";
1003998
if (session_->parameters_.perspective == Perspective::IS_SERVER) {
1004999
MoqtServerSetup response;
10051000
response.parameters = session_->parameters_;
1006-
response.selected_version = session_->parameters_.version;
10071001
SendOrBufferMessage(session_->framer_.SerializeServerSetup(response));
10081002
QUIC_DLOG(INFO) << ENDPOINT << "Sent the SETUP message";
10091003
}
@@ -1019,13 +1013,6 @@ void MoqtSession::ControlStream::OnServerSetupMessage(
10191013
"Received SERVER_SETUP from client");
10201014
return;
10211015
}
1022-
if (message.selected_version != session_->parameters_.version) {
1023-
// TODO(martinduke): Is this the right error code? See issue #346.
1024-
session_->Error(MoqtError::kProtocolViolation,
1025-
absl::StrCat("Version mismatch: expected 0x",
1026-
absl::Hex(session_->parameters_.version)));
1027-
return;
1028-
}
10291016
session_->peer_supports_object_ack_ = message.parameters.support_object_acks;
10301017
QUIC_DLOG(INFO) << ENDPOINT << "Received the SETUP message";
10311018
// TODO: handle path.

0 commit comments

Comments
 (0)