Allow all "token" chars from RFC 4566 when checking for legal mid names.
Previously only alphanumeric characters were allowed.
Bug: webrtc:9537
Change-Id: I3fd793ad88520b25ecd884efe3a698f2f0af4639
Reviewed-on: https://webrtc-review.googlesource.com/89388
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24167}diff --git a/api/rtp_headers.h b/api/rtp_headers.h
index c0b22cc..c762534 100644
--- a/api/rtp_headers.h
+++ b/api/rtp_headers.h
@@ -39,7 +39,14 @@
// maximum length that can be encoded with one-byte header extensions.
static constexpr size_t kMaxSize = 16;
- static bool IsLegalName(rtc::ArrayView<const char> name);
+ static bool IsLegalMidName(rtc::ArrayView<const char> name);
+ static bool IsLegalRsidName(rtc::ArrayView<const char> name);
+
+ // TODO(bugs.webrtc.org/9537): Deprecate and remove when third parties have
+ // migrated to "IsLegalRsidName".
+ static bool IsLegalName(rtc::ArrayView<const char> name) {
+ return IsLegalRsidName(name);
+ }
StringRtpHeaderExtension() { value_[0] = 0; }
explicit StringRtpHeaderExtension(rtc::ArrayView<const char> value) {
diff --git a/call/rtcp_demuxer.cc b/call/rtcp_demuxer.cc
index 40c9163..d441c05 100644
--- a/call/rtcp_demuxer.cc
+++ b/call/rtcp_demuxer.cc
@@ -35,7 +35,7 @@
void RtcpDemuxer::AddSink(const std::string& rsid,
RtcpPacketSinkInterface* sink) {
- RTC_DCHECK(StreamId::IsLegalName(rsid));
+ RTC_DCHECK(StreamId::IsLegalRsidName(rsid));
RTC_DCHECK(sink);
RTC_DCHECK(!ContainerHasKey(broadcast_sinks_, sink));
RTC_DCHECK(!MultimapAssociationExists(rsid_sinks_, rsid, sink));
diff --git a/call/rtp_demuxer.cc b/call/rtp_demuxer.cc
index a8944b5..8f0e2e5 100644
--- a/call/rtp_demuxer.cc
+++ b/call/rtp_demuxer.cc
@@ -38,8 +38,8 @@
RtpPacketSinkInterface* sink) {
RTC_DCHECK(!criteria.payload_types.empty() || !criteria.ssrcs.empty() ||
!criteria.mid.empty() || !criteria.rsid.empty());
- RTC_DCHECK(criteria.mid.empty() || Mid::IsLegalName(criteria.mid));
- RTC_DCHECK(criteria.rsid.empty() || StreamId::IsLegalName(criteria.rsid));
+ RTC_DCHECK(criteria.mid.empty() || Mid::IsLegalMidName(criteria.mid));
+ RTC_DCHECK(criteria.rsid.empty() || StreamId::IsLegalRsidName(criteria.rsid));
RTC_DCHECK(sink);
// We return false instead of DCHECKing for logical conflicts with the new
diff --git a/call/rtp_demuxer_unittest.cc b/call/rtp_demuxer_unittest.cc
index 76a13ee..b5ede3c 100644
--- a/call/rtp_demuxer_unittest.cc
+++ b/call/rtp_demuxer_unittest.cc
@@ -1491,9 +1491,9 @@
EXPECT_DEATH(AddSinkOnlyRsid("a_3", &sink), "");
}
-TEST_F(RtpDemuxerTest, MidMustBeAlphaNumeric) {
+TEST_F(RtpDemuxerTest, MidMustBeToken) {
MockRtpPacketSink sink;
- EXPECT_DEATH(AddSinkOnlyMid("a_3", &sink), "");
+ EXPECT_DEATH(AddSinkOnlyMid("a(3)", &sink), "");
}
TEST_F(RtpDemuxerTest, RsidMustNotExceedMaximumLength) {
diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.cc b/modules/rtp_rtcp/include/rtp_rtcp_defines.cc
index ad9bd45..f86b238 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp_defines.cc
+++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.cc
@@ -16,7 +16,19 @@
constexpr size_t StreamId::kMaxSize;
-bool StreamId::IsLegalName(rtc::ArrayView<const char> name) {
+// Check if passed character is a "token-char" from RFC 4566.
+static bool IsTokenChar(char ch) {
+ return ch == 0x21 || (ch >= 0x23 && ch <= 0x27) || ch == 0x2a || ch == 0x2b ||
+ ch == 0x2d || ch == 0x2e || (ch >= 0x30 && ch <= 0x39) ||
+ (ch >= 0x41 && ch <= 0x5a) || (ch >= 0x5e && ch <= 0x7e);
+}
+
+bool StreamId::IsLegalMidName(rtc::ArrayView<const char> name) {
+ return (name.size() <= kMaxSize && name.size() > 0 &&
+ std::all_of(name.data(), name.data() + name.size(), IsTokenChar));
+}
+
+bool StreamId::IsLegalRsidName(rtc::ArrayView<const char> name) {
return (name.size() <= kMaxSize && name.size() > 0 &&
std::all_of(name.data(), name.data() + name.size(), isalnum));
}
diff --git a/modules/rtp_rtcp/test/testAPI/test_api.cc b/modules/rtp_rtcp/test/testAPI/test_api.cc
index fd5067b..b11a18b 100644
--- a/modules/rtp_rtcp/test/testAPI/test_api.cc
+++ b/modules/rtp_rtcp/test/testAPI/test_api.cc
@@ -12,6 +12,7 @@
#include <algorithm>
#include <memory>
+#include <string>
#include <vector>
#include "rtc_base/checks.h"
@@ -160,4 +161,54 @@
EXPECT_EQ(kRtxRetransmitted, module_->RtxSendStatus());
}
+TEST_F(RtpRtcpAPITest, LegalMidName) {
+ static const std::string kLegalMidNames[] = {
+ // clang-format off
+ "audio",
+ "audio0",
+ "audio_0",
+ // clang-format on
+ };
+ for (const auto& name : kLegalMidNames) {
+ EXPECT_TRUE(StreamId::IsLegalMidName(name))
+ << "Mid should be legal: " << name;
+ }
+
+ static const std::string kNonLegalMidNames[] = {
+ // clang-format off
+ "",
+ "(audio0)",
+ // clang-format on
+ };
+ for (const auto& name : kNonLegalMidNames) {
+ EXPECT_FALSE(StreamId::IsLegalMidName(name))
+ << "Mid should not be legal: " << name;
+ }
+}
+
+TEST_F(RtpRtcpAPITest, LegalRsidName) {
+ static const std::string kLegalRsidNames[] = {
+ // clang-format off
+ "audio",
+ "audio0",
+ // clang-format on
+ };
+ for (const auto& name : kLegalRsidNames) {
+ EXPECT_TRUE(StreamId::IsLegalRsidName(name))
+ << "Rsid should be legal: " << name;
+ }
+
+ static const std::string kNonLegalRsidNames[] = {
+ // clang-format off
+ "",
+ "audio_0",
+ "(audio0)",
+ // clang-format on
+ };
+ for (const auto& name : kNonLegalRsidNames) {
+ EXPECT_FALSE(StreamId::IsLegalRsidName(name))
+ << "Rsid should not be legal: " << name;
+ }
+}
+
} // namespace webrtc