diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc index c596951f93..9a4dac9089 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc @@ -1996,6 +1996,12 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer( return false; // Fails the session setup. } + // Respond with sctpmap if the offer uses sctpmap. + const DataContentDescription* offer_data_description = + static_cast(data_content->description); + bool offer_uses_sctpmap = offer_data_description->use_sctpmap(); + data_answer->set_use_sctpmap(offer_uses_sctpmap); + bool rejected = !options.has_data() || data_content->rejected || !IsMediaProtocolSupported(MEDIA_TYPE_DATA, data_answer->protocol(), diff --git a/webrtc/pc/mediasession.h b/webrtc/pc/mediasession.h index 06815ecba3..68f4c37099 100644 --- a/webrtc/pc/mediasession.h +++ b/webrtc/pc/mediasession.h @@ -401,10 +401,18 @@ class VideoContentDescription : public MediaContentDescriptionImpl { class DataContentDescription : public MediaContentDescriptionImpl { public: + DataContentDescription() {} + virtual ContentDescription* Copy() const { return new DataContentDescription(*this); } virtual MediaType type() const { return MEDIA_TYPE_DATA; } + + bool use_sctpmap() const { return use_sctpmap_; } + void set_use_sctpmap(bool enable) { use_sctpmap_ = enable; } + + private: + bool use_sctpmap_ = true; }; // Creates media session descriptions according to the supplied codecs and @@ -456,9 +464,9 @@ class MediaSessionDescriptionFactory { const MediaSessionOptions& options, const SessionDescription* current_description) const; SessionDescription* CreateAnswer( - const SessionDescription* offer, - const MediaSessionOptions& options, - const SessionDescription* current_description) const; + const SessionDescription* offer, + const MediaSessionOptions& options, + const SessionDescription* current_description) const; private: const AudioCodecs& GetAudioCodecsForOffer( diff --git a/webrtc/pc/mediasession_unittest.cc b/webrtc/pc/mediasession_unittest.cc index 6676240b0c..6837e50fa6 100644 --- a/webrtc/pc/mediasession_unittest.cc +++ b/webrtc/pc/mediasession_unittest.cc @@ -912,27 +912,27 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswer) { std::unique_ptr answer( f2_.CreateAnswer(offer.get(), opts, NULL)); const ContentInfo* ac = answer->GetContentByName("audio"); - const ContentInfo* vc = answer->GetContentByName("data"); + const ContentInfo* dc = answer->GetContentByName("data"); ASSERT_TRUE(ac != NULL); - ASSERT_TRUE(vc != NULL); + ASSERT_TRUE(dc != NULL); EXPECT_EQ(std::string(NS_JINGLE_RTP), ac->type); - EXPECT_EQ(std::string(NS_JINGLE_RTP), vc->type); + EXPECT_EQ(std::string(NS_JINGLE_RTP), dc->type); const AudioContentDescription* acd = static_cast(ac->description); - const DataContentDescription* vcd = - static_cast(vc->description); + const DataContentDescription* dcd = + static_cast(dc->description); EXPECT_EQ(MEDIA_TYPE_AUDIO, acd->type()); EXPECT_EQ(MAKE_VECTOR(kAudioCodecsAnswer), acd->codecs()); EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // negotiated auto bw EXPECT_NE(0U, acd->first_ssrc()); // a random nonzero ssrc EXPECT_TRUE(acd->rtcp_mux()); // negotiated rtcp-mux ASSERT_CRYPTO(acd, 1U, CS_AES_CM_128_HMAC_SHA1_32); - EXPECT_EQ(MEDIA_TYPE_DATA, vcd->type()); - EXPECT_EQ(MAKE_VECTOR(kDataCodecsAnswer), vcd->codecs()); - EXPECT_NE(0U, vcd->first_ssrc()); // a random nonzero ssrc - EXPECT_TRUE(vcd->rtcp_mux()); // negotiated rtcp-mux - ASSERT_CRYPTO(vcd, 1U, CS_AES_CM_128_HMAC_SHA1_80); - EXPECT_EQ(std::string(cricket::kMediaProtocolSavpf), vcd->protocol()); + EXPECT_EQ(MEDIA_TYPE_DATA, dcd->type()); + EXPECT_EQ(MAKE_VECTOR(kDataCodecsAnswer), dcd->codecs()); + EXPECT_NE(0U, dcd->first_ssrc()); // a random nonzero ssrc + EXPECT_TRUE(dcd->rtcp_mux()); // negotiated rtcp-mux + ASSERT_CRYPTO(dcd, 1U, CS_AES_CM_128_HMAC_SHA1_80); + EXPECT_EQ(std::string(cricket::kMediaProtocolSavpf), dcd->protocol()); } TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerGcm) { @@ -946,27 +946,70 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerGcm) { std::unique_ptr answer( f2_.CreateAnswer(offer.get(), opts, NULL)); const ContentInfo* ac = answer->GetContentByName("audio"); - const ContentInfo* vc = answer->GetContentByName("data"); + const ContentInfo* dc = answer->GetContentByName("data"); ASSERT_TRUE(ac != NULL); - ASSERT_TRUE(vc != NULL); + ASSERT_TRUE(dc != NULL); EXPECT_EQ(std::string(NS_JINGLE_RTP), ac->type); - EXPECT_EQ(std::string(NS_JINGLE_RTP), vc->type); + EXPECT_EQ(std::string(NS_JINGLE_RTP), dc->type); const AudioContentDescription* acd = static_cast(ac->description); - const DataContentDescription* vcd = - static_cast(vc->description); + const DataContentDescription* dcd = + static_cast(dc->description); EXPECT_EQ(MEDIA_TYPE_AUDIO, acd->type()); EXPECT_EQ(MAKE_VECTOR(kAudioCodecsAnswer), acd->codecs()); EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // negotiated auto bw EXPECT_NE(0U, acd->first_ssrc()); // a random nonzero ssrc EXPECT_TRUE(acd->rtcp_mux()); // negotiated rtcp-mux ASSERT_CRYPTO(acd, 1U, CS_AEAD_AES_256_GCM); - EXPECT_EQ(MEDIA_TYPE_DATA, vcd->type()); - EXPECT_EQ(MAKE_VECTOR(kDataCodecsAnswer), vcd->codecs()); - EXPECT_NE(0U, vcd->first_ssrc()); // a random nonzero ssrc - EXPECT_TRUE(vcd->rtcp_mux()); // negotiated rtcp-mux - ASSERT_CRYPTO(vcd, 1U, CS_AEAD_AES_256_GCM); - EXPECT_EQ(std::string(cricket::kMediaProtocolSavpf), vcd->protocol()); + EXPECT_EQ(MEDIA_TYPE_DATA, dcd->type()); + EXPECT_EQ(MAKE_VECTOR(kDataCodecsAnswer), dcd->codecs()); + EXPECT_NE(0U, dcd->first_ssrc()); // a random nonzero ssrc + EXPECT_TRUE(dcd->rtcp_mux()); // negotiated rtcp-mux + ASSERT_CRYPTO(dcd, 1U, CS_AEAD_AES_256_GCM); + EXPECT_EQ(std::string(cricket::kMediaProtocolSavpf), dcd->protocol()); +} + +// The use_sctpmap flag should be set in a DataContentDescription by default. +// The answer's use_sctpmap flag should match the offer's. +TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerUsesSctpmap) { + MediaSessionOptions opts; + opts.data_channel_type = cricket::DCT_SCTP; + std::unique_ptr offer(f1_.CreateOffer(opts, NULL)); + ASSERT_TRUE(offer.get() != NULL); + ContentInfo* dc_offer = offer->GetContentByName("data"); + ASSERT_TRUE(dc_offer != NULL); + DataContentDescription* dcd_offer = + static_cast(dc_offer->description); + EXPECT_TRUE(dcd_offer->use_sctpmap()); + + std::unique_ptr answer( + f2_.CreateAnswer(offer.get(), opts, NULL)); + const ContentInfo* dc_answer = answer->GetContentByName("data"); + ASSERT_TRUE(dc_answer != NULL); + const DataContentDescription* dcd_answer = + static_cast(dc_answer->description); + EXPECT_TRUE(dcd_answer->use_sctpmap()); +} + +// The answer's use_sctpmap flag should match the offer's. +TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerWithoutSctpmap) { + MediaSessionOptions opts; + opts.data_channel_type = cricket::DCT_SCTP; + std::unique_ptr offer(f1_.CreateOffer(opts, NULL)); + ASSERT_TRUE(offer.get() != NULL); + ContentInfo* dc_offer = offer->GetContentByName("data"); + ASSERT_TRUE(dc_offer != NULL); + DataContentDescription* dcd_offer = + static_cast(dc_offer->description); + dcd_offer->set_use_sctpmap(false); + + std::unique_ptr answer( + f2_.CreateAnswer(offer.get(), opts, NULL)); + const ContentInfo* dc_answer = answer->GetContentByName("data"); + ASSERT_TRUE(dc_answer != NULL); + const DataContentDescription* dcd_answer = + static_cast(dc_answer->description); + EXPECT_FALSE(dcd_answer->use_sctpmap()); } // Verifies that the order of the media contents in the offer is preserved in diff --git a/webrtc/pc/webrtcsdp.cc b/webrtc/pc/webrtcsdp.cc index ed49c344e8..b749d6ff32 100644 --- a/webrtc/pc/webrtcsdp.cc +++ b/webrtc/pc/webrtcsdp.cc @@ -234,7 +234,9 @@ static void BuildMediaDescription(const ContentInfo* content_info, const std::vector& candidates, bool unified_plan_sdp, std::string* message); -static void BuildSctpContentAttributes(std::string* message, int sctp_port); +static void BuildSctpContentAttributes(std::string* message, + int sctp_port, + bool use_sctpmap); static void BuildRtpContentAttributes(const MediaContentDescription* media_desc, const MediaType media_type, bool unified_plan_sdp, @@ -1275,16 +1277,21 @@ void BuildMediaDescription(const ContentInfo* content_info, if (IsDtlsSctp(media_desc->protocol())) { fmt.append(" "); - for (std::vector::const_iterator it = - data_desc->codecs().begin(); - it != data_desc->codecs().end(); ++it) { - if (cricket::CodecNamesEq(it->name, cricket::kGoogleSctpDataCodecName) - && it->GetParam(cricket::kCodecParamPort, &sctp_port)) { - break; + if (data_desc->use_sctpmap()) { + for (std::vector::const_iterator it = + data_desc->codecs().begin(); + it != data_desc->codecs().end(); ++it) { + if (cricket::CodecNamesEq(it->name, + cricket::kGoogleSctpDataCodecName) && + it->GetParam(cricket::kCodecParamPort, &sctp_port)) { + break; + } } - } - fmt.append(rtc::ToString(sctp_port)); + fmt.append(rtc::ToString(sctp_port)); + } else { + fmt.append(kDefaultSctpmapProtocol); + } } else { for (std::vector::const_iterator it = data_desc->codecs().begin(); @@ -1407,23 +1414,34 @@ void BuildMediaDescription(const ContentInfo* content_info, AddLine(os.str(), message); if (IsDtlsSctp(media_desc->protocol())) { - BuildSctpContentAttributes(message, sctp_port); + const DataContentDescription* data_desc = + static_cast(media_desc); + bool use_sctpmap = data_desc->use_sctpmap(); + BuildSctpContentAttributes(message, sctp_port, use_sctpmap); } else if (IsRtp(media_desc->protocol())) { BuildRtpContentAttributes(media_desc, media_type, unified_plan_sdp, message); } } -void BuildSctpContentAttributes(std::string* message, int sctp_port) { - // draft-ietf-mmusic-sctp-sdp-04 - // a=sctpmap:sctpmap-number protocol [streams] - // TODO(lally): switch this over to mmusic-sctp-sdp-12 (or later), with - // 'a=sctp-port:' +void BuildSctpContentAttributes(std::string* message, + int sctp_port, + bool use_sctpmap) { std::ostringstream os; - InitAttrLine(kAttributeSctpmap, &os); - os << kSdpDelimiterColon << sctp_port << kSdpDelimiterSpace - << kDefaultSctpmapProtocol << kSdpDelimiterSpace - << cricket::kMaxSctpStreams; + if (use_sctpmap) { + // draft-ietf-mmusic-sctp-sdp-04 + // a=sctpmap:sctpmap-number protocol [streams] + InitAttrLine(kAttributeSctpmap, &os); + os << kSdpDelimiterColon << sctp_port << kSdpDelimiterSpace + << kDefaultSctpmapProtocol << kSdpDelimiterSpace + << cricket::kMaxSctpStreams; + } else { + // draft-ietf-mmusic-sctp-sdp-23 + // a=sctp-port: + InitAttrLine(kAttributeSctpPort, &os); + os << kSdpDelimiterColon << sctp_port; + // TODO(zstein): emit max-message-size here + } AddLine(os.str(), message); } @@ -2291,6 +2309,7 @@ bool ParseMediaDescription(const std::string& message, std::vector fields; rtc::split(line.substr(kLinePrefixLength), kSdpDelimiterSpace, &fields); + const size_t expected_min_fields = 4; if (fields.size() < expected_min_fields) { return ParseFailedExpectMinFieldNum(line, expected_min_fields, error); @@ -2351,10 +2370,15 @@ bool ParseMediaDescription(const std::string& message, candidates, error); content.reset(data_desc); - int p; - if (data_desc && IsDtlsSctp(protocol) && rtc::FromString(fields[3], &p)) { - if (!AddSctpDataCodec(data_desc, p)) - return false; + if (data_desc && IsDtlsSctp(protocol)) { + int p; + if (rtc::FromString(fields[3], &p)) { + if (!AddSctpDataCodec(data_desc, p)) { + return false; + } + } else if (fields[3] == kDefaultSctpmapProtocol) { + data_desc->set_use_sctpmap(false); + } } } else { LOG(LS_WARNING) << "Unsupported media type: " << line; diff --git a/webrtc/pc/webrtcsdp_unittest.cc b/webrtc/pc/webrtcsdp_unittest.cc index 1a6a212cdd..f5cac8921f 100644 --- a/webrtc/pc/webrtcsdp_unittest.cc +++ b/webrtc/pc/webrtcsdp_unittest.cc @@ -65,6 +65,9 @@ typedef std::vector AudioCodecs; typedef std::vector Candidates; static const uint32_t kDefaultSctpPort = 5000; +static const char kDefaultSctpPortStr[] = "5000"; +static const uint16_t kUnusualSctpPort = 9556; +static const char kUnusualSctpPortStr[] = "9556"; static const char kSessionTime[] = "t=0 0\r\n"; static const uint32_t kCandidatePriority = 2130706432U; // pref = 1.0 static const char kAttributeIceUfragVoice[] = "a=ice-ufrag:ufrag_voice\r\n"; @@ -1210,6 +1213,11 @@ class WebRtcSdpTest : public testing::Test { } } + void CompareDataContentDescription(const DataContentDescription* dcd1, + const DataContentDescription* dcd2) { + EXPECT_EQ(dcd1->use_sctpmap(), dcd2->use_sctpmap()); + CompareMediaContentDescription(dcd1, dcd2); + } void CompareSessionDescription(const SessionDescription& desc1, const SessionDescription& desc2) { @@ -1251,7 +1259,7 @@ class WebRtcSdpTest : public testing::Test { static_cast(c1.description); const DataContentDescription* dcd2 = static_cast(c2.description); - CompareMediaContentDescription(dcd1, dcd2); + CompareDataContentDescription(dcd1, dcd2); } } @@ -1486,9 +1494,10 @@ class WebRtcSdpTest : public testing::Test { return true; } - void AddSctpDataChannel() { + void AddSctpDataChannel(bool use_sctpmap) { std::unique_ptr data(new DataContentDescription()); data_desc_ = data.get(); + data_desc_->set_use_sctpmap(use_sctpmap); data_desc_->set_protocol(cricket::kMediaProtocolDtlsSctp); DataCodec codec(cricket::kGoogleSctpDataCodecPlType, cricket::kGoogleSctpDataCodecName); @@ -2058,7 +2067,8 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithRtpDataChannel) { } TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithSctpDataChannel) { - AddSctpDataChannel(); + bool use_sctpmap = true; + AddSctpDataChannel(use_sctpmap); JsepSessionDescription jsep_desc(kDummyString); ASSERT_TRUE(jsep_desc.Initialize(desc_.Copy(), kSessionId, kSessionVersion)); @@ -2070,7 +2080,8 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithSctpDataChannel) { } TEST_F(WebRtcSdpTest, SerializeWithSctpDataChannelAndNewPort) { - AddSctpDataChannel(); + bool use_sctpmap = true; + AddSctpDataChannel(use_sctpmap); JsepSessionDescription jsep_desc(kDummyString); ASSERT_TRUE(jsep_desc.Initialize(desc_.Copy(), kSessionId, kSessionVersion)); @@ -2570,7 +2581,8 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithRtpDataChannels) { } TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannels) { - AddSctpDataChannel(); + bool use_sctpmap = true; + AddSctpDataChannel(use_sctpmap); JsepSessionDescription jdesc(kDummyString); ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion)); @@ -2596,7 +2608,8 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannels) { } TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpPort) { - AddSctpDataChannel(); + bool use_sctpmap = false; + AddSctpDataChannel(use_sctpmap); JsepSessionDescription jdesc(kDummyString); ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion)); @@ -2622,7 +2635,8 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpPort) { } TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpColonPort) { - AddSctpDataChannel(); + bool use_sctpmap = false; + AddSctpDataChannel(use_sctpmap); JsepSessionDescription jdesc(kDummyString); ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion)); @@ -2650,7 +2664,8 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpColonPort) { // Test to check the behaviour if sctp-port is specified // on the m= line and in a=sctp-port. TEST_F(WebRtcSdpTest, DeserializeSdpWithMultiSctpPort) { - AddSctpDataChannel(); + bool use_sctpmap = true; + AddSctpDataChannel(use_sctpmap); JsepSessionDescription jdesc(kDummyString); ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion)); @@ -2676,20 +2691,10 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithCorruptedSctpDataChannels) { // No crash is a pass. } -TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelAndNewPort) { - AddSctpDataChannel(); - const uint16_t kUnusualSctpPort = 9556; - char default_portstr[16]; - char unusual_portstr[16]; - rtc::sprintfn(default_portstr, sizeof(default_portstr), "%d", - kDefaultSctpPort); - rtc::sprintfn(unusual_portstr, sizeof(unusual_portstr), "%d", - kUnusualSctpPort); - - // First setup the expected JsepSessionDescription. - JsepSessionDescription jdesc(kDummyString); +void MutateJsepSctpPort(JsepSessionDescription& jdesc, + const SessionDescription& desc) { // take our pre-built session description and change the SCTP port. - cricket::SessionDescription* mutant = desc_.Copy(); + cricket::SessionDescription* mutant = desc.Copy(); DataContentDescription* dcdesc = static_cast( mutant->GetContentDescriptionByName(kDataContentName)); std::vector codecs(dcdesc->codecs()); @@ -2701,27 +2706,46 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelAndNewPort) { // note: mutant's owned by jdesc now. ASSERT_TRUE(jdesc.Initialize(mutant, kSessionId, kSessionVersion)); mutant = NULL; +} + +TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelAndUnusualPort) { + bool use_sctpmap = true; + AddSctpDataChannel(use_sctpmap); + + // First setup the expected JsepSessionDescription. + JsepSessionDescription jdesc(kDummyString); + MutateJsepSctpPort(jdesc, desc_); // Then get the deserialized JsepSessionDescription. std::string sdp_with_data = kSdpString; sdp_with_data.append(kSdpSctpDataChannelString); - rtc::replace_substrs(default_portstr, strlen(default_portstr), - unusual_portstr, strlen(unusual_portstr), - &sdp_with_data); + rtc::replace_substrs(kDefaultSctpPortStr, strlen(kDefaultSctpPortStr), + kUnusualSctpPortStr, strlen(kUnusualSctpPortStr), + &sdp_with_data); JsepSessionDescription jdesc_output(kDummyString); EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output)); EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output)); +} + +TEST_F(WebRtcSdpTest, + DeserializeSdpWithSctpDataChannelAndUnusualPortInAttribute) { + bool use_sctpmap = false; + AddSctpDataChannel(use_sctpmap); + + JsepSessionDescription jdesc(kDummyString); + MutateJsepSctpPort(jdesc, desc_); // We need to test the deserialized JsepSessionDescription from // kSdpSctpDataChannelStringWithSctpPort for // draft-ietf-mmusic-sctp-sdp-07 // a=sctp-port - sdp_with_data = kSdpString; + std::string sdp_with_data = kSdpString; sdp_with_data.append(kSdpSctpDataChannelStringWithSctpPort); - rtc::replace_substrs(default_portstr, strlen(default_portstr), - unusual_portstr, strlen(unusual_portstr), - &sdp_with_data); + rtc::replace_substrs(kDefaultSctpPortStr, strlen(kDefaultSctpPortStr), + kUnusualSctpPortStr, strlen(kUnusualSctpPortStr), + &sdp_with_data); + JsepSessionDescription jdesc_output(kDummyString); EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output)); EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output)); @@ -2744,7 +2768,8 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithRtpDataChannelsAndBandwidth) { } TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsAndBandwidth) { - AddSctpDataChannel(); + bool use_sctpmap = true; + AddSctpDataChannel(use_sctpmap); JsepSessionDescription jdesc(kDummyString); DataContentDescription* dcd = static_cast( GetFirstDataContent(&desc_)->description);