From 7ec3f9f838f83d17f7ac1a938f174152fc3767a7 Mon Sep 17 00:00:00 2001 From: "jiayl@webrtc.org" Date: Fri, 8 Aug 2014 23:09:15 +0000 Subject: [PATCH] Fix a bug in parsing IceCandidate with IPV6 address. It used to treat ":" as a candidate delimiter and got confused by the ":" in the IPV6 address. The new logic is to check if the input has multiple lines. If so, returns error. BUG=3669 R=pthatcher@webrtc.org Review URL: https://webrtc-codereview.appspot.com/18079004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6859 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/webrtcsdp.cc | 75 +++++++++++---------------- talk/app/webrtc/webrtcsdp_unittest.cc | 41 ++++++++++----- 2 files changed, 58 insertions(+), 58 deletions(-) diff --git a/talk/app/webrtc/webrtcsdp.cc b/talk/app/webrtc/webrtcsdp.cc index 64f8e5e2c..2eb5fb32b 100644 --- a/talk/app/webrtc/webrtcsdp.cc +++ b/talk/app/webrtc/webrtcsdp.cc @@ -478,16 +478,6 @@ static void AddAttributeLine(const std::string& attribute, int value, AddLine(os.str(), message); } -// Returns the first line of the message without the line breaker. -static bool GetFirstLine(const std::string& message, std::string* line) { - size_t pos = 0; - if (!GetLine(message, &pos, line)) { - // If GetLine failed, just return the full |message|. - *line = message; - } - return true; -} - static bool IsLineType(const std::string& message, const char type, size_t line_start) { @@ -521,21 +511,6 @@ static bool HasAttribute(const std::string& line, return (line.compare(kLinePrefixLength, attribute.size(), attribute) == 0); } -// Verifies the candiate to be of the format candidate: -static bool IsRawCandidate(const std::string& line) { - // Checking candiadte-attribute is starting with "candidate" str. - if (line.compare(0, strlen(kAttributeCandidate), kAttributeCandidate) != 0) { - return false; - } - const size_t first_candidate = line.find(kSdpDelimiterColon); - if (first_candidate == std::string::npos) - return false; - // In this format we only expecting one candiate. If any additional - // candidates present, whole string will be discared. - const size_t any_other = line.find(kSdpDelimiterColon, first_candidate + 1); - return (any_other == std::string::npos); -} - static bool AddSsrcLine(uint32 ssrc_id, const std::string& attribute, const std::string& value, std::string* message) { // RFC 5576 @@ -949,22 +924,36 @@ bool ParseCandidate(const std::string& message, Candidate* candidate, ASSERT(candidate != NULL); // Get the first line from |message|. - std::string first_line; - GetFirstLine(message, &first_line); + std::string first_line = message; + size_t pos = 0; + GetLine(message, &pos, &first_line); - size_t start_pos = kLinePrefixLength; // Starting position to parse. - if (IsRawCandidate(first_line)) { - // From WebRTC draft section 4.8.1.1 candidate-attribute will be - // just candidate: not a=candidate:CRLF - start_pos = 0; - } else if (!IsLineType(first_line, kLineTypeAttributes) || - !HasAttribute(first_line, kAttributeCandidate)) { - // Must start with a=candidate line. - // Expecting to be of the format a=candidate:CRLF. + // Makes sure |message| contains only one line. + if (message.size() > first_line.size()) { + std::string left, right; + if (SplitByDelimiter(message, kNewLine, &left, &right) && !right.empty()) { + return ParseFailed(message, 0, "Expect one line only", error); + } + } + + // From WebRTC draft section 4.8.1.1 candidate-attribute should be + // candidate: when trickled, but we still support + // a=candidate:CRLF for backward compatibility and for parsing a line + // from the SDP. + if (IsLineType(first_line, kLineTypeAttributes)) { + first_line = first_line.substr(kLinePrefixLength); + } + + std::string attribute_candidate; + std::string candidate_value; + + // |first_line| must be in the form of "candidate:". + if (!SplitByDelimiter(first_line, kSdpDelimiterColon, + &attribute_candidate, &candidate_value) || + attribute_candidate != kAttributeCandidate) { if (is_raw) { std::ostringstream description; - description << "Expect line: " - << kAttributeCandidate + description << "Expect line: " << kAttributeCandidate << ":" << ""; return ParseFailed(first_line, 0, description.str(), error); } else { @@ -974,8 +963,8 @@ bool ParseCandidate(const std::string& message, Candidate* candidate, } std::vector fields; - rtc::split(first_line.substr(start_pos), - kSdpDelimiterSpace, &fields); + rtc::split(candidate_value, kSdpDelimiterSpace, &fields); + // RFC 5245 // a=candidate: // typ @@ -986,10 +975,8 @@ bool ParseCandidate(const std::string& message, Candidate* candidate, (fields[6] != kAttributeCandidateTyp)) { return ParseFailedExpectMinFieldNum(first_line, expected_min_fields, error); } - std::string foundation; - if (!GetValue(fields[0], kAttributeCandidate, &foundation, error)) { - return false; - } + std::string foundation = fields[0]; + int component_id = 0; if (!GetValueFromString(first_line, fields[1], &component_id, error)) { return false; diff --git a/talk/app/webrtc/webrtcsdp_unittest.cc b/talk/app/webrtc/webrtcsdp_unittest.cc index f0546f80a..9f5e1eba6 100644 --- a/talk/app/webrtc/webrtcsdp_unittest.cc +++ b/talk/app/webrtc/webrtcsdp_unittest.cc @@ -354,6 +354,7 @@ static const char kRawCandidate[] = static const char kSdpOneCandidate[] = "a=candidate:a0+B/1 1 udp 2130706432 192.168.1.5 1234 typ host " "generation 2\r\n"; + static const char kSdpTcpActiveCandidate[] = "candidate:a0+B/1 1 tcp 2130706432 192.168.1.5 9 typ host " "tcptype active generation 2"; @@ -367,6 +368,10 @@ static const char kSdpTcpInvalidCandidate[] = "candidate:a0+B/1 1 tcp 2130706432 192.168.1.5 9 typ host " "tcptype invalid generation 2"; +// One candidate reference string with IPV6 address. +static const char kRawIPV6Candidate[] = + "candidate:a0+B/1 1 udp 2130706432 " + "abcd::abcd::abcd::abcd::abcd::abcd::abcd::abcd 1234 typ host generation 2"; // One candidate reference string. static const char kSdpOneCandidateOldFormat[] = @@ -1918,15 +1923,6 @@ TEST_F(WebRtcSdpTest, DeserializeCandidate) { expected.set_generation(0); EXPECT_TRUE(jcandidate.candidate().IsEquivalent(expected)); - // Multiple candidate lines. - // Only the first line will be deserialized. The rest will be ignored. - sdp = kSdpOneCandidate; - sdp.append("a=candidate:1 2 tcp 1234 192.168.1.100 5678 typ host\r\n"); - EXPECT_TRUE(SdpDeserializeCandidate(sdp, &jcandidate)); - EXPECT_EQ(kDummyMid, jcandidate.sdp_mid()); - EXPECT_EQ(kDummyIndex, jcandidate.sdp_mline_index()); - EXPECT_TRUE(jcandidate.candidate().IsEquivalent(jcandidate_->candidate())); - sdp = kSdpTcpActiveCandidate; EXPECT_TRUE(SdpDeserializeCandidate(sdp, &jcandidate)); // Make a cricket::Candidate equivalent to kSdpTcpCandidate string. @@ -1943,8 +1939,6 @@ TEST_F(WebRtcSdpTest, DeserializeCandidate) { EXPECT_TRUE(SdpDeserializeCandidate(sdp, &jcandidate)); sdp = kSdpTcpSOCandidate; EXPECT_TRUE(SdpDeserializeCandidate(sdp, &jcandidate)); - sdp = kSdpTcpInvalidCandidate; - EXPECT_FALSE(SdpDeserializeCandidate(sdp, &jcandidate)); } // This test verifies the deserialization of candidate-attribute @@ -1976,10 +1970,29 @@ TEST_F(WebRtcSdpTest, DeserializeRawCandidateAttribute) { Replace("candidate:", "", &candidate_attribute); EXPECT_FALSE(SdpDeserializeCandidate(candidate_attribute, &jcandidate)); - // Concatenating additional candidate. Expecting deserialization to fail. - candidate_attribute = kRawCandidate; - candidate_attribute.append("candidate:1 2 udp 1234 192.168.1.1 typ host"); + // Candidate line with IPV6 address. + EXPECT_TRUE(SdpDeserializeCandidate(kRawIPV6Candidate, &jcandidate)); +} + +// This test verifies that the deserialization of an invalid candidate string +// fails. +TEST_F(WebRtcSdpTest, DeserializeInvalidCandidiate) { + JsepIceCandidate jcandidate(kDummyMid, kDummyIndex); + + std::string candidate_attribute = kRawCandidate; + candidate_attribute.replace(0, 1, "x"); EXPECT_FALSE(SdpDeserializeCandidate(candidate_attribute, &jcandidate)); + + candidate_attribute = kSdpOneCandidate; + candidate_attribute.replace(0, 1, "x"); + EXPECT_FALSE(SdpDeserializeCandidate(candidate_attribute, &jcandidate)); + + candidate_attribute = kRawCandidate; + candidate_attribute.append("\r\n"); + candidate_attribute.append(kRawCandidate); + EXPECT_FALSE(SdpDeserializeCandidate(candidate_attribute, &jcandidate)); + + EXPECT_FALSE(SdpDeserializeCandidate(kSdpTcpInvalidCandidate, &jcandidate)); } TEST_F(WebRtcSdpTest, DeserializeSdpWithRtpDataChannels) {