From 48cce4d9e82736b21f2df77b5d915122b3150ae0 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Thu, 11 Apr 2019 10:41:24 +0200 Subject: [PATCH] Parse "max-message-size" parameter from SCTP SDP description Bug: chromium:943975 Change-Id: I559093cfa3686f2a388b872774df8f0737963281 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132224 Commit-Queue: Harald Alvestrand Reviewed-by: Seth Hampson Cr-Commit-Position: refs/heads/master@{#27555} --- media/base/media_constants.cc | 1 + media/base/media_constants.h | 1 + pc/webrtc_sdp.cc | 78 ++++++++++++++++++++++++++++++----- pc/webrtc_sdp_unittest.cc | 47 ++++++++++++++++++++- 4 files changed, 114 insertions(+), 13 deletions(-) diff --git a/media/base/media_constants.cc b/media/base/media_constants.cc index b10304a26c..6078d4817b 100644 --- a/media/base/media_constants.cc +++ b/media/base/media_constants.cc @@ -90,6 +90,7 @@ const char kCodecParamMinBitrate[] = "x-google-min-bitrate"; const char kCodecParamStartBitrate[] = "x-google-start-bitrate"; const char kCodecParamMaxQuantization[] = "x-google-max-quantization"; const char kCodecParamPort[] = "x-google-port"; +const char kCodecParamMaxMessageSize[] = "x-google-max-message-size"; const int kGoogleRtpDataCodecPlType = 109; const char kGoogleRtpDataCodecName[] = "google-data"; diff --git a/media/base/media_constants.h b/media/base/media_constants.h index ea3c49a926..0f940f40fb 100644 --- a/media/base/media_constants.h +++ b/media/base/media_constants.h @@ -112,6 +112,7 @@ extern const char kCodecParamMinBitrate[]; extern const char kCodecParamStartBitrate[]; extern const char kCodecParamMaxQuantization[]; extern const char kCodecParamPort[]; +extern const char kCodecParamMaxMessageSize[]; // We put the data codec names here so callers of DataEngine::CreateChannel // don't have to import rtpdataengine.h to get the codec names they want to diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index 006e4a1c38..a119503b8b 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -166,9 +166,10 @@ static const char kAttributeRecvOnly[] = "recvonly"; static const char kAttributeRtcpFb[] = "rtcp-fb"; static const char kAttributeSendRecv[] = "sendrecv"; static const char kAttributeInactive[] = "inactive"; -// draft-ietf-mmusic-sctp-sdp-07 -// a=sctp-port +// draft-ietf-mmusic-sctp-sdp-26 +// a=sctp-port, a=max-message-size static const char kAttributeSctpPort[] = "sctp-port"; +static const char kAttributeMaxMessageSize[] = "max-message-size"; // draft-ietf-mmusic-sdp-simulcast-13 // a=simulcast static const char kAttributeSimulcast[] = "simulcast"; @@ -1240,7 +1241,7 @@ bool ParseIceOptions(const std::string& line, bool ParseSctpPort(const std::string& line, int* sctp_port, SdpParseError* error) { - // draft-ietf-mmusic-sctp-sdp-07 + // draft-ietf-mmusic-sctp-sdp-26 // a=sctp-port std::vector fields; const size_t expected_min_fields = 2; @@ -1258,6 +1259,23 @@ bool ParseSctpPort(const std::string& line, return true; } +bool ParseSctpMaxMessageSize(const std::string& line, + int* max_message_size, + SdpParseError* error) { + // draft-ietf-mmusic-sctp-sdp-26 + // a=max-message-size:199999 + std::vector fields; + const size_t expected_min_fields = 2; + rtc::split(line.substr(kLinePrefixLength), kSdpDelimiterColonChar, &fields); + if (fields.size() < expected_min_fields) { + return ParseFailedExpectMinFieldNum(line, expected_min_fields, error); + } + if (!rtc::FromString(fields[1], max_message_size)) { + return ParseFailed(line, "Invalid SCTP max message size.", error); + } + return true; +} + bool ParseExtmap(const std::string& line, RtpExtension* extmap, SdpParseError* error) { @@ -1815,18 +1833,40 @@ void AddRtcpFbLines(const T& codec, std::string* message) { } } -bool AddSctpDataCodec(DataContentDescription* media_desc, int sctp_port) { +cricket::DataCodec FindOrMakeSctpDataCodec(DataContentDescription* media_desc) { for (const auto& codec : media_desc->codecs()) { if (absl::EqualsIgnoreCase(codec.name, cricket::kGoogleSctpDataCodecName)) { - return ParseFailed("", "Can't have multiple sctp port attributes.", NULL); + return codec; } } - // Add the SCTP Port number as a pseudo-codec "port" parameter cricket::DataCodec codec_port(cricket::kGoogleSctpDataCodecPlType, cricket::kGoogleSctpDataCodecName); - codec_port.SetParam(cricket::kCodecParamPort, sctp_port); - RTC_LOG(INFO) << "AddSctpDataCodec: Got SCTP Port Number " << sctp_port; - media_desc->AddCodec(codec_port); + return codec_port; +} + +bool AddOrModifySctpDataCodecPort(DataContentDescription* media_desc, + int sctp_port) { + // Add the SCTP Port number as a pseudo-codec "port" parameter + auto codec = FindOrMakeSctpDataCodec(media_desc); + int dummy; + if (codec.GetParam(cricket::kCodecParamPort, &dummy)) { + return false; + } + codec.SetParam(cricket::kCodecParamPort, sctp_port); + media_desc->AddOrReplaceCodec(codec); + return true; +} + +bool AddOrModifySctpDataMaxMessageSize(DataContentDescription* media_desc, + int max_message_size) { + // Add the SCTP Max Message Size as a pseudo-parameter to the codec + auto codec = FindOrMakeSctpDataCodec(media_desc); + int dummy; + if (codec.GetParam(cricket::kCodecParamMaxMessageSize, &dummy)) { + return false; + } + codec.SetParam(cricket::kCodecParamMaxMessageSize, max_message_size); + media_desc->AddOrReplaceCodec(codec); return true; } @@ -2716,7 +2756,7 @@ bool ParseMediaDescription( if (data_desc && IsDtlsSctp(protocol)) { int p; if (rtc::FromString(fields[3], &p)) { - if (!AddSctpDataCodec(data_desc.get(), p)) { + if (!AddOrModifySctpDataCodecPort(data_desc.get(), p)) { return false; } } else if (fields[3] == kDefaultSctpmapProtocol) { @@ -3101,7 +3141,23 @@ bool ParseContent(const std::string& message, if (!ParseSctpPort(line, &sctp_port, error)) { return false; } - if (!AddSctpDataCodec(media_desc->as_data(), sctp_port)) { + if (!AddOrModifySctpDataCodecPort(media_desc->as_data(), sctp_port)) { + return false; + } + } else if (IsDtlsSctp(protocol) && + HasAttribute(line, kAttributeMaxMessageSize)) { + if (media_type != cricket::MEDIA_TYPE_DATA) { + return ParseFailed( + line, + "max-message-size attribute found in non-data media description.", + error); + } + int max_message_size; + if (!ParseSctpMaxMessageSize(line, &max_message_size, error)) { + return false; + } + if (!AddOrModifySctpDataMaxMessageSize(media_desc->as_data(), + max_message_size)) { return false; } } else if (IsRtp(protocol)) { diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc index a700a5a9c9..1fa7ba4b40 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -283,18 +283,19 @@ static const char kSdpSctpDataChannelString[] = "a=sctpmap:5000 webrtc-datachannel 1024\r\n"; // draft-ietf-mmusic-sctp-sdp-12 +// Note - this is invalid per draft-ietf-mmusic-sctp-sdp-26, +// since the separator after "sctp-port" needs to be a colon. static const char kSdpSctpDataChannelStringWithSctpPort[] = "m=application 9 DTLS/SCTP webrtc-datachannel\r\n" - "a=max-message-size=100000\r\n" "a=sctp-port 5000\r\n" "c=IN IP4 0.0.0.0\r\n" "a=ice-ufrag:ufrag_data\r\n" "a=ice-pwd:pwd_data\r\n" "a=mid:data_content_name\r\n"; +// draft-ietf-mmusic-sctp-sdp-26 static const char kSdpSctpDataChannelStringWithSctpColonPort[] = "m=application 9 DTLS/SCTP webrtc-datachannel\r\n" - "a=max-message-size=100000\r\n" "a=sctp-port:5000\r\n" "c=IN IP4 0.0.0.0\r\n" "a=ice-ufrag:ufrag_data\r\n" @@ -2863,6 +2864,48 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpColonPort) { EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output)); } +// Helper function to set the max-message-size parameter in the +// SCTP data codec. +void MutateJsepSctpMaxMessageSize(const SessionDescription& desc, + const std::string& new_value, + JsepSessionDescription* jdesc) { + cricket::SessionDescription* mutant = desc.Copy(); + DataContentDescription* dcdesc = + mutant->GetContentDescriptionByName(kDataContentName)->as_data(); + std::vector codecs(dcdesc->codecs()); + codecs[0].SetParam(cricket::kCodecParamMaxMessageSize, new_value); + dcdesc->set_codecs(codecs); + jdesc->Initialize(mutant, kSessionId, kSessionVersion); +} + +TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithMaxMessageSize) { + bool use_sctpmap = false; + AddSctpDataChannel(use_sctpmap); + JsepSessionDescription jdesc(kDummyType); + std::string sdp_with_data = kSdpString; + + sdp_with_data.append(kSdpSctpDataChannelStringWithSctpColonPort); + sdp_with_data.append("a=max-message-size:12345\r\n"); + MutateJsepSctpMaxMessageSize(desc_, "12345", &jdesc); + JsepSessionDescription jdesc_output(kDummyType); + + // Verify with DTLS/SCTP. + EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output)); + EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output)); + + // Verify with UDP/DTLS/SCTP. + sdp_with_data.replace(sdp_with_data.find(kDtlsSctp), strlen(kDtlsSctp), + kUdpDtlsSctp); + EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output)); + EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output)); + + // Verify with TCP/DTLS/SCTP. + sdp_with_data.replace(sdp_with_data.find(kUdpDtlsSctp), strlen(kUdpDtlsSctp), + kTcpDtlsSctp); + EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output)); + EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output)); +} + // Test to check the behaviour if sctp-port is specified // on the m= line and in a=sctp-port. TEST_F(WebRtcSdpTest, DeserializeSdpWithMultiSctpPort) {