Reject attempts to change the media kind for a m-line with a previously used mid
which can happen if the remote end reuses a mid.
BUG=webrtc:15471
Change-Id: I38da7dced712400002bc61d616e481a1255aa896
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/319460
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40776}
diff --git a/pc/media_session.cc b/pc/media_session.cc
index a2ea39f..53c422f 100644
--- a/pc/media_session.cc
+++ b/pc/media_session.cc
@@ -2341,7 +2341,14 @@
// recycled.
if (current_content && !current_content->rejected &&
current_content->name == media_description_options.mid) {
- RTC_CHECK(IsMediaContentOfType(current_content, MEDIA_TYPE_AUDIO));
+ if (!IsMediaContentOfType(current_content, MEDIA_TYPE_AUDIO)) {
+ // TODO(bugs.webrtc.org/15471): add a unit test for this since
+ // it is not clear how this can happen for offers.
+ RTC_LOG(LS_ERROR) << "Media type for content with mid='"
+ << current_content->name
+ << "' does not match previous type.";
+ return false;
+ }
const AudioContentDescription* acd =
current_content->media_description()->as_audio();
for (const AudioCodec& codec : acd->codecs()) {
@@ -2434,7 +2441,15 @@
// recycled.
if (current_content && !current_content->rejected &&
current_content->name == media_description_options.mid) {
- RTC_CHECK(IsMediaContentOfType(current_content, MEDIA_TYPE_VIDEO));
+ if (!IsMediaContentOfType(current_content, MEDIA_TYPE_VIDEO)) {
+ // TODO(bugs.webrtc.org/15471): add a unit test for this since
+ // it is not clear how this can happen for offers.
+ RTC_LOG(LS_ERROR) << "Media type for content with mid='"
+ << current_content->name
+ << "' does not match previous type.";
+ return false;
+ }
+
const VideoContentDescription* vcd =
current_content->media_description()->as_video();
for (const VideoCodec& codec : vcd->codecs()) {
@@ -2645,7 +2660,13 @@
// recycled.
if (current_content && !current_content->rejected &&
current_content->name == media_description_options.mid) {
- RTC_CHECK(IsMediaContentOfType(current_content, MEDIA_TYPE_AUDIO));
+ if (!IsMediaContentOfType(current_content, MEDIA_TYPE_AUDIO)) {
+ // Can happen if the remote side re-uses a MID while recycling.
+ RTC_LOG(LS_ERROR) << "Media type for content with mid='"
+ << current_content->name
+ << "' does not match previous type.";
+ return false;
+ }
const AudioContentDescription* acd =
current_content->media_description()->as_audio();
for (const AudioCodec& codec : acd->codecs()) {
@@ -2770,7 +2791,13 @@
// recycled.
if (current_content && !current_content->rejected &&
current_content->name == media_description_options.mid) {
- RTC_CHECK(IsMediaContentOfType(current_content, MEDIA_TYPE_VIDEO));
+ if (!IsMediaContentOfType(current_content, MEDIA_TYPE_VIDEO)) {
+ // Can happen if the remote side re-uses a MID while recycling.
+ RTC_LOG(LS_ERROR) << "Media type for content with mid='"
+ << current_content->name
+ << "' does not match previous type.";
+ return false;
+ }
const VideoContentDescription* vcd =
current_content->media_description()->as_video();
for (const VideoCodec& codec : vcd->codecs()) {
diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index 2537a41..448f85a 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc
@@ -1006,4 +1006,55 @@
}
}
+// Test variant with boolean order for audio-video and video-audio.
+class SdpOfferAnswerShuffleMediaTypes
+ : public SdpOfferAnswerTest,
+ public testing::WithParamInterface<bool> {
+ public:
+ SdpOfferAnswerShuffleMediaTypes() : SdpOfferAnswerTest() {}
+};
+
+TEST_P(SdpOfferAnswerShuffleMediaTypes,
+ RecyclingWithDifferentKindAndSameMidFails) {
+ bool audio_first = GetParam();
+ auto pc1 = CreatePeerConnection();
+ auto pc2 = CreatePeerConnection();
+ if (audio_first) {
+ pc1->AddAudioTrack("audio_track", {});
+ pc2->AddVideoTrack("video_track", {});
+ } else {
+ pc2->AddAudioTrack("audio_track", {});
+ pc1->AddVideoTrack("video_track", {});
+ }
+
+ auto initial_offer = pc1->CreateOfferAndSetAsLocal();
+ ASSERT_EQ(initial_offer->description()->contents().size(), 1u);
+ auto mid1 = initial_offer->description()->contents()[0].mid();
+ std::string rejected_answer_sdp =
+ "v=0\r\n"
+ "o=- 8621259572628890423 2 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "m=" +
+ std::string(audio_first ? "audio" : "video") +
+ " 0 UDP/TLS/RTP/SAVPF 111\r\n"
+ "c=IN IP4 0.0.0.0\r\n";
+ auto rejected_answer =
+ CreateSessionDescription(SdpType::kAnswer, rejected_answer_sdp);
+ EXPECT_TRUE(pc1->SetRemoteDescription(std::move(rejected_answer)));
+
+ auto offer =
+ pc2->CreateOfferAndSetAsLocal(); // This will generate a mid=0 too
+ ASSERT_EQ(offer->description()->contents().size(), 1u);
+ auto mid2 = offer->description()->contents()[0].mid();
+ EXPECT_EQ(mid1, mid2); // Check that the mids collided.
+ EXPECT_TRUE(pc1->SetRemoteDescription(std::move(offer)));
+ auto answer = pc1->CreateAnswer();
+ EXPECT_FALSE(pc1->SetLocalDescription(std::move(answer)));
+}
+
+INSTANTIATE_TEST_SUITE_P(SdpOfferAnswerShuffleMediaTypes,
+ SdpOfferAnswerShuffleMediaTypes,
+ ::testing::Values(true, false));
+
} // namespace webrtc
