Fix bug where rtcp::TransportFeedback may generate incorrect messages.

webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc

Index: webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
index e09c29d72c39a6c59114132435ed4543ed66ac00..a7a14a6f86b5bb76611b01dabdd9f0e1356cfbb6 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
@@ -19,11 +19,13 @@
 #include "webrtc/common_types.h"
 #include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_observer.h"
 #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h"
+#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_receiver.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_sender.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h"
+#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 
 namespace webrtc {
 
@@ -120,9 +122,10 @@ class RtcpReceiverTest : public ::testing::Test {
     rtcp_packet_info_.ntp_frac = rtcpPacketInformation.ntp_frac;
     rtcp_packet_info_.rtp_timestamp = rtcpPacketInformation.rtp_timestamp;
     rtcp_packet_info_.xr_dlrr_item = rtcpPacketInformation.xr_dlrr_item;
-    if (rtcpPacketInformation.VoIPMetric) {
+    if (rtcpPacketInformation.VoIPMetric)
       rtcp_packet_info_.AddVoIPMetric(rtcpPacketInformation.VoIPMetric);
-    }
+    rtcp_packet_info_.transport_feedback_.reset(
+        rtcpPacketInformation.transport_feedback_.release());
     return 0;
   }
 
@@ -1028,6 +1031,69 @@ TEST_F(RtcpReceiverTest, Callbacks) {
                                kCumulativeLoss, kJitter));
 }
 
+TEST_F(RtcpReceiverTest, ReceivesTransportFeedback) {
+  const uint32_t kSenderSsrc = 0x10203;
+  const uint32_t kSourceSsrc = 0x123456;
+
+  std::set<uint32_t> ssrcs;
+  ssrcs.insert(kSourceSsrc);
+  rtcp_receiver_->SetSsrcs(kSourceSsrc, ssrcs);
+
+  // Send a compound packet with a TransportFeedback followed by something else.

[stefan-webrtc, 2015/09/30 10:45:27]

Where's the "followed by something else" part? I only see a transport feedback
being sent here?

[sprang_webrtc, 2015/09/30 11:09:27]

Copy/paste mistake. Removed comment.

+  rtcp::TransportFeedback packet;
+  packet.WithMediaSourceSsrc(kSourceSsrc);
+  packet.WithPacketSenderSsrc(kSenderSsrc);
+  packet.WithBase(1, 1000);
+  packet.WithReceivedPacket(1, 1000);
+
+  rtc::scoped_ptr<rtcp::RawPacket> built_packet = packet.Build();
+  ASSERT_TRUE(built_packet.get() != nullptr);
+
+  EXPECT_EQ(0,
+            InjectRtcpPacket(built_packet->Buffer(), built_packet->Length()));
+
+  EXPECT_NE(0u, rtcp_packet_info_.rtcpPacketTypeFlags & kRtcpTransportFeedback);
+  EXPECT_TRUE(rtcp_packet_info_.transport_feedback_.get() != nullptr);
+}
+
+TEST_F(RtcpReceiverTest, HandlesInvalidTransportFeedback) {
+  const uint32_t kSenderSsrc = 0x10203;
+  const uint32_t kSourceSsrc = 0x123456;
+
+  std::set<uint32_t> ssrcs;
+  ssrcs.insert(kSourceSsrc);
+  rtcp_receiver_->SetSsrcs(kSourceSsrc, ssrcs);
+
+  // Send a compound packet with a TransportFeedback followed by something else.
+  rtcp::TransportFeedback packet;
+  packet.WithMediaSourceSsrc(kSourceSsrc);
+  packet.WithPacketSenderSsrc(kSenderSsrc);
+  packet.WithBase(1, 1000);
+  packet.WithReceivedPacket(1, 1000);
+
+  static uint32_t kBitrateBps = 50000;
+  rtcp::Remb remb;
+  remb.From(kSourceSsrc);
+  remb.WithBitrateBps(kBitrateBps);
+  packet.Append(&remb);
+
+  rtc::scoped_ptr<rtcp::RawPacket> built_packet = packet.Build();
+  ASSERT_TRUE(built_packet.get() != nullptr);
+
+  // Modify the TransportFeedback packet so that it is invalid.
+  const size_t kStatusCountOffset = 14;
+  ByteWriter<uint16_t>::WriteBigEndian(
+      &built_packet->MutableBuffer()[kStatusCountOffset], 42);

[stefan-webrtc, 2015/09/30 10:45:27]

Sounds like we may want to fuzz this code. Maybe ask pbos if he could help out?

[sprang_webrtc, 2015/09/30 11:09:27]

Acknowledged.

+
+  EXPECT_EQ(0,
+            InjectRtcpPacket(built_packet->Buffer(), built_packet->Length()));
+
+  // Transport feedback should be ignored, but next packet should work.
+  EXPECT_EQ(0u, rtcp_packet_info_.rtcpPacketTypeFlags & kRtcpTransportFeedback);
+  EXPECT_NE(0u, rtcp_packet_info_.rtcpPacketTypeFlags & kRtcpRemb);
+  EXPECT_EQ(kBitrateBps, rtcp_packet_info_.receiverEstimatedMaxBitrate);
+}
+
 }  // Anonymous namespace
 
 }  // namespace webrtc