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}
This commit is contained in:
Taylor Brandstetter 2020-04-02 12:36:38 -07:00 committed by Commit Bot
parent 10575a2cb3
commit 8206bc0f00
2 changed files with 41 additions and 3 deletions

View file

@ -1111,11 +1111,14 @@ bool ParseCandidate(const std::string& message,
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 @@ bool ParseCandidate(const std::string& message,
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 @@ void BuildCandidate(const std::vector<Candidate>& candidates,
<< 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() << " ";
}

View file

@ -2460,6 +2460,32 @@ TEST_F(WebRtcSdpTest, SerializeTcpCandidates) {
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");