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

@@ -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:<blah>
-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;
+  std::string first_line = message;
-  GetFirstLine(message, &first_line);
+  size_t pos = 0;
+  GetLine(message, &pos, &first_line);
 
-  size_t start_pos = kLinePrefixLength;  // Starting position to parse.
+  // Makes sure |message| contains only one line.
-  if (IsRawCandidate(first_line)) {
+  if (message.size() > first_line.size()) {
-    // From WebRTC draft section 4.8.1.1 candidate-attribute will be
+    std::string left, right;
-    // just candidate:<candidate> not a=candidate:<blah>CRLF
+    if (SplitByDelimiter(message, kNewLine, &left, &right) && !right.empty()) {
-    start_pos = 0;
+      return ParseFailed(message, 0, "Expect one line only", error);
-  } else if (!IsLineType(first_line, kLineTypeAttributes) ||
+    }
-             !HasAttribute(first_line, kAttributeCandidate)) {
+  }
-    // Must start with a=candidate line.
+
-    // Expecting to be of the format a=candidate:<blah>CRLF.
+  // From WebRTC draft section 4.8.1.1 candidate-attribute should be
+  // candidate:<candidate> when trickled, but we still support
+  // a=candidate:<blah>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:<value>".
+  if (!SplitByDelimiter(first_line, kSdpDelimiterColon,
+                        &attribute_candidate, &candidate_value) ||
+      attribute_candidate != kAttributeCandidate) {
     if (is_raw) {
       std::ostringstream description;
-      description << "Expect line: "
+      description << "Expect line: " << kAttributeCandidate
-                  << kAttributeCandidate
                   << ":" << "<candidate-str>";
       return ParseFailed(first_line, 0, description.str(), error);
     } else {
@@ -974,8 +963,8 @@ bool ParseCandidate(const std::string& message, Candidate* candidate,
   }
 
   std::vector<std::string> fields;
-  rtc::split(first_line.substr(start_pos),
+  rtc::split(candidate_value, kSdpDelimiterSpace, &fields);
-                   kSdpDelimiterSpace, &fields);
+
   // RFC 5245
   // a=candidate:<foundation> <component-id> <transport> <priority>
   // <connection-address> <port> typ <candidate-types>
@@ -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;
+  std::string foundation = fields[0];
-  if (!GetValue(fields[0], kAttributeCandidate, &foundation, error)) {
+
-    return false;
-  }
   int component_id = 0;
   if (!GetValueFromString(first_line, fields[1], &component_id, error)) {
     return false;

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 line with IPV6 address.
-  candidate_attribute = kRawCandidate;
+  EXPECT_TRUE(SdpDeserializeCandidate(kRawIPV6Candidate, &jcandidate));
-  candidate_attribute.append("candidate:1 2 udp 1234 192.168.1.1 typ host");
+}
+
+// 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) {