commit 465bc0fd8707935ee3a96d9a801b9996d47c16c7
author Philipp Hancke <phancke@microsoft.com> Fri Aug 25 10:42:36 2023
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Aug 29 09:42:11 2023
tree 98db1cd5193c47665f68791f2adc759717d327b7
parent 2ea5f15a5abf5483fcf5f05adb1f8494648af6e5

Validate rejected m-lines less strictly

since their content typically is not processed further.

BUG=webrtc:142258

Change-Id: I5bcfb6c3a6f3a301acb497b83f8a4dbc3023c5db
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/317603
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#40649}

pc/sdp_offer_answer.cc

diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 2ae2602..7d7edd2 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -417,14 +417,18 @@
   for (const cricket::ContentGroup* bundle_group : bundle_groups) {
     std::map<int, RtpCodecParameters> payload_to_codec_parameters;
     for (const std::string& content_name : bundle_group->content_names()) {
-      const cricket::MediaContentDescription* media_description =
-          description.GetContentDescriptionByName(content_name);
-      if (!media_description) {
+      const ContentInfo* content_description =
+          description.GetContentByName(content_name);
+      if (!content_description) {
         LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
                              "A BUNDLE group contains a MID='" + content_name +
                                  "' matching no m= section.");
       }
-      if (!media_description->has_codecs()) {
+      const cricket::MediaContentDescription* media_description =
+          content_description->media_description();
+      RTC_DCHECK(media_description);
+      if (content_description->rejected || !media_description ||
+          !media_description->has_codecs()) {
         continue;
       }
       const auto type = media_description->type();
@@ -480,13 +484,21 @@
   for (const cricket::ContentGroup* bundle_group : bundle_groups) {
     std::map<int, RtpExtension> id_to_extension;
     for (const std::string& content_name : bundle_group->content_names()) {
-      const cricket::MediaContentDescription* media_description =
-          description.GetContentDescriptionByName(content_name);
-      if (!media_description) {
+      const ContentInfo* content_description =
+          description.GetContentByName(content_name);
+      if (!content_description) {
         LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
                              "A BUNDLE group contains a MID='" + content_name +
                                  "' matching no m= section.");
       }
+      const cricket::MediaContentDescription* media_description =
+          content_description->media_description();
+      RTC_DCHECK(media_description);
+      if (content_description->rejected || !media_description ||
+          !media_description->has_codecs()) {
+        continue;
+      }
+
       for (const auto& extension : media_description->rtp_header_extensions()) {
         auto error =
             FindDuplicateHeaderExtensionIds(extension, id_to_extension);
@@ -502,7 +514,7 @@
 RTCError ValidateRtpHeaderExtensionsForSpecSimulcast(
     const cricket::SessionDescription& description) {
   for (const ContentInfo& content : description.contents()) {
-    if (content.type != MediaProtocolType::kRtp) {
+    if (content.type != MediaProtocolType::kRtp || content.rejected) {
       continue;
     }
     const auto media_description = content.media_description();

pc/sdp_offer_answer_unittest.cc

diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index 3e250c2..5cbf79b 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc
@@ -165,10 +165,22 @@
   ASSERT_NE(desc, nullptr);
   RTCError error;
   pc->SetRemoteDescription(std::move(desc), &error);
+  // There is no error yet but the metrics counter will increase.
   EXPECT_TRUE(error.ok());
   EXPECT_METRIC_EQ(
       1, webrtc::metrics::NumEvents(
              "WebRTC.PeerConnection.ValidBundledPayloadTypes", false));
+
+  // Tolerate codec collisions in rejected m-lines.
+  pc = CreatePeerConnection();
+  auto rejected_offer = CreateSessionDescription(
+      SdpType::kOffer,
+      absl::StrReplaceAll(sdp, {{"m=video 9 ", "m=video 0 "}}));
+  pc->SetRemoteDescription(std::move(rejected_offer), &error);
+  EXPECT_TRUE(error.ok());
+  EXPECT_METRIC_EQ(1,
+                   webrtc::metrics::NumEvents(
+                       "WebRTC.PeerConnection.ValidBundledPayloadTypes", true));
 }
 
 TEST_F(SdpOfferAnswerTest, BundleRejectsCodecCollisionsVideoFmtp) {
@@ -598,6 +610,13 @@
   auto answer_with_extensions =
       CreateSessionDescription(SdpType::kAnswer, sdp + extensions);
   EXPECT_TRUE(pc->SetRemoteDescription(std::move(answer_with_extensions)));
+
+  // Tolerate the lack of mid/rid extensions in rejected m-lines.
+  EXPECT_TRUE(pc->CreateOfferAndSetAsLocal());
+  auto rejected_answer = CreateSessionDescription(
+      SdpType::kAnswer,
+      absl::StrReplaceAll(sdp, {{"m=video 9 ", "m=video 0 "}}));
+  EXPECT_TRUE(pc->SetRemoteDescription(std::move(rejected_answer)));
 }
 
 TEST_F(SdpOfferAnswerTest, ExpectAllSsrcsSpecifiedInSsrcGroupFid) {