Bug 1427009 - limit accepted mid length to 16 chars. r?bwc draft
authorMichael Froman <mfroman@mozilla.com>
Mon, 08 Jan 2018 16:23:06 -0600
changeset 717504 8872de82ddb4d7a8ea1ddf6fea8b3cf2268cb2d9
parent 717422 05fed903f40f05fd923ba2137696ecc1fa0bafe6
child 745281 3de677b9ab9abd6a4dc7b53f6f8cf818ed076749
push id94706
push userbmo:mfroman@nostrum.com
push dateTue, 09 Jan 2018 04:58:08 +0000
reviewersbwc
bugs1427009
milestone59.0a1
Bug 1427009 - limit accepted mid length to 16 chars. r?bwc webrtc.org only supports one-byte rtp header extensions which means we can only support 16 character mids for now. MozReview-Commit-ID: C7aTeB5Bi2M
dom/media/tests/mochitest/mochitest.ini
dom/media/tests/mochitest/test_peerConnection_basicAudioVideoVerifyTooLongMidFails.html
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
--- a/dom/media/tests/mochitest/mochitest.ini
+++ b/dom/media/tests/mochitest/mochitest.ini
@@ -131,16 +131,18 @@ skip-if = toolkit == 'android'  # Bug 11
 [test_peerConnection_basicAudioVideoNoBundle.html]
 skip-if = (android_version == '18') # android(Bug 1189784, timeouts on 4.3 emulator)
 [test_peerConnection_basicAudioVideoNoBundleNoRtcpMux.html]
 skip-if = android_version == '18' # android(Bug 1189784, timeouts on 4.3 emulator)
 [test_peerConnection_basicAudioVideoNoRtcpMux.html]
 skip-if = android_version == '18' # android(Bug 1189784, timeouts on 4.3 emulator)
 [test_peerConnection_basicAudioVideoTransceivers.html]
 skip-if = android_version == '18' # android(Bug 1189784, timeouts on 4.3 emulator)
+[test_peerConnection_basicAudioVideoVerifyTooLongMidFails.html]
+skip-if = (android_version == '18') # android(Bug 1189784, timeouts on 4.3 emulator)
 [test_peerConnection_basicVideo.html]
 skip-if = (android_version == '18' && debug) # android(Bug 1189784, timeouts on 4.3 emulator)
 [test_peerConnection_basicVideoVerifyRtpHeaderExtensions.html]
 skip-if = (android_version == '18') # android(Bug 1189784, timeouts on 4.3 emulator)
 [test_peerConnection_basicScreenshare.html]
 # frequent timeouts/crashes on e10s (bug 1048455)
 skip-if = toolkit == 'android' # no screenshare on android
 [test_peerConnection_basicWindowshare.html]
new file mode 100644
--- /dev/null
+++ b/dom/media/tests/mochitest/test_peerConnection_basicAudioVideoVerifyTooLongMidFails.html
@@ -0,0 +1,45 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <script type="application/javascript" src="pc.js"></script>
+</head>
+<body>
+<pre id="test">
+<script type="application/javascript">
+  createHTML({
+    bug: "1427009",
+    title: "Test mid longer than 16 characters fails"
+  });
+
+  var test;
+  runNetworkTest(function (options) {
+    options = options || { };
+    options.bundle = false;
+    test = new PeerConnectionTest(options);
+    test.setMediaConstraints([{audio: true}, {video: true}],
+                             [{audio: true}, {video: true}]);
+
+    test.chain.replaceAfter("PC_LOCAL_CREATE_OFFER",
+      [
+        function PC_LOCAL_MUNGE_OFFER_SDP(test) {
+          test.originalOffer.sdp =
+            test.originalOffer.sdp.replace(/a=mid:.*\r\n/g,
+                                           "a=mid:really_long_mid_over_16_chars\r\n");
+        },
+        function PC_LOCAL_EXPECT_SET_LOCAL_DESCRIPTION_FAIL(test) {
+          return test.setLocalDescription(test.pcLocal,
+                                          test.originalOffer,
+                                          HAVE_LOCAL_OFFER)
+           .then(() => ok(false, "setLocalDescription must fail"),
+                 e => is(e.name, "InvalidSessionDescriptionError",
+                         "setLocalDescription must fail and did"));
+        }
+      ], 0 // first occurance
+    );
+
+    test.run();
+  });
+</script>
+</pre>
+</body>
+</html>
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -1126,19 +1126,24 @@ JsepSessionImpl::InitTransport(const Sdp
   } else {
     transport->mComponents = 1;
   }
 
   if (msection.GetAttributeList().HasAttribute(SdpAttribute::kMidAttribute)) {
     transport->mTransportId = msection.GetAttributeList().GetMid();
   } else {
     std::ostringstream os;
-    os << "level_" << msection.GetLevel() << "(no mid)";
+    os << "no_mid_lvl_" << msection.GetLevel();
+    // This works providing we don't have an msection level higher than 99999.
+    // We need to fit inside the 16 character mid limitation that results from
+    // not having two-byte rtp header extensions support in webrtc.org yet.
     transport->mTransportId = os.str();
   }
+  // This assert can go away when webrtc.org supports 2-byte rtp header exts.
+  MOZ_ASSERT(transport->mTransportId.length() <= 16);
 }
 
 nsresult
 JsepSessionImpl::FinalizeTransport(const SdpAttributeList& remote,
                                    const SdpAttributeList& answer,
                                    const RefPtr<JsepTransport>& transport)
 {
   UniquePtr<JsepIceTransport> ice = MakeUnique<JsepIceTransport>();
@@ -1284,16 +1289,24 @@ JsepSessionImpl::ParseSdp(const std::str
     if (mSdpHelper.MsectionIsDisabled(parsed->GetMediaSection(i))) {
       // Disabled, let this stuff slide.
       continue;
     }
 
     const SdpMediaSection& msection(parsed->GetMediaSection(i));
     auto& mediaAttrs = msection.GetAttributeList();
 
+    if (mediaAttrs.HasAttribute(SdpAttribute::kMidAttribute) &&
+        mediaAttrs.GetMid().length() > 16) {
+      JSEP_SET_ERROR("Invalid description, mid length greater than 16 "
+                     "unsupported until 2-byte rtp header extensions are "
+                     "supported in webrtc.org");
+      return NS_ERROR_INVALID_ARG;
+    }
+
     if (mediaAttrs.GetIceUfrag().empty()) {
       JSEP_SET_ERROR("Invalid description, no ice-ufrag attribute");
       return NS_ERROR_INVALID_ARG;
     }
 
     if (mediaAttrs.GetIcePwd().empty()) {
       JSEP_SET_ERROR("Invalid description, no ice-pwd attribute");
       return NS_ERROR_INVALID_ARG;