From 211856b956d8099ef99b9dbe16ce77dcf78d0bb4 Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Thu, 6 Sep 2018 12:12:28 +0200 Subject: [PATCH] Make HasAttribute handle partial matching of attribute names. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improve HasAttribute to handle the case where the beginning of an attribute name is also an attribute name in it self. Two attributes that have this relation are extmap-allow-mixed and extmap. Bug: webrtc:9712 Change-Id: Iee660cc6e3dc7f2e7c56664a4f0ffb298eca9208 Reviewed-on: https://webrtc-review.googlesource.com/97422 Commit-Queue: Johannes Kron Reviewed-by: Erik Språng Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#24640} --- pc/webrtcsdp.cc | 14 +++++++++++++- pc/webrtcsdp_unittest.cc | 29 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/pc/webrtcsdp.cc b/pc/webrtcsdp.cc index e392857a8a..665b4d411a 100644 --- a/pc/webrtcsdp.cc +++ b/pc/webrtcsdp.cc @@ -541,7 +541,19 @@ static bool GetLineWithType(const std::string& message, static bool HasAttribute(const std::string& line, const std::string& attribute) { - return (line.compare(kLinePrefixLength, attribute.size(), attribute) == 0); + if (line.compare(kLinePrefixLength, attribute.size(), attribute) == 0) { + // Make sure that the match is not only a partial match. If length of + // strings doesn't match, the next character of the line must be ':' or ' '. + // This function is also used for media descriptions (e.g., "m=audio 9..."), + // hence the need to also allow space in the end. + RTC_CHECK_LE(kLinePrefixLength + attribute.size(), line.size()); + if ((kLinePrefixLength + attribute.size()) == line.size() || + line[kLinePrefixLength + attribute.size()] == kSdpDelimiterColonChar || + line[kLinePrefixLength + attribute.size()] == kSdpDelimiterSpaceChar) { + return true; + } + } + return false; } static bool AddSsrcLine(uint32_t ssrc_id, diff --git a/pc/webrtcsdp_unittest.cc b/pc/webrtcsdp_unittest.cc index bfae1b9a12..9c10554631 100644 --- a/pc/webrtcsdp_unittest.cc +++ b/pc/webrtcsdp_unittest.cc @@ -3566,6 +3566,35 @@ TEST_F(WebRtcSdpTest, IceCredentialsInCandidateStringIgnored) { EXPECT_EQ("pwd_voice", c.password()); } +// Test that attribute lines "a=ice-ufrag-something"/"a=ice-pwd-something" are +// ignored, and only the "a=ice-ufrag"/"a=ice-pwd" attributes are used. +// Regression test for: +// https://bugs.chromium.org/p/webrtc/issues/detail?id=9712 +TEST_F(WebRtcSdpTest, AttributeWithPartialMatchingNameIsIgnored) { + static const char kSdpWithFooIceCredentials[] = + "v=0\r\n" + "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "m=audio 9 RTP/SAVPF 111\r\n" + "c=IN IP4 0.0.0.0\r\n" + "a=rtcp:9 IN IP4 0.0.0.0\r\n" + "a=ice-ufrag-something:foo\r\na=ice-pwd-something:bar\r\n" + "a=ice-ufrag:ufrag_voice\r\na=ice-pwd:pwd_voice\r\n" + "a=rtpmap:111 opus/48000/2\r\n" + "a=candidate:a0+B/1 1 udp 2130706432 192.168.1.5 1234 typ host " + "generation 2\r\n"; + + JsepSessionDescription jdesc_output(kDummyType); + EXPECT_TRUE(SdpDeserialize(kSdpWithFooIceCredentials, &jdesc_output)); + const IceCandidateCollection* candidates = jdesc_output.candidates(0); + ASSERT_NE(nullptr, candidates); + ASSERT_EQ(1U, candidates->count()); + cricket::Candidate c = candidates->at(0)->candidate(); + EXPECT_EQ("ufrag_voice", c.username()); + EXPECT_EQ("pwd_voice", c.password()); +} + // Test that SDP with an invalid port number in "a=candidate" lines is // rejected, without crashing. // Regression test for: