Bug 1325991 - sections with bundle-only should have port set to 0. r=drno draft
authorMichael Froman <mfroman@mozilla.com>
Fri, 30 Dec 2016 13:29:51 -0600
changeset 454936 f5cc3a32d1a6805b42e0f9730d2f91b43cb87061
parent 453266 2785aaf276ba29fb2e1f5607d90d441fee42efb4
child 540849 9b52daa38304463b305e87e8ab23e7f0211d19a5
push id40085
push userbmo:mfroman@nostrum.com
push dateFri, 30 Dec 2016 19:42:03 +0000
reviewersdrno
bugs1325991
milestone53.0a1
Bug 1325991 - sections with bundle-only should have port set to 0. r=drno MozReview-Commit-ID: 6O7X1MWZhZI
media/webrtc/signaling/gtest/jsep_session_unittest.cpp
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
media/webrtc/signaling/src/sdp/SdpHelper.cpp
--- a/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
+++ b/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
@@ -819,31 +819,36 @@ protected:
         }
       }
 
       void CheckDefaultRtpCandidate(bool expectDefault,
                                     const SdpMediaSection& msection,
                                     size_t transportLevel,
                                     const std::string& context) const
       {
+        Address expectedAddress = "0.0.0.0";
+        Port expectedPort = 9U;
+
         if (expectDefault) {
           // Copy so we can be terse and use []
           auto defaultCandidates = mDefaultCandidates;
-          ASSERT_EQ(defaultCandidates[transportLevel][RTP].first,
-                    msection.GetConnection().GetAddress())
-            << context << " (level " << msection.GetLevel() << ")";
-          ASSERT_EQ(defaultCandidates[transportLevel][RTP].second,
-                    msection.GetPort())
-            << context << " (level " << msection.GetLevel() << ")";
-        } else {
-          ASSERT_EQ("0.0.0.0", msection.GetConnection().GetAddress())
-            << context << " (level " << msection.GetLevel() << ")";
-          ASSERT_EQ(9U, msection.GetPort())
-            << context << " (level " << msection.GetLevel() << ")";
+          expectedAddress = defaultCandidates[transportLevel][RTP].first;
+          expectedPort = defaultCandidates[transportLevel][RTP].second;
         }
+
+        // if bundle-only attribute is present, expect port 0
+        const SdpAttributeList& attrs = msection.GetAttributeList();
+        if (attrs.HasAttribute(SdpAttribute::kBundleOnlyAttribute)) {
+          expectedPort = 0U;
+        }
+
+        ASSERT_EQ(expectedAddress, msection.GetConnection().GetAddress())
+          << context << " (level " << msection.GetLevel() << ")";
+        ASSERT_EQ(expectedPort, msection.GetPort())
+          << context << " (level " << msection.GetLevel() << ")";
       }
 
       void CheckDefaultRtcpCandidate(bool expectDefault,
                                      const SdpMediaSection& msection,
                                      size_t transportLevel,
                                      const std::string& context) const
       {
         if (expectDefault) {
@@ -1069,21 +1074,25 @@ private:
       auto& msection = sdp->GetMediaSection(i);
 
       if (msection.GetMediaType() == SdpMediaSection::kApplication) {
         ASSERT_EQ(SdpMediaSection::kDtlsSctp, msection.GetProtocol());
       } else {
         ASSERT_EQ(SdpMediaSection::kUdpTlsRtpSavpf, msection.GetProtocol());
       }
 
-      if (msection.GetPort() == 0) {
+      const SdpAttributeList& attrs = msection.GetAttributeList();
+      bool bundle_only = attrs.HasAttribute(SdpAttribute::kBundleOnlyAttribute);
+
+      // port 0 only means disabled when the bundle-only attribute is missing
+      if (!bundle_only && msection.GetPort() == 0) {
         ValidateDisabledMSection(&msection);
         continue;
       }
-      const SdpAttributeList& attrs = msection.GetAttributeList();
+
       ASSERT_EQ(source.mIceUfrag, attrs.GetIceUfrag());
       ASSERT_EQ(source.mIcePwd, attrs.GetIcePwd());
       const SdpFingerprintAttributeList& fps = attrs.GetFingerprint();
       for (auto fp = fps.mFingerprints.begin(); fp != fps.mFingerprints.end();
            ++fp) {
         std::string alg_str = "None";
 
         if (fp->hashFunc == SdpFingerprintAttributeList::kSha1) {
@@ -4147,20 +4156,22 @@ TEST_P(JsepSessionTest, TestMaxBundle)
   std::string offer = mSessionOff.GetLocalDescription();
   SipccSdpParser parser;
   UniquePtr<Sdp> parsedOffer = parser.Parse(offer);
   ASSERT_TRUE(parsedOffer.get());
 
   ASSERT_FALSE(
       parsedOffer->GetMediaSection(0).GetAttributeList().HasAttribute(
         SdpAttribute::kBundleOnlyAttribute));
+  ASSERT_NE(0U, parsedOffer->GetMediaSection(0).GetPort());
   for (size_t i = 1; i < parsedOffer->GetMediaSectionCount(); ++i) {
     ASSERT_TRUE(
         parsedOffer->GetMediaSection(i).GetAttributeList().HasAttribute(
           SdpAttribute::kBundleOnlyAttribute));
+    ASSERT_EQ(0U, parsedOffer->GetMediaSection(i).GetPort());
   }
 
 
   CheckPairs(mSessionOff, "Offerer pairs");
   CheckPairs(mSessionAns, "Answerer pairs");
   EXPECT_EQ(1U, GetActiveTransportCount(mSessionOff));
   EXPECT_EQ(1U, GetActiveTransportCount(mSessionAns));
 }
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -597,16 +597,18 @@ JsepSessionImpl::SetupBundle(Sdp* sdp) c
           // m-section
           useBundleOnly = !mids.empty();
           break;
       }
 
       if (useBundleOnly) {
         attrs.SetAttribute(
             new SdpFlagAttribute(SdpAttribute::kBundleOnlyAttribute));
+        // Set port to 0 for sections with bundle-only attribute. (mjf)
+        sdp->GetMediaSection(i).SetPort(0);
       }
 
       mids.push_back(attrs.GetMid());
     }
   }
 
   if (mids.size() > 1) {
     UniquePtr<SdpGroupAttributeList> groupAttr(new SdpGroupAttributeList);
--- a/media/webrtc/signaling/src/sdp/SdpHelper.cpp
+++ b/media/webrtc/signaling/src/sdp/SdpHelper.cpp
@@ -419,24 +419,29 @@ SdpHelper::SetDefaultAddresses(const std
 void
 SdpHelper::SetDefaultAddresses(const std::string& defaultCandidateAddr,
                                uint16_t defaultCandidatePort,
                                const std::string& defaultRtcpCandidateAddr,
                                uint16_t defaultRtcpCandidatePort,
                                SdpMediaSection* msection)
 {
   msection->GetConnection().SetAddress(defaultCandidateAddr);
-  msection->SetPort(defaultCandidatePort);
+  SdpAttributeList& attrList = msection->GetAttributeList();
+
+  // only set the port if there is no bundle-only attribute
+  if (!attrList.HasAttribute(SdpAttribute::kBundleOnlyAttribute)) {
+    msection->SetPort(defaultCandidatePort);
+  }
 
   if (!defaultRtcpCandidateAddr.empty()) {
     sdp::AddrType ipVersion = sdp::kIPv4;
     if (defaultRtcpCandidateAddr.find(':') != std::string::npos) {
       ipVersion = sdp::kIPv6;
     }
-    msection->GetAttributeList().SetAttribute(new SdpRtcpAttribute(
+    attrList.SetAttribute(new SdpRtcpAttribute(
           defaultRtcpCandidatePort,
           sdp::kInternet,
           ipVersion,
           defaultRtcpCandidateAddr));
   }
 }
 
 nsresult