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}

api/rtp_headers.h

@@ -39,7 +39,14 @@ class StringRtpHeaderExtension {
   // 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) {

call/rtcp_demuxer.cc

@@ -35,7 +35,7 @@ void RtcpDemuxer::AddSink(uint32_t sender_ssrc, RtcpPacketSinkInterface* sink) {
 
 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));

call/rtp_demuxer.cc

@@ -38,8 +38,8 @@ bool RtpDemuxer::AddSink(const RtpDemuxerCriteria& criteria,
                          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.mid.empty() || Mid::IsLegalMidName(criteria.mid));
-  RTC_DCHECK(criteria.rsid.empty() || StreamId::IsLegalName(criteria.rsid));
+  RTC_DCHECK(criteria.rsid.empty() || StreamId::IsLegalRsidName(criteria.rsid));
   RTC_DCHECK(sink);
 
   // We return false instead of DCHECKing for logical conflicts with the new

call/rtp_demuxer_unittest.cc

@@ -1491,9 +1491,9 @@ TEST_F(RtpDemuxerTest, RsidMustBeAlphaNumeric) {
   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) {

modules/rtp_rtcp/include/rtp_rtcp_defines.cc

@@ -16,7 +16,19 @@ StreamDataCounters::StreamDataCounters() : first_packet_time_ms(-1) {}
 
 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));
 }

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 @@ TEST_F(RtpRtcpAPITest, RtxSender) {
   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