Relax parsing of a=bundle-only with a nonzero port.
This use of a=bundle-only is unspecified, but not disallowed, so it
should simply result in the attribute being ignored, not a parse
failure.
Note that older versions of Firefox may generate SDP with a=bundle-only
and a nonzero port, so this also fixes an interop issue. See:
https://bugzilla.mozilla.org/show_bug.cgi?id=1325991
BUG=webrtc:4674
Review-Url: https://codereview.webrtc.org/2609863003
Cr-Commit-Position: refs/heads/master@{#15890}
diff --git a/webrtc/api/webrtcsdp.cc b/webrtc/api/webrtcsdp.cc
index f18653d..e5ee416 100644
--- a/webrtc/api/webrtcsdp.cc
+++ b/webrtc/api/webrtcsdp.cc
@@ -2372,14 +2372,17 @@
}
bool content_rejected = false;
+ // A port of 0 is not interpreted as a rejected m= section when it's
+ // used along with a=bundle-only.
if (bundle_only) {
- // A port of 0 is not interpreted as a rejected m= section when it's
- // used along with a=bundle-only.
if (!port_rejected) {
- return ParseFailed(
- "a=bundle-only",
- "a=bundle-only MUST only be used in combination with a 0 port.",
- error);
+ // Usage of bundle-only with a nonzero port is unspecified. So just
+ // ignore bundle-only if we see this.
+ bundle_only = false;
+ LOG(LS_WARNING)
+ << "a=bundle-only attribute observed with a nonzero "
+ << "port; this usage is unspecified so the attribute is being "
+ << "ignored.";
}
} else {
// If not using bundle-only, interpret port 0 in the normal way; the m=
diff --git a/webrtc/api/webrtcsdp_unittest.cc b/webrtc/api/webrtcsdp_unittest.cc
index 32f8bba..d69b135 100644
--- a/webrtc/api/webrtcsdp_unittest.cc
+++ b/webrtc/api/webrtcsdp_unittest.cc
@@ -3239,17 +3239,24 @@
TEST_F(WebRtcSdpTest, DeserializeBundleOnlyAttribute) {
MakeBundleOnlyDescription();
JsepSessionDescription deserialized_description(kDummyString);
- EXPECT_TRUE(
+ ASSERT_TRUE(
SdpDeserialize(kBundleOnlySdpFullString, &deserialized_description));
EXPECT_TRUE(CompareSessionDescription(jdesc_, deserialized_description));
}
-// "a=bundle-only" should only be used in combination with a 0 port on the m=
-// line. We should fail to parse anything else.
-TEST_F(WebRtcSdpTest, FailToDeserializeBundleOnlyWithNonzeroPort) {
- std::string bad_sdp = kBundleOnlySdpFullString;
- Replace("m=video 0", "m=video 9", &bad_sdp);
- ExpectParseFailure(bad_sdp, "a=bundle-only");
+// The semantics of "a=bundle-only" are only defined when it's used in
+// combination with a 0 port on the m= line. We should ignore it if used with a
+// nonzero port.
+TEST_F(WebRtcSdpTest, IgnoreBundleOnlyWithNonzeroPort) {
+ // Make the base bundle-only description but unset the bundle-only flag.
+ MakeBundleOnlyDescription();
+ jdesc_.description()->contents()[1].bundle_only = false;
+
+ std::string modified_sdp = kBundleOnlySdpFullString;
+ Replace("m=video 0", "m=video 9", &modified_sdp);
+ JsepSessionDescription deserialized_description(kDummyString);
+ ASSERT_TRUE(SdpDeserialize(modified_sdp, &deserialized_description));
+ EXPECT_TRUE(CompareSessionDescription(jdesc_, deserialized_description));
}
TEST_F(WebRtcSdpTest, SerializeBundleOnlyAttribute) {