Handle missing tcptype on TCP ICE candidates.
Our implementation accepts TCP candidates with a missing tcptype
field, treating this as a passive candidate.
However, if you try to convert such a candidate to SDP and back,
which chromium started to do at some point, this was resulting in an
error. This CL fixes that.
Bug: webrtc:11423
Change-Id: Iec48d340f421f63f2b7a16c9496ea92ccd165981
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/172020
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Taylor <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31026}
diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index 7846e5e..f77327f 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -1111,11 +1111,14 @@
if (!StringToProto(transport.c_str(), &protocol)) {
return ParseFailed(first_line, "Unsupported transport type.", error);
}
+ bool tcp_protocol = false;
switch (protocol) {
+ // Supported protocols.
case cricket::PROTO_UDP:
+ break;
case cricket::PROTO_TCP:
case cricket::PROTO_SSLTCP:
- // Supported protocol.
+ tcp_protocol = true;
break;
default:
return ParseFailed(first_line, "Unsupported transport type.", error);
@@ -1172,9 +1175,14 @@
return ParseFailed(first_line, "Invalid TCP candidate type.", error);
}
- if (protocol != cricket::PROTO_TCP) {
+ if (!tcp_protocol) {
return ParseFailed(first_line, "Invalid non-TCP candidate", error);
}
+ } else if (tcp_protocol) {
+ // We allow the tcptype to be missing, for backwards compatibility,
+ // treating it as a passive candidate.
+ // TODO(bugs.webrtc.org/11466): Treat a missing tcptype as an error?
+ tcptype = cricket::TCPTYPE_PASSIVE_STR;
}
// Extension
@@ -2007,7 +2015,11 @@
<< candidate.related_address().PortAsString() << " ";
}
- if (candidate.protocol() == cricket::TCP_PROTOCOL_NAME) {
+ // Note that we allow the tcptype to be missing, for backwards
+ // compatibility; the implementation treats this as a passive candidate.
+ // TODO(bugs.webrtc.org/11466): Treat a missing tcptype as an error?
+ if (candidate.protocol() == cricket::TCP_PROTOCOL_NAME &&
+ !candidate.tcptype().empty()) {
os << kTcpCandidateType << " " << candidate.tcptype() << " ";
}
diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc
index b849f01..a2ad4b8 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -2460,6 +2460,32 @@
EXPECT_EQ(std::string(kSdpTcpActiveCandidate), message);
}
+// Test serializing a TCP candidate that came in with a missing tcptype. This
+// shouldn't happen according to the spec, but our implementation has been
+// accepting this for quite some time, treating it as a passive candidate.
+//
+// So, we should be able to at least convert such candidates to and from SDP.
+// See: bugs.webrtc.org/11423
+TEST_F(WebRtcSdpTest, ParseTcpCandidateWithoutTcptype) {
+ std::string missing_tcptype =
+ "candidate:a0+B/1 1 tcp 2130706432 192.168.1.5 9999 typ host";
+ JsepIceCandidate jcandidate(kDummyMid, kDummyIndex);
+ EXPECT_TRUE(SdpDeserializeCandidate(missing_tcptype, &jcandidate));
+
+ EXPECT_EQ(std::string(cricket::TCPTYPE_PASSIVE_STR),
+ jcandidate.candidate().tcptype());
+}
+
+TEST_F(WebRtcSdpTest, ParseSslTcpCandidate) {
+ std::string ssltcp =
+ "candidate:a0+B/1 1 ssltcp 2130706432 192.168.1.5 9999 typ host tcptype "
+ "passive";
+ JsepIceCandidate jcandidate(kDummyMid, kDummyIndex);
+ EXPECT_TRUE(SdpDeserializeCandidate(ssltcp, &jcandidate));
+
+ EXPECT_EQ(std::string("ssltcp"), jcandidate.candidate().protocol());
+}
+
TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithH264) {
cricket::VideoCodec h264_codec("H264");
h264_codec.SetParam("profile-level-id", "42e01f");
