diff --git a/pc/media_session.cc b/pc/media_session.cc index a2ea39f890..53c422f412 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -2341,7 +2341,14 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer( // 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 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( // 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 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( // 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 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( // 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 2537a414b6..448f85a3e2 100644 --- a/pc/sdp_offer_answer_unittest.cc +++ b/pc/sdp_offer_answer_unittest.cc @@ -1006,4 +1006,55 @@ TEST_F(SdpOfferAnswerTest, SdpMungingWithInvalidPayloadTypeIsRejected) { } } +// Test variant with boolean order for audio-video and video-audio. +class SdpOfferAnswerShuffleMediaTypes + : public SdpOfferAnswerTest, + public testing::WithParamInterface { + 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