zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reject the descriptions that attempt to change the order of m= sections

Browse Source 
in current local description.

When setting the descriptions, the order of m= sections would be compared
against existing m= sections and an error would be returned if the order
doesn't match.

Previously reviewed on: https://codereview.webrtc.org/3012313002/

BUG=chromium:763842
TBR=deadbeef@webrtc.org

Change-Id: I577e3424830b0a4c5ecd5524923873e30ad23d43
Reviewed-on: https://webrtc-review.googlesource.com/1200
Commit-Queue: Zhi Huang <zhihuang@webrtc.org>
Reviewed-by: Zhi Huang <zhihuang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19842}
This commit is contained in:
Zhi Huang
2017-09-14 01:15:03 -07:00
committed by Commit Bot
parent 9c66aee407
commit 2a5e4268f8
4 changed files with 84 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
webrtc/pc/peerconnectioninterface_unittest.cc
View File

@ -8,6 +8,7 @@
* be found in the AUTHORS file in the root of the source tree.
*/
#include <algorithm>
#include <memory>
#include <sstream>
#include <string>
@ -3831,6 +3832,39 @@ TEST_F(PeerConnectionInterfaceTest, CreateAnswerWithoutRemoteDescription) {
EXPECT_TRUE(DoCreateAnswer(&answer, nullptr));
}
// Test that an error is returned if a description is applied that doesn't
// respect the order of existing media sections.
TEST_F(PeerConnectionInterfaceTest,
MediaSectionOrderEnforcedForSubsequentOffers) {
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)));
std::unique_ptr<SessionDescriptionInterface> answer;
ASSERT_TRUE(DoCreateAnswer(&answer, nullptr));
EXPECT_TRUE(DoSetLocalDescription(std::move(answer)));
// A remote offer with different m=line order should be rejected.
ASSERT_TRUE(DoCreateOffer(&offer, &constraints));
std::reverse(offer->description()->contents().begin(),
offer->description()->contents().end());
std::reverse(offer->description()->transport_infos().begin(),
offer->description()->transport_infos().end());
EXPECT_FALSE(DoSetRemoteDescription(std::move(offer)));
// A subsequent local offer with different m=line order should be rejected.
ASSERT_TRUE(DoCreateOffer(&offer, &constraints));
std::reverse(offer->description()->contents().begin(),
offer->description()->contents().end());
std::reverse(offer->description()->transport_infos().begin(),
offer->description()->transport_infos().end());
EXPECT_FALSE(DoSetLocalDescription(std::move(offer)));
}
class PeerConnectionMediaConfigTest : public testing::Test {
protected:
void SetUp() override {

59
webrtc/pc/webrtcsession.cc
View File

@ -61,8 +61,12 @@ const char kBundleWithoutRtcpMux[] = "RTCP-MUX must be enabled when BUNDLE "
const char kCreateChannelFailed[] = "Failed to create channels.";
const char kInvalidCandidates[] = "Description contains invalid candidates.";
const char kInvalidSdp[] = "Invalid session description.";
const char kMlineMismatch[] =
"Offer and answer descriptions m-lines are not matching. Rejecting answer.";
const char kMlineMismatchInAnswer[] =
"The order of m-lines in answer doesn't match order in offer. Rejecting "
"answer.";
const char kMlineMismatchInSubsequentOffer[] =
"The order of m-lines in subsequent offer doesn't match order from "
"previous offer/answer.";
const char kPushDownTDFailed[] =
"Failed to push down transport description:";
const char kSdpWithoutDtlsFingerprint[] =
@ -136,31 +140,39 @@ IceCandidatePairType GetIceCandidatePairCounter(
return kIceCandidatePairMax;
}
// Compares |answer| against |offer|. Comparision is done
// for number of m-lines in answer against offer. If matches true will be
// returned otherwise false.
static bool VerifyMediaDescriptions(
const SessionDescription* answer, const SessionDescription* offer) {
if (offer->contents().size() != answer->contents().size())
// 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) {
return false;
for (size_t i = 0; i < offer->contents().size(); ++i) {
if ((offer->contents()[i].name) != answer->contents()[i].name) {
}
for (size_t i = 0;
i < desc1->contents().size() && i < desc2->contents().size(); ++i) {
if ((desc2->contents()[i].name) != desc1->contents()[i].name) {
return false;
}
const MediaContentDescription* offer_mdesc =
const MediaContentDescription* desc2_mdesc =
static_cast<const MediaContentDescription*>(
offer->contents()[i].description);
const MediaContentDescription* answer_mdesc =
desc2->contents()[i].description);
const MediaContentDescription* desc1_mdesc =
static_cast<const MediaContentDescription*>(
answer->contents()[i].description);
if (offer_mdesc->type() != answer_mdesc->type()) {
desc1->contents()[i].description);
if (desc2_mdesc->type() != desc1_mdesc->type()) {
return false;
}
}
return true;
}
static bool MediaSectionsHaveSameCount(const SessionDescription* desc1,
const SessionDescription* desc2) {
if (!desc1 || !desc2) {
return false;
}
return desc1->contents().size() == desc2->contents().size();
}
// Checks that each non-rejected content has SDES crypto keys or a DTLS
// fingerprint, unless it's in a BUNDLE group, in which case only the
// BUNDLE-tag section (first media section/description in the BUNDLE group)
@ -2153,12 +2165,21 @@ bool WebRtcSession::ValidateSessionDescription(
// m-lines that do not rtcp-mux enabled.
// Verify m-lines in Answer when compared against Offer.
if (action == kAnswer) {
if (action == kAnswer || action == kPrAnswer) {
const cricket::SessionDescription* offer_desc =
(source == cricket::CS_LOCAL) ? remote_description()->description()
: local_description()->description();
if (!VerifyMediaDescriptions(sdesc->description(), offer_desc)) {
return BadAnswerSdp(source, kMlineMismatch, err_desc);
if (!MediaSectionsHaveSameCount(sdesc->description(), offer_desc) ||
!MediaSectionsInSameOrder(sdesc->description(), offer_desc)) {
return BadAnswerSdp(source, kMlineMismatchInAnswer, err_desc);
}
} else {
// The re-offers should respect the order of m= sections in current local
// description. See RFC3264 Section 8 paragraph 4 for more details.
if (local_description() &&
!MediaSectionsInSameOrder(sdesc->description(),
local_description()->description())) {
return BadOfferSdp(source, kMlineMismatchInSubsequentOffer, err_desc);
}
}

3
webrtc/pc/webrtcsession.h
View File

@ -61,7 +61,8 @@ extern const char kBundleWithoutRtcpMux[];
extern const char kCreateChannelFailed[];
extern const char kInvalidCandidates[];
extern const char kInvalidSdp[];
extern const char kMlineMismatch[];
extern const char kMlineMismatchInAnswer[];
extern const char kMlineMismatchInSubsequentOffer[];
extern const char kPushDownTDFailed[];
extern const char kSdpWithoutDtlsFingerprint[];
extern const char kSdpWithoutSdesCrypto[];

13
webrtc/pc/webrtcsession_unittest.cc
View File

@ -70,7 +70,7 @@ using webrtc::WebRtcSession;
using webrtc::kBundleWithoutRtcpMux;
using webrtc::kCreateChannelFailed;
using webrtc::kInvalidSdp;
using webrtc::kMlineMismatch;
using webrtc::kMlineMismatchInAnswer;
using webrtc::kPushDownTDFailed;
using webrtc::kSdpWithoutIceUfragPwd;
using webrtc::kSdpWithoutDtlsFingerprint;
@ -3747,7 +3747,8 @@ TEST_F(WebRtcSessionTest, TestIncorrectMLinesInRemoteAnswer) {
EXPECT_TRUE(modified_answer->Initialize(answer_copy,
answer->session_id(),
answer->session_version()));
SetRemoteDescriptionAnswerExpectError(kMlineMismatch, modified_answer);
SetRemoteDescriptionAnswerExpectError(kMlineMismatchInAnswer,
modified_answer);
// Different content names.
std::string sdp;
@ -3760,7 +3761,8 @@ TEST_F(WebRtcSessionTest, TestIncorrectMLinesInRemoteAnswer) {
&sdp);
SessionDescriptionInterface* modified_answer1 =
CreateSessionDescription(JsepSessionDescription::kAnswer, sdp, NULL);
SetRemoteDescriptionAnswerExpectError(kMlineMismatch, modified_answer1);
SetRemoteDescriptionAnswerExpectError(kMlineMismatchInAnswer,
modified_answer1);
// Different media types.
EXPECT_TRUE(answer->ToString(&sdp));
@ -3772,7 +3774,8 @@ TEST_F(WebRtcSessionTest, TestIncorrectMLinesInRemoteAnswer) {
&sdp);
SessionDescriptionInterface* modified_answer2 =
CreateSessionDescription(JsepSessionDescription::kAnswer, sdp, NULL);
SetRemoteDescriptionAnswerExpectError(kMlineMismatch, modified_answer2);
SetRemoteDescriptionAnswerExpectError(kMlineMismatchInAnswer,
modified_answer2);
SetRemoteDescriptionWithoutError(answer.release());
}
@ -3794,7 +3797,7 @@ TEST_F(WebRtcSessionTest, TestIncorrectMLinesInLocalAnswer) {
EXPECT_TRUE(modified_answer->Initialize(answer_copy,
answer->session_id(),
answer->session_version()));
SetLocalDescriptionAnswerExpectError(kMlineMismatch, modified_answer);
SetLocalDescriptionAnswerExpectError(kMlineMismatchInAnswer, modified_answer);
SetLocalDescriptionWithoutError(answer);
}