diff --git a/pc/jsep_session_description.cc b/pc/jsep_session_description.cc index 001aa768c65..4c0453198dd 100644 --- a/pc/jsep_session_description.cc +++ b/pc/jsep_session_description.cc @@ -90,7 +90,23 @@ void UpdateConnectionAddress( } rtc::SocketAddress connection_addr(ip, port); if (rtc::IPIsUnspec(connection_addr.ipaddr()) && !hostname.empty()) { - connection_addr = rtc::SocketAddress(hostname, port); + // When a hostname candidate becomes the (default) connection address, + // we use the dummy address 0.0.0.0 and port 9 in the c= and the m= lines. + // + // We have observed in deployment that with a FQDN in a c= line, SDP parsing + // could fail in other JSEP implementations. We note that the wildcard + // addresses (0.0.0.0 or ::) with port 9 are given the exception as the + // connection address that will not result in an ICE mismatch + // (draft-ietf-mmusic-ice-sip-sdp). Also, 0.0.0.0 or :: can be used as the + // connection address in the initial offer or answer with trickle ICE + // if the offerer or answerer does not want to include the host IP address + // (draft-ietf-mmusic-trickle-ice-sip), and in particular 0.0.0.0 has been + // widely deployed for this use without outstanding compatibility issues. + // Combining the above considerations, we use 0.0.0.0 with port 9 to + // populate the c= and the m= lines. See |BuildMediaDescription| in + // webrtc_sdp.cc for the SDP generation with + // |media_desc->connection_address()|. + connection_addr = rtc::SocketAddress(kDummyAddress, kDummyPort); } media_desc->set_connection_address(connection_addr); } diff --git a/pc/jsep_session_description_unittest.cc b/pc/jsep_session_description_unittest.cc index 8abb500480c..ef86ef41fb9 100644 --- a/pc/jsep_session_description_unittest.cc +++ b/pc/jsep_session_description_unittest.cc @@ -223,11 +223,14 @@ TEST_F(JsepSessionDescriptionTest, AddHostnameCandidate) { c.set_protocol(cricket::UDP_PROTOCOL_NAME); c.set_address(rtc::SocketAddress("example.local", 1234)); c.set_type(cricket::LOCAL_PORT_TYPE); - JsepIceCandidate hostname_candidate("audio", 0, c); + const size_t audio_index = 0; + JsepIceCandidate hostname_candidate("audio", audio_index, c); EXPECT_TRUE(jsep_desc_->AddCandidate(&hostname_candidate)); + ASSERT_NE(nullptr, jsep_desc_->description()); - const auto& content = jsep_desc_->description()->contents()[0]; - EXPECT_EQ("example.local:1234", + ASSERT_EQ(2u, jsep_desc_->description()->contents().size()); + const auto& content = jsep_desc_->description()->contents()[audio_index]; + EXPECT_EQ("0.0.0.0:9", content.media_description()->connection_address().ToString()); } @@ -242,6 +245,39 @@ TEST_F(JsepSessionDescriptionTest, SerializeDeserialize) { EXPECT_EQ(sdp, parsed_sdp); } +// Test that we can serialize a JsepSessionDescription when a hostname candidate +// is the default destination and deserialize it again. The connection address +// in the deserialized description should be the dummy address 0.0.0.0:9. +TEST_F(JsepSessionDescriptionTest, SerializeDeserializeWithHostnameCandidate) { + cricket::Candidate c; + c.set_component(cricket::ICE_CANDIDATE_COMPONENT_RTP); + c.set_protocol(cricket::UDP_PROTOCOL_NAME); + c.set_address(rtc::SocketAddress("example.local", 1234)); + c.set_type(cricket::LOCAL_PORT_TYPE); + const size_t audio_index = 0; + const size_t video_index = 1; + JsepIceCandidate hostname_candidate_audio("audio", audio_index, c); + JsepIceCandidate hostname_candidate_video("video", video_index, c); + EXPECT_TRUE(jsep_desc_->AddCandidate(&hostname_candidate_audio)); + EXPECT_TRUE(jsep_desc_->AddCandidate(&hostname_candidate_video)); + + std::string sdp = Serialize(jsep_desc_.get()); + + auto parsed_jsep_desc = DeSerialize(sdp); + EXPECT_EQ(2u, parsed_jsep_desc->number_of_mediasections()); + + ASSERT_NE(nullptr, parsed_jsep_desc->description()); + ASSERT_EQ(2u, parsed_jsep_desc->description()->contents().size()); + const auto& audio_content = + parsed_jsep_desc->description()->contents()[audio_index]; + const auto& video_content = + parsed_jsep_desc->description()->contents()[video_index]; + EXPECT_EQ("0.0.0.0:9", + audio_content.media_description()->connection_address().ToString()); + EXPECT_EQ("0.0.0.0:9", + video_content.media_description()->connection_address().ToString()); +} + // Tests that we can serialize and deserialize a JsepSesssionDescription // with candidates. TEST_F(JsepSessionDescriptionTest, SerializeDeserializeWithCandidates) { diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index 7d33a6a4f80..a022e5642dd 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -1468,10 +1468,6 @@ void BuildMediaDescription(const ContentInfo* content_info, } else if (media_desc->connection_address().family() == AF_INET6) { os << " " << kConnectionIpv6Addrtype << " " << media_desc->connection_address().ipaddr().ToString(); - } else if (!media_desc->connection_address().hostname().empty()) { - // For hostname candidates, we use c=IN IP4 . - os << " " << kConnectionIpv4Addrtype << " " - << media_desc->connection_address().hostname(); } else { os << " " << kConnectionIpv4Addrtype << " " << kDummyAddress; } diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc index 2be454ffc99..bbf28c9f75f 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -4259,31 +4259,6 @@ TEST_F(WebRtcSdpTest, SerializeAndDeserializeWithConnectionAddress) { video_desc->connection_address().ToString()); } -// Test that a media description that contains a hostname connection address can -// be correctly serialized. -TEST_F(WebRtcSdpTest, SerializeAndDeserializeWithHostnameConnectionAddress) { - JsepSessionDescription expected_jsep(kDummyType); - cricket::Candidate c; - const rtc::SocketAddress hostname_addr("example.local", 1234); - audio_desc_->set_connection_address(hostname_addr); - video_desc_->set_connection_address(hostname_addr); - ASSERT_TRUE( - expected_jsep.Initialize(desc_.Clone(), kSessionId, kSessionVersion)); - // Serialization. - std::string message = webrtc::SdpSerialize(expected_jsep); - // Deserialization. - JsepSessionDescription jdesc(kDummyType); - ASSERT_TRUE(SdpDeserialize(message, &jdesc)); - auto audio_desc = jdesc.description() - ->GetContentByName(kAudioContentName) - ->media_description(); - auto video_desc = jdesc.description() - ->GetContentByName(kVideoContentName) - ->media_description(); - EXPECT_EQ(hostname_addr, audio_desc->connection_address()); - EXPECT_EQ(hostname_addr, video_desc->connection_address()); -} - // RFC4566 says "If a session has no meaningful name, the value "s= " SHOULD be // used (i.e., a single space as the session name)." So we should accept that. TEST_F(WebRtcSdpTest, DeserializeEmptySessionName) {