diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc index a1ad8d6427..205fdbb092 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc @@ -13,12 +13,12 @@ #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" - -using webrtc::RTCPUtility::RtcpCommonHeader; +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/common_header.h" namespace webrtc { namespace rtcp { - +constexpr uint8_t App::kPacketType; +constexpr size_t App::kMaxDataSize; // Application-Defined packet (APP) (RFC 3550). // // 0 1 2 3 @@ -32,13 +32,22 @@ namespace rtcp { // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // 8 | application-dependent data ... // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ -bool App::Parse(const RtcpCommonHeader& header, const uint8_t* payload) { - RTC_DCHECK(header.packet_type == kPacketType); - - sub_type_ = header.count_or_format; - ssrc_ = ByteReader::ReadBigEndian(&payload[0]); - name_ = ByteReader::ReadBigEndian(&payload[4]); - data_.SetData(&payload[8], header.payload_size_bytes - 8); +bool App::Parse(const CommonHeader& packet) { + RTC_DCHECK_EQ(packet.type(), kPacketType); + if (packet.payload_size_bytes() < kAppBaseLength) { + LOG(LS_WARNING) << "Packet is too small to be a valid APP packet"; + return false; + } + if (packet.payload_size_bytes() % 4 != 0) { + LOG(LS_WARNING) + << "Packet payload must be 32 bits aligned to make a valid APP packet"; + return false; + } + sub_type_ = packet.fmt(); + ssrc_ = ByteReader::ReadBigEndian(&packet.payload()[0]); + name_ = ByteReader::ReadBigEndian(&packet.payload()[4]); + data_.SetData(packet.payload() + kAppBaseLength, + packet.payload_size_bytes() - kAppBaseLength); return true; } @@ -49,10 +58,10 @@ void App::WithSubType(uint8_t subtype) { void App::WithData(const uint8_t* data, size_t data_length) { RTC_DCHECK(data); - RTC_DCHECK_EQ(0u, data_length % 4) << "Data must be 32 bits aligned."; - RTC_DCHECK(data_length <= kMaxDataSize) << "App data size << " << data_length - << "exceed maximum of " - << kMaxDataSize << " bytes."; + RTC_DCHECK_EQ(data_length % 4, 0u) << "Data must be 32 bits aligned."; + RTC_DCHECK_LE(data_length, kMaxDataSize) << "App data size " << data_length + << " exceed maximum of " + << kMaxDataSize << " bytes."; data_.SetData(data, data_length); } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/app.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/app.h index f5a885cfdd..fea55e4b65 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/app.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/app.h @@ -13,26 +13,20 @@ #include "webrtc/base/buffer.h" #include "webrtc/base/constructormagic.h" -#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet.h" -#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" namespace webrtc { namespace rtcp { +class CommonHeader; class App : public RtcpPacket { public: - static const uint8_t kPacketType = 204; - // 28 bytes for UDP header - // 12 bytes for RTCP app header - static const size_t kMaxDataSize = IP_PACKET_SIZE - 12 - 28; + static constexpr uint8_t kPacketType = 204; App() : sub_type_(0), ssrc_(0), name_(0) {} - - virtual ~App() {} + ~App() override {} // Parse assumes header is already parsed and validated. - bool Parse(const RTCPUtility::RtcpCommonHeader& header, - const uint8_t* payload); // Size of the payload is in the header. + bool Parse(const CommonHeader& packet); void From(uint32_t ssrc) { ssrc_ = ssrc; } void WithSubType(uint8_t subtype); @@ -52,7 +46,11 @@ class App : public RtcpPacket { RtcpPacket::PacketReadyCallback* callback) const override; private: - size_t BlockLength() const override { return 12 + data_.size(); } + static constexpr size_t kAppBaseLength = 8; // Ssrc and Name. + static constexpr size_t kMaxDataSize = 0xffff * 4 - kAppBaseLength; + size_t BlockLength() const override { + return kHeaderLength + kAppBaseLength + data_.size(); + } uint8_t sub_type_; uint32_t ssrc_; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/app_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/app_unittest.cc index 2aa4d94e78..abc8f04f00 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/app_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/app_unittest.cc @@ -10,70 +10,101 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/app.h" -#include - +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" -#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" - -using webrtc::rtcp::App; -using webrtc::RTCPUtility::RtcpCommonHeader; -using webrtc::RTCPUtility::RtcpParseCommonHeader; +#include "webrtc/test/rtcp_packet_parser.h" namespace webrtc { namespace { -const uint32_t kName = ((uint32_t)'n' << 24) | ((uint32_t)'a' << 16) | - ((uint32_t)'m' << 8) | (uint32_t)'e'; -const uint32_t kSenderSsrc = 0x12345678; - -class RtcpPacketAppTest : public ::testing::Test { - protected: - void BuildPacket() { packet = app.Build(); } - void ParsePacket() { - RtcpCommonHeader header; - EXPECT_TRUE(RtcpParseCommonHeader(packet.data(), packet.size(), &header)); - // Check there is exactly one RTCP packet in the buffer. - EXPECT_EQ(header.BlockSize(), packet.size()); - EXPECT_TRUE(parsed_.Parse( - header, packet.data() + RtcpCommonHeader::kHeaderSizeBytes)); - } - - App app; - rtc::Buffer packet; - const App& parsed() { return parsed_; } - - private: - App parsed_; -}; - -TEST_F(RtcpPacketAppTest, WithNoData) { - app.WithSubType(30); - app.WithName(kName); - - BuildPacket(); - ParsePacket(); - - EXPECT_EQ(30U, parsed().sub_type()); - EXPECT_EQ(kName, parsed().name()); - EXPECT_EQ(0u, parsed().data_size()); -} - -TEST_F(RtcpPacketAppTest, WithData) { - app.From(kSenderSsrc); - app.WithSubType(30); - app.WithName(kName); - const uint8_t kData[] = {'t', 'e', 's', 't', 'd', 'a', 't', 'a'}; - const size_t kDataLength = sizeof(kData) / sizeof(kData[0]); - app.WithData(kData, kDataLength); - - BuildPacket(); - ParsePacket(); - - EXPECT_EQ(30U, parsed().sub_type()); - EXPECT_EQ(kName, parsed().name()); - EXPECT_EQ(kDataLength, parsed().data_size()); - EXPECT_EQ(0, memcmp(kData, parsed().data(), kDataLength)); -} +using ::testing::ElementsAreArray; +using ::testing::make_tuple; +using ::webrtc::rtcp::App; +constexpr uint32_t kName = ((uint32_t)'n' << 24) | ((uint32_t)'a' << 16) | + ((uint32_t)'m' << 8) | (uint32_t)'e'; +constexpr uint8_t kSubtype = 0x1e; +constexpr uint32_t kSenderSsrc = 0x12345678; +constexpr uint8_t kData[] = {'t', 'e', 's', 't', 'd', 'a', 't', 'a'}; +constexpr uint8_t kVersionBits = 2 << 6; +constexpr uint8_t kPaddingBit = 1 << 5; +// clang-format off +constexpr uint8_t kPacketWithoutData[] = { + kVersionBits | kSubtype, App::kPacketType, 0x00, 0x02, + 0x12, 0x34, 0x56, 0x78, + 'n', 'a', 'm', 'e'}; +constexpr uint8_t kPacketWithData[] = { + kVersionBits | kSubtype, App::kPacketType, 0x00, 0x04, + 0x12, 0x34, 0x56, 0x78, + 'n', 'a', 'm', 'e', + 't', 'e', 's', 't', + 'd', 'a', 't', 'a'}; +constexpr uint8_t kTooSmallPacket[] = { + kVersionBits | kSubtype, App::kPacketType, 0x00, 0x01, + 0x12, 0x34, 0x56, 0x78}; +constexpr uint8_t kPaddingSize = 1; +constexpr uint8_t kPacketWithUnalignedPayload[] = { + kVersionBits | kPaddingBit | kSubtype, App::kPacketType, 0x00, 0x03, + 0x12, 0x34, 0x56, 0x78, + 'n', 'a', 'm', 'e', + 'd', 'a', 't', kPaddingSize}; +// clang-format on } // namespace + +TEST(RtcpPacketAppTest, CreateWithoutData) { + App app; + app.From(kSenderSsrc); + app.WithSubType(kSubtype); + app.WithName(kName); + + rtc::Buffer raw = app.Build(); + + EXPECT_THAT(make_tuple(raw.data(), raw.size()), + ElementsAreArray(kPacketWithoutData)); +} + +TEST(RtcpPacketAppTest, ParseWithoutData) { + App parsed; + EXPECT_TRUE(test::ParseSinglePacket(kPacketWithoutData, &parsed)); + + EXPECT_EQ(kSenderSsrc, parsed.ssrc()); + EXPECT_EQ(kSubtype, parsed.sub_type()); + EXPECT_EQ(kName, parsed.name()); + EXPECT_EQ(0u, parsed.data_size()); +} + +TEST(RtcpPacketAppTest, CreateWithData) { + App app; + app.From(kSenderSsrc); + app.WithSubType(kSubtype); + app.WithName(kName); + app.WithData(kData, sizeof(kData)); + + rtc::Buffer raw = app.Build(); + + EXPECT_THAT(make_tuple(raw.data(), raw.size()), + ElementsAreArray(kPacketWithData)); +} + +TEST(RtcpPacketAppTest, ParseWithData) { + App parsed; + EXPECT_TRUE(test::ParseSinglePacket(kPacketWithData, &parsed)); + + EXPECT_EQ(kSenderSsrc, parsed.ssrc()); + EXPECT_EQ(kSubtype, parsed.sub_type()); + EXPECT_EQ(kName, parsed.name()); + EXPECT_THAT(make_tuple(parsed.data(), parsed.data_size()), + ElementsAreArray(kData)); +} + +TEST(RtcpPacketAppTest, ParseFailsOnTooSmallPacket) { + App parsed; + EXPECT_FALSE(test::ParseSinglePacket(kTooSmallPacket, &parsed)); +} + +TEST(RtcpPacketAppTest, ParseFailsOnUnalignedPayload) { + App parsed; + EXPECT_FALSE(test::ParseSinglePacket(kPacketWithUnalignedPayload, &parsed)); +} + } // namespace webrtc