Accept SDP with TRANSPORT attributes missing from bundled m= sections.

webrtc/api/webrtcsession.cc

 /*
  *  Copyright 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 146 matching lines @@
         static_cast<const MediaContentDescription*>(
             answer->contents()[i].description);
     if (offer_mdesc->type() != answer_mdesc->type()) {
       return false;
     }
   }
   return true;
 }
 
 // Checks that each non-rejected content has SDES crypto keys or a DTLS
-// fingerprint. Mismatches, such as replying with a DTLS fingerprint to SDES
-// keys, will be caught in Transport negotiation, and backstopped by Channel's
-// |srtp_required| check.
+// fingerprint, unless it's in a BUNDLE group, in which case only the
+// BUNDLE-tag section needs a ufrag and pwd. Mismatches, such as replying with

[pthatcher1, 2017/02/17 19:17:32]

I translation from spec speak to understandable text would be nice, perhaps
something like:

BUNDLE-tag section (the first media section of the BUNDLE group)

[Taylor Brandstetter, 2017/02/17 21:49:14]

Done.

+// a DTLS fingerprint to SDES keys, will be caught in JsepTransport
+// negotiation, and backstopped by Channel's |srtp_required| check.
 static bool VerifyCrypto(const SessionDescription* desc,
                          bool dtls_enabled,
                          std::string* error) {
+  const cricket::ContentGroup* bundle =
+      desc->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
   const ContentInfos& contents = desc->contents();
   for (size_t index = 0; index < contents.size(); ++index) {
     const ContentInfo* cinfo = &contents[index];
     if (cinfo->rejected) {
       continue;
     }
+    if (bundle && bundle->HasContentName(cinfo->name) &&
+        cinfo->name != *(bundle->FirstContentName())) {
+      continue;

[pthatcher1, 2017/02/17 19:17:32]

A comment here like "It's not the first, so it spec-compliant sections may not
have any attributes".

[Taylor Brandstetter, 2017/02/17 21:49:14]

Done.

+    }
 
-    // If the content isn't rejected, crypto must be present.
+    // If the content isn't rejected or bundled into another m= section, crypto
+    // must be present.
     const MediaContentDescription* media =
         static_cast<const MediaContentDescription*>(cinfo->description);
     const TransportInfo* tinfo = desc->GetTransportInfoByName(cinfo->name);
     if (!media || !tinfo) {
       // Something is not right.
       LOG(LS_ERROR) << kInvalidSdp;
       *error = kInvalidSdp;
       return false;
     }
     if (dtls_enabled) {
       if (!tinfo->description.identity_fingerprint) {
         LOG(LS_WARNING) <<
             "Session description must have DTLS fingerprint if DTLS enabled.";
         *error = kSdpWithoutDtlsFingerprint;
         return false;
       }
     } else {
       if (media->cryptos().empty()) {
         LOG(LS_WARNING) <<
             "Session description must have SDES when DTLS disabled.";
         *error = kSdpWithoutSdesCrypto;
         return false;
       }
     }
   }
 
   return true;
 }
 
-// Checks that each non-rejected content has ice-ufrag and ice-pwd set.
+// Checks that each non-rejected content has ice-ufrag and ice-pwd set, unless
+// it's in a BUNDLE group, in which case only the BUNDLE-tag section needs a
+// ufrag and pwd.

[pthatcher1, 2017/02/17 19:17:32]

Like the comment above, please translate "BUNDLE-tag section" (that might not be
the worse name I've seen from a spec, but it's up there).

[Taylor Brandstetter, 2017/02/17 21:49:14]

Done.

 static bool VerifyIceUfragPwdPresent(const SessionDescription* desc) {
+  const cricket::ContentGroup* bundle =
+      desc->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
   const ContentInfos& contents = desc->contents();
   for (size_t index = 0; index < contents.size(); ++index) {
     const ContentInfo* cinfo = &contents[index];
     if (cinfo->rejected) {
       continue;
     }
+    if (bundle && bundle->HasContentName(cinfo->name) &&
+        cinfo->name != *(bundle->FirstContentName())) {
+      continue;
+    }
 
-    // If the content isn't rejected, ice-ufrag and ice-pwd must be present.
+    // If the content isn't rejected or bundled into another m= section,
+    // ice-ufrag and ice-pwd must be present.
     const TransportInfo* tinfo = desc->GetTransportInfoByName(cinfo->name);
     if (!tinfo) {
       // Something is not right.
       LOG(LS_ERROR) << kInvalidSdp;
       return false;
     }
     if (tinfo->description.ice_ufrag.empty() ||
         tinfo->description.ice_pwd.empty()) {
       LOG(LS_ERROR) << "Session description must have ice ufrag and pwd.";
       return false;
@@ skipping 2195 matching lines @@
   bool need_to_delete_rtcp = (rtp_data_channel_->rtcp_transport() != nullptr);
   channel_manager_->DestroyRtpDataChannel(rtp_data_channel_.release());
   transport_controller_->DestroyTransportChannel(
       transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP);
   if (need_to_delete_rtcp) {
     transport_controller_->DestroyTransportChannel(
         transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP);
   }
 }
 }  // namespace webrtc