sdp: measure rtp header extension collisions
since extension ids are required to be unique in a BUNDLE group:
https://www.rfc-editor.org/rfc/rfc8843#name-rtp-header-extensions-consi
Measure how much enforcing this would break in UMA first.
BUG=webrtc:14782
Change-Id: Ieaf7a436feea677032499e11ca14973eebda322e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288362
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38914}
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 84a0756..d0bd3c8 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -491,6 +491,52 @@
return RTCError::OK();
}
+RTCError FindDuplicateHeaderExtensionIds(
+ const RtpExtension extension,
+ std::map<int, RtpExtension>& id_to_extension) {
+ auto existing_extension = id_to_extension.find(extension.id);
+ if (existing_extension != id_to_extension.end() &&
+ extension != existing_extension->second) {
+ return RTCError(
+ RTCErrorType::INVALID_PARAMETER,
+ "A BUNDLE group contains a codec collision for "
+ "header extension id='" +
+ rtc::ToString(extension.id) +
+ ". The id must be the same across all bundled media descriptions");
+ }
+ id_to_extension.insert(std::make_pair(extension.id, extension));
+ return RTCError::OK();
+}
+
+RTCError ValidateBundledRtpHeaderExtensions(
+ const cricket::SessionDescription& description) {
+ // https://www.rfc-editor.org/rfc/rfc8843#name-rtp-header-extensions-consi
+ // ... the identifier used for a given extension MUST identify the same
+ // extension across all the bundled media descriptions.
+ std::vector<const cricket::ContentGroup*> bundle_groups =
+ description.GetGroupsByName(cricket::GROUP_TYPE_BUNDLE);
+ for (const cricket::ContentGroup* bundle_group : bundle_groups) {
+ std::map<int, RtpExtension> id_to_extension;
+ for (const std::string& content_name : bundle_group->content_names()) {
+ const cricket::MediaContentDescription* media_description =
+ description.GetContentDescriptionByName(content_name);
+ if (!media_description) {
+ return RTCError(RTCErrorType::INVALID_PARAMETER,
+ "A BUNDLE group contains a MID='" + content_name +
+ "' matching no m= section.");
+ }
+ for (const auto& extension : media_description->rtp_header_extensions()) {
+ auto error =
+ FindDuplicateHeaderExtensionIds(extension, id_to_extension);
+ if (!error.ok()) {
+ return error;
+ }
+ }
+ }
+ }
+ return RTCError::OK();
+}
+
bool IsValidOfferToReceiveMedia(int value) {
typedef PeerConnectionInterface::RTCOfferAnswerOptions Options;
return (value >= Options::kUndefined) &&
@@ -3371,12 +3417,18 @@
return RTCError(RTCErrorType::INVALID_PARAMETER, kSdpWithoutIceUfragPwd);
}
- // Validate bundle, payload types and that there are no collisions.
+ // Validate that there are no collisions of bundled payload types.
error = ValidateBundledPayloadTypes(*sdesc->description());
// TODO(bugs.webrtc.org/14420): actually reject.
RTC_HISTOGRAM_BOOLEAN("WebRTC.PeerConnection.ValidBundledPayloadTypes",
error.ok());
+ // Validate that there are no collisions of bundled header extensions ids.
+ error = ValidateBundledRtpHeaderExtensions(*sdesc->description());
+ // TODO(bugs.webrtc.org/14782): actually reject.
+ RTC_HISTOGRAM_BOOLEAN("WebRTC.PeerConnection.ValidBundledExtensionIds",
+ error.ok());
+
if (!pc_->ValidateBundleSettings(sdesc->description(),
bundle_groups_by_mid)) {
return RTCError(RTCErrorType::INVALID_PARAMETER, kBundleWithoutRtcpMux);
diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index ecac7f8..bdd3301 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc
@@ -238,6 +238,130 @@
"WebRTC.PeerConnection.ValidBundledPayloadTypes", false));
}
+TEST_F(SdpOfferAnswerTest, BundleMeasuresHeaderExtensionIdCollision) {
+ auto pc = CreatePeerConnection();
+ std::string sdp =
+ "v=0\r\n"
+ "o=- 0 3 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "a=group:BUNDLE 0 1\r\n"
+ "a=fingerprint:sha-1 "
+ "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
+ "a=setup:actpass\r\n"
+ "a=ice-ufrag:ETEn\r\n"
+ "a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
+ "m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp-mux\r\n"
+ "a=sendonly\r\n"
+ "a=mid:0\r\n"
+ "a=rtpmap:111 opus/48000/2\r\n"
+ "a=extmap:3 "
+ "http://www.ietf.org/id/"
+ "draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n"
+ "m=video 9 UDP/TLS/RTP/SAVPF 112\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp-mux\r\n"
+ "a=sendonly\r\n"
+ "a=mid:1\r\n"
+ "a=rtpmap:112 VP8/90000\r\n"
+ "a=extmap:3 "
+ "http://www.ietf.org/id/"
+ "draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n";
+ auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
+ ASSERT_NE(desc, nullptr);
+ RTCError error;
+ pc->SetRemoteDescription(std::move(desc), &error);
+ EXPECT_TRUE(error.ok());
+ EXPECT_METRIC_EQ(1,
+ webrtc::metrics::NumEvents(
+ "WebRTC.PeerConnection.ValidBundledExtensionIds", true));
+}
+
+// extmap:3 is used with two different URIs which is not allowed.
+TEST_F(SdpOfferAnswerTest, BundleRejectsHeaderExtensionIdCollision) {
+ auto pc = CreatePeerConnection();
+ std::string sdp =
+ "v=0\r\n"
+ "o=- 0 3 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "a=group:BUNDLE 0 1\r\n"
+ "a=fingerprint:sha-1 "
+ "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
+ "a=setup:actpass\r\n"
+ "a=ice-ufrag:ETEn\r\n"
+ "a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
+ "m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp-mux\r\n"
+ "a=sendonly\r\n"
+ "a=mid:0\r\n"
+ "a=rtpmap:111 opus/48000/2\r\n"
+ "a=extmap:3 "
+ "http://www.ietf.org/id/"
+ "draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n"
+ "m=video 9 UDP/TLS/RTP/SAVPF 112\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp-mux\r\n"
+ "a=sendonly\r\n"
+ "a=mid:1\r\n"
+ "a=rtpmap:112 VP8/90000\r\n"
+ "a=extmap:3 urn:3gpp:video-orientation\r\n";
+ auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
+ ASSERT_NE(desc, nullptr);
+ RTCError error;
+ pc->SetRemoteDescription(std::move(desc), &error);
+ EXPECT_TRUE(error.ok());
+ EXPECT_METRIC_EQ(
+ 1, webrtc::metrics::NumEvents(
+ "WebRTC.PeerConnection.ValidBundledExtensionIds", false));
+}
+
+// transport-wide cc is negotiated with two different ids 3 and 4.
+// This is not a good idea but tolerable.
+TEST_F(SdpOfferAnswerTest, BundleAcceptsDifferentIdsForSameExtension) {
+ auto pc = CreatePeerConnection();
+ std::string sdp =
+ "v=0\r\n"
+ "o=- 0 3 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "a=group:BUNDLE 0 1\r\n"
+ "a=fingerprint:sha-1 "
+ "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
+ "a=setup:actpass\r\n"
+ "a=ice-ufrag:ETEn\r\n"
+ "a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
+ "m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp-mux\r\n"
+ "a=sendonly\r\n"
+ "a=mid:0\r\n"
+ "a=rtpmap:111 opus/48000/2\r\n"
+ "a=extmap:3 "
+ "http://www.ietf.org/id/"
+ "draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n"
+ "m=video 9 UDP/TLS/RTP/SAVPF 112\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp-mux\r\n"
+ "a=sendonly\r\n"
+ "a=mid:1\r\n"
+ "a=rtpmap:112 VP8/90000\r\n"
+ "a=extmap:4 "
+ "http://www.ietf.org/id/"
+ "draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n";
+ auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
+ ASSERT_NE(desc, nullptr);
+ RTCError error;
+ pc->SetRemoteDescription(std::move(desc), &error);
+ EXPECT_TRUE(error.ok());
+ EXPECT_METRIC_EQ(1,
+ webrtc::metrics::NumEvents(
+ "WebRTC.PeerConnection.ValidBundledExtensionIds", true));
+}
+
TEST_F(SdpOfferAnswerTest, LargeMidsAreRejected) {
auto pc = CreatePeerConnection();
std::string sdp =
