Skip to content

Commit 78bef07

Browse files
afrindmeta-codesync[bot]
authored andcommitted
Reject unexpected frame types on bidi streams
Summary: Previously, bidiStreamDemuxer silently ignored unrecognized frame types, causing connections to hang until idle timeout (30s). This was observed with clients sending LEGACY_CLIENT_SETUP (0x40) after negotiating moqt-16 via ALPN, where CLIENT_SETUP is 0x20. Now the session immediately closes with PROTOCOL_VIOLATION and logs the unexpected frame type. Reviewed By: sharmafb Differential Revision: D96628382 fbshipit-source-id: 23f9894fe3a98039d60585749beb72e623810c23
1 parent d6a62bc commit 78bef07

2 files changed

Lines changed: 58 additions & 0 deletions

File tree

moxygen/MoQSession.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5312,6 +5312,10 @@ folly::coro::Task<void> MoQSession::bidiStreamDemuxer(
53125312
subscribeNamespaceReceiverReadLoop(
53135313
std::move(bh), std::move(accumulatedData))))
53145314
.start();
5315+
} else {
5316+
XLOG(ERR) << "Unexpected frame type on bidi stream: "
5317+
<< folly::to_underlying(*frameType) << " sess=" << this;
5318+
close(SessionCloseErrorCode::PROTOCOL_VIOLATION);
53155319
}
53165320
}
53175321
}

moxygen/test/MoQSessionTests.cpp

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,60 @@ TEST(MoQSessionTest, SharedPtrCycleBreaksOnCleanup) {
530530
EXPECT_TRUE(weakSession.expired()) << "session freed after close()";
531531
}
532532

533+
TEST(MoQSessionTest, BidiStreamRejectsUnexpectedFrameType) {
534+
folly::EventBase eventBase;
535+
auto moqExecutor = std::make_shared<MoQFollyExecutorImpl>(&eventBase);
536+
auto [clientWt, serverWt] =
537+
proxygen::test::FakeSharedWebTransport::makeSharedWebTransport();
538+
539+
class TestServerSetupCallback : public MoQSession::ServerSetupCallback {
540+
public:
541+
folly::Try<ServerSetup> onClientSetup(
542+
ClientSetup,
543+
const std::shared_ptr<MoQSession>&) override {
544+
return folly::Try<ServerSetup>(ServerSetup{});
545+
}
546+
folly::Expected<folly::Unit, SessionCloseErrorCode> validateAuthority(
547+
const ClientSetup&,
548+
uint64_t,
549+
std::shared_ptr<MoQSession>) override {
550+
return folly::unit;
551+
}
552+
};
553+
554+
TestServerSetupCallback serverSetupCallback;
555+
auto serverSession = std::make_shared<MoQRelaySession>(
556+
folly::MaybeManagedPtr<proxygen::WebTransport>(serverWt.get()),
557+
serverSetupCallback,
558+
moqExecutor);
559+
560+
// Set version to moqt-16 via ALPN so bidiStreamDemuxer is used
561+
serverSession->validateAndSetVersionFromAlpn("moqt-16");
562+
563+
// Set the peer handler so createBidiStream triggers onNewBidiStream
564+
clientWt->setPeerHandler(serverSession.get());
565+
566+
// Create a bidi stream from the client side
567+
auto bidiResult = clientWt->createBidiStream();
568+
ASSERT_TRUE(bidiResult.hasValue());
569+
auto writeHandle = bidiResult->writeHandle;
570+
571+
// Write LEGACY_CLIENT_SETUP frame type (0x40) which is invalid for moqt-16.
572+
// 0x40 as a QUIC varint encodes as two bytes: 0x40 0x40
573+
auto buf = folly::IOBuf::create(4);
574+
buf->append(4);
575+
buf->writableData()[0] = 0x40;
576+
buf->writableData()[1] = 0x40;
577+
buf->writableData()[2] = 0x00;
578+
buf->writableData()[3] = 0x00;
579+
writeHandle->writeStreamData(std::move(buf), false, nullptr);
580+
581+
eventBase.loop();
582+
583+
// The server should have closed the session due to unexpected frame type
584+
EXPECT_TRUE(serverWt->isSessionClosed());
585+
}
586+
533587
INSTANTIATE_TEST_SUITE_P(
534588
MoQSessionTest,
535589
MoQSessionTest,

0 commit comments

Comments
 (0)