Bug 1401592: Common rid validation logic for SDP and JS. r=mjf

Includes a configured max rid length, since it is not very reasonable to assume
that rids of length 255 are supported by the RTP engine, regardless of what
the IETF specs say.

Differential Revision: https://phabricator.services.mozilla.com/D156832

dom/media/webrtc/jsapi/PeerConnectionCtx.cpp

@@ -6,6 +6,7 @@
 
 #include "api/audio/audio_mixer.h"
 #include "api/audio_codecs/builtin_audio_decoder_factory.h"
+#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
 #include "call/audio_state.h"
 #include "common/browser_logging/CSFLog.h"
 #include "common/browser_logging/WebRtcLog.h"
@@ -490,7 +491,8 @@ void PeerConnectionCtx::ForEachPeerConnection(Function&& aFunction) const {
 
 nsresult PeerConnectionCtx::Initialize() {
   initGMP();
-
+  SdpRidAttributeList::kMaxRidLength =
+      webrtc::BaseRtpStringExtension::kMaxValueSizeBytes;
   nsresult rv = NS_NewTimerWithFuncCallback(
       getter_AddRefs(mTelemetryTimer), EverySecondTelemetryCallback_m, this,
       1000, nsITimer::TYPE_REPEATING_PRECISE_CAN_SKIP,

dom/media/webrtc/jsep/JsepTrack.cpp

@@ -199,7 +199,13 @@ void JsepTrack::SendTrackSetRemote(SsrcGenerator& aSsrcGenerator,
   if (mRids.empty()) {
     // Initial configuration
     for (const auto& ridAttr : rids) {
-      mRids.push_back(ridAttr.id);
+      // The sipcc-based parser will detect this problem earlier on, but right
+      // now the rust-based parser will not. So, we do a little bit of
+      // belt-and-suspenders here.
+      std::string dummy;
+      if (SdpRidAttributeList::CheckRidValidity(ridAttr.id, &dummy)) {
+        mRids.push_back(ridAttr.id);
+      }
     }
     if (mRids.size() > mMaxEncodings) {
       mRids.resize(mMaxEncodings);

dom/media/webrtc/sdp/SdpAttribute.cpp

@@ -7,6 +7,7 @@
 #include "sdp/SdpAttribute.h"
 #include "sdp/SdpHelper.h"
 #include <iomanip>
+#include <bitset>
 
 #ifdef CRLF
 #  undef CRLF
@@ -925,7 +926,7 @@ void SdpRidAttributeList::Rid::SerializeParameters(std::ostream& os) const {
 // Remove this function. See Bug 1469702
 bool SdpRidAttributeList::Rid::Parse(std::istream& is, std::string* error) {
   id = ParseToken(is, " ", error);
-  if (id.empty()) {
+  if (!CheckRidValidity(id, error)) {
     return false;
   }
 
@@ -943,6 +944,65 @@ bool SdpRidAttributeList::Rid::Parse(std::istream& is, std::string* error) {
   return ParseParameters(is, error);
 }
 
+static std::bitset<256> GetAllowedRidCharacters() {
+  // From RFC 8851:
+  // rid-id            = 1*(alpha-numeric / "-" / "_")
+  std::bitset<256> result;
+  for (unsigned char c = 'a'; c <= 'z'; ++c) {
+    result.set(c);
+  }
+  for (unsigned char c = 'A'; c <= 'Z'; ++c) {
+    result.set(c);
+  }
+  for (unsigned char c = '0'; c <= '9'; ++c) {
+    result.set(c);
+  }
+  // NOTE: RFC 8851 says these are allowed, but RFC 8852 says they are not
+  // https://www.rfc-editor.org/errata/eid7132
+  // result.set('-');
+  // result.set('_');
+  return result;
+}
+
+/* static */
+bool SdpRidAttributeList::CheckRidValidity(const std::string& aRid,
+                                           std::string* aError) {
+  if (aRid.empty()) {
+    *aError = "Rid must be non-empty (according to RFC 8851)";
+    return false;
+  }
+
+  // We need to check against a maximum length, but that is nowhere
+  // specified in webrtc-pc right now.
+  if (aRid.size() > 255) {
+    *aError = "Rid can be at most 255 characters long (according to RFC 8852)";
+    return false;
+  }
+
+  if (aRid.size() > kMaxRidLength) {
+    std::ostringstream ss;
+    ss << "Rid can be at most " << kMaxRidLength
+       << " characters long (due to internal limitations)";
+    *aError = ss.str();
+    return false;
+  }
+
+  static const std::bitset<256> allowed = GetAllowedRidCharacters();
+  for (unsigned char c : aRid) {
+    if (!allowed[c]) {
+      *aError =
+          "Rid can contain only alphanumeric characters (according to RFC "
+          "8852)";
+      return false;
+    }
+  }
+
+  return true;
+}
+
+// This can be overridden if necessary
+size_t SdpRidAttributeList::kMaxRidLength = 255;
+
 void SdpRidAttributeList::Rid::Serialize(std::ostream& os) const {
   os << id << " " << direction;
   SerializeParameters(os);
@@ -1096,6 +1156,9 @@ bool SdpSimulcastAttribute::Version::Parse(std::istream& is,
       *error = "Missing rid";
       return false;
     }
+    if (!SdpRidAttributeList::CheckRidValidity(value, error)) {
+      return false;
+    }
     choices.push_back(Encoding(value, paused));
   } while (SkipChar(is, ',', error));
 

dom/media/webrtc/sdp/SdpAttribute.h

@@ -952,6 +952,9 @@ class SdpRidAttributeList : public SdpAttribute {
     return new SdpRidAttributeList(*this);
   }
 
+  static bool CheckRidValidity(const std::string& aRid, std::string* aError);
+  static size_t kMaxRidLength;
+
   virtual void Serialize(std::ostream& os) const override;
 
   // Remove this function. See Bug 1469702

media/webrtc/signaling/gtest/sdp_unittests.cpp

@@ -5166,20 +5166,6 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
     ASSERT_EQ(0U, rid.dependIds.size());
   }
 
-  {
-    SdpRidAttributeList::Rid rid(ParseRid("0123456789az-_ recv max-width=800"));
-    ASSERT_EQ("0123456789az-_", rid.id);
-    ASSERT_EQ(sdp::kRecv, rid.direction);
-    ASSERT_EQ(0U, rid.formats.size());
-    ASSERT_EQ(800U, rid.constraints.maxWidth);
-    ASSERT_EQ(0U, rid.constraints.maxHeight);
-    ASSERT_FALSE(rid.constraints.maxFps.isSome());
-    ASSERT_EQ(0U, rid.constraints.maxFs);
-    ASSERT_EQ(0U, rid.constraints.maxBr);
-    ASSERT_EQ(0U, rid.constraints.maxPps);
-    ASSERT_EQ(0U, rid.dependIds.size());
-  }
-
   {
     SdpRidAttributeList::Rid rid(ParseRid("foo send"));
     ASSERT_EQ(0U, rid.formats.size());
@@ -5430,6 +5416,7 @@ TEST(NewSdpTestNoFixture, CheckRidInvalidParse)
   ParseInvalid<SdpRidAttributeList::Rid>("foo send depend=", 16);
   ParseInvalid<SdpRidAttributeList::Rid>("foo send depend=,", 16);
   ParseInvalid<SdpRidAttributeList::Rid>("foo send depend=1,", 18);
+  ParseInvalid<SdpRidAttributeList::Rid>("0123456789az-_", 14);
 }
 
 TEST(NewSdpTestNoFixture, CheckRidSerialize)