Reject the subsequent offer with fewer m= sections.

If the subsequent offer contains fewer m= sections than the existing
description, it would be rejected.

The helper method MediaSectionsInSameOrder is modified and it will
compare the number of m= sections before matching the media type.

Bug: chromium:773620
Change-Id: Ic8999445f4bc023da1d85a65659583db1687ec37
Reviewed-on: https://webrtc-review.googlesource.com/9621
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Zhi Huang <zhihuang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20298}
This commit is contained in:
Zhi Huang 2017-10-13 16:54:34 -07:00 committed by Commit Bot
parent 569f4e7e58
commit a8264dbdd9
2 changed files with 61 additions and 20 deletions

View File

@ -3740,7 +3740,7 @@ TEST_F(PeerConnectionInterfaceTest, CreateAnswerWithoutRemoteDescription) {
EXPECT_TRUE(DoCreateAnswer(&answer, nullptr));
}
// Test that an error is returned if a description is applied that doesn't
// Tests that an error is returned if a description is applied that doesn't
// respect the order of existing media sections.
TEST_F(PeerConnectionInterfaceTest,
MediaSectionOrderEnforcedForSubsequentOffers) {
@ -3773,6 +3773,37 @@ TEST_F(PeerConnectionInterfaceTest,
EXPECT_FALSE(DoSetLocalDescription(std::move(offer)));
}
// Tests that an error is returned if a description is applied that has fewer
// media sections than the existing description.
TEST_F(PeerConnectionInterfaceTest,
MediaSectionCountEnforcedForSubsequentOffer) {
CreatePeerConnection();
FakeConstraints constraints;
constraints.SetMandatoryReceiveAudio(true);
constraints.SetMandatoryReceiveVideo(true);
std::unique_ptr<SessionDescriptionInterface> offer;
ASSERT_TRUE(DoCreateOffer(&offer, &constraints));
EXPECT_TRUE(DoSetRemoteDescription(std::move(offer)));
// A remote offer with fewer media sections should be rejected.
ASSERT_TRUE(DoCreateOffer(&offer, &constraints));
offer->description()->contents().pop_back();
offer->description()->contents().pop_back();
ASSERT_TRUE(offer->description()->contents().empty());
EXPECT_FALSE(DoSetRemoteDescription(std::move(offer)));
std::unique_ptr<SessionDescriptionInterface> answer;
ASSERT_TRUE(DoCreateAnswer(&answer, nullptr));
EXPECT_TRUE(DoSetLocalDescription(std::move(answer)));
// A subsequent local offer with fewer media sections should be rejected.
ASSERT_TRUE(DoCreateOffer(&offer, &constraints));
offer->description()->contents().pop_back();
offer->description()->contents().pop_back();
ASSERT_TRUE(offer->description()->contents().empty());
EXPECT_FALSE(DoSetLocalDescription(std::move(offer)));
}
class PeerConnectionMediaConfigTest : public testing::Test {
protected:
void SetUp() override {

View File

@ -140,25 +140,30 @@ IceCandidatePairType GetIceCandidatePairCounter(
return kIceCandidatePairMax;
}
// Verify that the order of media sections in |desc1| matches |desc2|. The
// number of m= sections could be different.
static bool MediaSectionsInSameOrder(const SessionDescription* desc1,
const SessionDescription* desc2) {
if (!desc1 || !desc2) {
// Verify that the order of media sections in |new_desc| matches
// |existing_desc|. The number of m= sections in |new_desc| should be no less
// than |existing_desc|.
static bool MediaSectionsInSameOrder(const SessionDescription* existing_desc,
const SessionDescription* new_desc) {
if (!existing_desc || !new_desc) {
return false;
}
for (size_t i = 0;
i < desc1->contents().size() && i < desc2->contents().size(); ++i) {
if ((desc2->contents()[i].name) != desc1->contents()[i].name) {
if (existing_desc->contents().size() > new_desc->contents().size()) {
return false;
}
for (size_t i = 0; i < existing_desc->contents().size(); ++i) {
if (new_desc->contents()[i].name != existing_desc->contents()[i].name) {
return false;
}
const MediaContentDescription* desc2_mdesc =
const MediaContentDescription* new_desc_mdesc =
static_cast<const MediaContentDescription*>(
desc2->contents()[i].description);
const MediaContentDescription* desc1_mdesc =
new_desc->contents()[i].description);
const MediaContentDescription* existing_desc_mdesc =
static_cast<const MediaContentDescription*>(
desc1->contents()[i].description);
if (desc2_mdesc->type() != desc1_mdesc->type()) {
existing_desc->contents()[i].description);
if (new_desc_mdesc->type() != existing_desc_mdesc->type()) {
return false;
}
}
@ -2165,16 +2170,21 @@ bool WebRtcSession::ValidateSessionDescription(
const cricket::SessionDescription* offer_desc =
(source == cricket::CS_LOCAL) ? remote_description()->description()
: local_description()->description();
if (!MediaSectionsHaveSameCount(sdesc->description(), offer_desc) ||
!MediaSectionsInSameOrder(sdesc->description(), offer_desc)) {
if (!MediaSectionsHaveSameCount(offer_desc, sdesc->description()) ||
!MediaSectionsInSameOrder(offer_desc, sdesc->description())) {
return BadAnswerSdp(source, kMlineMismatchInAnswer, err_desc);
}
} else {
// The re-offers should respect the order of m= sections in current local
const cricket::SessionDescription* current_desc = nullptr;
if (source == cricket::CS_LOCAL && local_description()) {
current_desc = local_description()->description();
} else if (source == cricket::CS_REMOTE && remote_description()) {
current_desc = remote_description()->description();
}
// The re-offers should respect the order of m= sections in current
// description. See RFC3264 Section 8 paragraph 4 for more details.
if (local_description() &&
!MediaSectionsInSameOrder(sdesc->description(),
local_description()->description())) {
if (current_desc &&
!MediaSectionsInSameOrder(current_desc, sdesc->description())) {
return BadOfferSdp(source, kMlineMismatchInSubsequentOffer, err_desc);
}
}