commit 1c5808145e8b151800b0320b8a7316a09b706488
author Harald Alvestrand <hta@webrtc.org> Mon Jul 04 09:41:24 2022
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Jul 04 11:04:12 2022
tree a76e5f09aca92eb9c6bbbd4b5433f00c80da9939
parent 0fd2ed516bf2abfb7c646f3e57291f1f004d63d2

Ignore RID that appears without an a=simulcast entry

RID is defined for multiple usages in RFC 8851, but we only support
usage with a=simulcast as specified in RFC 8853.

Bug: chromium:1341043
Change-Id: Ie72074c5b394bdc41865938a86ec9c7629e1f5e0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267628
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37417}

pc/peer_connection_simulcast_unittest.cc

diff --git a/pc/peer_connection_simulcast_unittest.cc b/pc/peer_connection_simulcast_unittest.cc
index e704aeb..b9681dc 100644
--- a/pc/peer_connection_simulcast_unittest.cc
+++ b/pc/peer_connection_simulcast_unittest.cc
@@ -602,6 +602,27 @@
               ElementsAre(Field("rid", &RtpEncodingParameters::rid, Eq(""))));
 }
 
+// Check that modifying the offer to remove simulcast and at the same
+// time leaving in a RID line does not cause an exception.
+TEST_F(PeerConnectionSimulcastTests, SimulcastSldModificationRejected) {
+  auto local = CreatePeerConnectionWrapper();
+  auto remote = CreatePeerConnectionWrapper();
+  auto layers = CreateLayers({"1", "2", "3"}, true);
+  AddTransceiver(local.get(), layers);
+  auto offer = local->CreateOffer();
+  std::string as_string;
+  EXPECT_TRUE(offer->ToString(&as_string));
+  auto simulcast_marker = "a=rid:3 send\r\na=simulcast:send 1;2;3\r\n";
+  auto pos = as_string.find(simulcast_marker);
+  EXPECT_NE(pos, std::string::npos);
+  as_string.erase(pos, strlen(simulcast_marker));
+  SdpParseError parse_error;
+  auto modified_offer =
+      CreateSessionDescription(SdpType::kOffer, as_string, &parse_error);
+  EXPECT_TRUE(modified_offer);
+  EXPECT_TRUE(local->SetLocalDescription(std::move(modified_offer)));
+}
+
 #if RTC_METRICS_ENABLED
 //
 // Checks the logged metrics when simulcast is not used.

pc/webrtc_sdp.cc

diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index 8e45524..c942689 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -3282,7 +3282,6 @@
 
   // If simulcast is specifed, split the rids into send and receive.
   // Rids that do not appear in simulcast attribute will be removed.
-  // If it is not specified, we assume that all rids are for send layers.
   std::vector<RidDescription> send_rids;
   std::vector<RidDescription> receive_rids;
   if (!simulcast.empty()) {
@@ -3309,7 +3308,11 @@
 
     media_desc->set_simulcast_description(simulcast);
   } else {
-    send_rids = rids;
+    // RID is specified in RFC 8851, which identifies a lot of usages.
+    // We only support RFC 8853 usage of RID, not anything else.
+    // Ignore all RID parameters when a=simulcast is missing.
+    // In particular do NOT do send_rids = rids;
+    RTC_LOG(LS_VERBOSE) << "Ignoring send_rids without simulcast";
   }
 
   media_desc->set_receive_rids(receive_rids);