Cherry-pick WebRTC 235826 change
https://bugs.webkit.org/show_bug.cgi?id=235510
<rdar://87884184>
Reviewed by Eric Carlson.
Cherry-picking above fixing for compliance.
* Source/webrtc/media/engine/webrtc_media_engine.cc:
* Source/webrtc/media/engine/webrtc_media_engine.h:
* Source/webrtc/media/engine/webrtc_media_engine_unittest.cc:
* Source/webrtc/media/engine/webrtc_video_engine.cc:
* Source/webrtc/media/engine/webrtc_video_engine.h:
* Source/webrtc/media/engine/webrtc_voice_engine.cc:
* Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map.cc:
* Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map_unittest.cc:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@288464 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/ThirdParty/libwebrtc/ChangeLog b/Source/ThirdParty/libwebrtc/ChangeLog
index 203e3f1..627bce1 100644
--- a/Source/ThirdParty/libwebrtc/ChangeLog
+++ b/Source/ThirdParty/libwebrtc/ChangeLog
@@ -1,5 +1,24 @@
2022-01-24 Youenn Fablet <youenn@apple.com>
+ Cherry-pick WebRTC 235826 change
+ https://bugs.webkit.org/show_bug.cgi?id=235510
+ <rdar://87884184>
+
+ Reviewed by Eric Carlson.
+
+ Cherry-picking above fixing for compliance.
+
+ * Source/webrtc/media/engine/webrtc_media_engine.cc:
+ * Source/webrtc/media/engine/webrtc_media_engine.h:
+ * Source/webrtc/media/engine/webrtc_media_engine_unittest.cc:
+ * Source/webrtc/media/engine/webrtc_video_engine.cc:
+ * Source/webrtc/media/engine/webrtc_video_engine.h:
+ * Source/webrtc/media/engine/webrtc_voice_engine.cc:
+ * Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map.cc:
+ * Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map_unittest.cc:
+
+2022-01-24 Youenn Fablet <youenn@apple.com>
+
Reject large number of WebRTC audio channels
https://bugs.webkit.org/show_bug.cgi?id=235511
diff --git a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine.cc b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine.cc
index 6ce52e4..f083b9c 100644
--- a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine.cc
+++ b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine.cc
@@ -10,6 +10,7 @@
#include "media/engine/webrtc_media_engine.h"
+#include <map>
#include <memory>
#include <utility>
@@ -74,7 +75,8 @@
} // namespace
bool ValidateRtpExtensions(
- const std::vector<webrtc::RtpExtension>& extensions) {
+ rtc::ArrayView<const webrtc::RtpExtension> extensions,
+ rtc::ArrayView<const webrtc::RtpExtension> old_extensions) {
bool id_used[1 + webrtc::RtpExtension::kMaxId] = {false};
for (const auto& extension : extensions) {
if (extension.id < webrtc::RtpExtension::kMinId ||
@@ -89,6 +91,45 @@
}
id_used[extension.id] = true;
}
+ // Validate the extension list against the already negotiated extensions.
+ // Re-registering is OK, re-mapping (either same URL at new ID or same
+ // ID used with new URL) is an illegal remap.
+
+ // This is required in order to avoid a crash when registering an
+ // extension. A better structure would use the registered extensions
+ // in the RTPSender. This requires spinning through:
+ //
+ // WebRtcVoiceMediaChannel::::WebRtcAudioSendStream::stream_ (pointer)
+ // AudioSendStream::rtp_rtcp_module_ (pointer)
+ // ModuleRtpRtcpImpl2::rtp_sender_ (pointer)
+ // RtpSenderContext::packet_generator (struct member)
+ // RTPSender::rtp_header_extension_map_ (class member)
+ //
+ // Getting at this seems like a hard slog.
+ if (!old_extensions.empty()) {
+ absl::string_view urimap[1 + webrtc::RtpExtension::kMaxId];
+ std::map<absl::string_view, int> idmap;
+ for (const auto& old_extension : old_extensions) {
+ urimap[old_extension.id] = old_extension.uri;
+ idmap[old_extension.uri] = old_extension.id;
+ }
+ for (const auto& extension : extensions) {
+ if (!urimap[extension.id].empty() &&
+ urimap[extension.id] != extension.uri) {
+ RTC_LOG(LS_ERROR) << "Extension negotiation failure: " << extension.id
+ << " was mapped to " << urimap[extension.id]
+ << " but is proposed changed to " << extension.uri;
+ return false;
+ }
+ const auto& it = idmap.find(extension.uri);
+ if (it != idmap.end() && it->second != extension.id) {
+ RTC_LOG(LS_ERROR) << "Extension negotation failure: " << extension.uri
+ << " was identified by " << it->second
+ << " but is proposed changed to " << extension.id;
+ return false;
+ }
+ }
+ }
return true;
}
@@ -97,7 +138,8 @@
bool (*supported)(absl::string_view),
bool filter_redundant_extensions,
const webrtc::WebRtcKeyValueConfig& trials) {
- RTC_DCHECK(ValidateRtpExtensions(extensions));
+ // Don't check against old parameters; this should have been done earlier.
+ RTC_DCHECK(ValidateRtpExtensions(extensions, {}));
RTC_DCHECK(supported);
std::vector<webrtc::RtpExtension> result;
diff --git a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine.h b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine.h
index 34ec4cd..ff97760 100644
--- a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine.h
+++ b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine.h
@@ -63,8 +63,11 @@
MediaEngineDependencies dependencies);
// Verify that extension IDs are within 1-byte extension range and are not
-// overlapping.
-bool ValidateRtpExtensions(const std::vector<webrtc::RtpExtension>& extensions);
+// overlapping, and that they form a legal change from previously registerd
+// extensions (if any).
+bool ValidateRtpExtensions(
+ rtc::ArrayView<const webrtc::RtpExtension> extennsions,
+ rtc::ArrayView<const webrtc::RtpExtension> old_extensions);
// Discard any extensions not validated by the 'supported' predicate. Duplicate
// extensions are removed if 'filter_redundant_extensions' is set, and also any
diff --git a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine_unittest.cc b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine_unittest.cc
index 78d13fc..81982fa 100644
--- a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine_unittest.cc
+++ b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_media_engine_unittest.cc
@@ -66,41 +66,68 @@
}
} // namespace
-TEST(WebRtcMediaEngineTest, ValidateRtpExtensions_EmptyList) {
+TEST(WebRtcMediaEngineTest, ValidateRtpExtensionsEmptyList) {
std::vector<RtpExtension> extensions;
- EXPECT_TRUE(ValidateRtpExtensions(extensions));
+ EXPECT_TRUE(ValidateRtpExtensions(extensions, {}));
}
-TEST(WebRtcMediaEngineTest, ValidateRtpExtensions_AllGood) {
+TEST(WebRtcMediaEngineTest, ValidateRtpExtensionsAllGood) {
std::vector<RtpExtension> extensions = MakeUniqueExtensions();
- EXPECT_TRUE(ValidateRtpExtensions(extensions));
+ EXPECT_TRUE(ValidateRtpExtensions(extensions, {}));
}
-TEST(WebRtcMediaEngineTest, ValidateRtpExtensions_OutOfRangeId_Low) {
+TEST(WebRtcMediaEngineTest, ValidateRtpExtensionsOutOfRangeId_Low) {
std::vector<RtpExtension> extensions = MakeUniqueExtensions();
extensions.push_back(RtpExtension("foo", 0));
- EXPECT_FALSE(ValidateRtpExtensions(extensions));
+ EXPECT_FALSE(ValidateRtpExtensions(extensions, {}));
}
-TEST(WebRtcMediaEngineTest, ValidateRtpExtensions_OutOfRangeId_High) {
+TEST(WebRtcMediaEngineTest, ValidateRtpExtensionsOutOfRangeIdHigh) {
std::vector<RtpExtension> extensions = MakeUniqueExtensions();
extensions.push_back(RtpExtension("foo", 256));
- EXPECT_FALSE(ValidateRtpExtensions(extensions));
+ EXPECT_FALSE(ValidateRtpExtensions(extensions, {}));
}
-TEST(WebRtcMediaEngineTest, ValidateRtpExtensions_OverlappingIds_StartOfSet) {
+TEST(WebRtcMediaEngineTest, ValidateRtpExtensionsOverlappingIdsStartOfSet) {
std::vector<RtpExtension> extensions = MakeUniqueExtensions();
extensions.push_back(RtpExtension("foo", 1));
- EXPECT_FALSE(ValidateRtpExtensions(extensions));
+ EXPECT_FALSE(ValidateRtpExtensions(extensions, {}));
}
-TEST(WebRtcMediaEngineTest, ValidateRtpExtensions_OverlappingIds_EndOfSet) {
+TEST(WebRtcMediaEngineTest, ValidateRtpExtensionsOverlappingIdsEndOfSet) {
std::vector<RtpExtension> extensions = MakeUniqueExtensions();
extensions.push_back(RtpExtension("foo", 255));
- EXPECT_FALSE(ValidateRtpExtensions(extensions));
+ EXPECT_FALSE(ValidateRtpExtensions(extensions, {}));
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_EmptyList) {
+TEST(WebRtcMediaEngineTest, ValidateRtpExtensionsEmptyToEmpty) {
+ std::vector<RtpExtension> extensions;
+ EXPECT_TRUE(ValidateRtpExtensions(extensions, extensions));
+}
+
+TEST(WebRtcMediaEngineTest, ValidateRtpExtensionsNoChange) {
+ std::vector<RtpExtension> extensions = MakeUniqueExtensions();
+ EXPECT_TRUE(ValidateRtpExtensions(extensions, extensions));
+}
+
+TEST(WebRtcMediaEngineTest, ValidateRtpExtensionsChangeIdNotUrl) {
+ std::vector<RtpExtension> old_extensions = MakeUniqueExtensions();
+ std::vector<RtpExtension> new_extensions = old_extensions;
+ std::swap(new_extensions[0].id, new_extensions[1].id);
+
+ EXPECT_FALSE(ValidateRtpExtensions(new_extensions, old_extensions));
+}
+
+TEST(WebRtcMediaEngineTest, ValidateRtpExtensionsChangeIdForUrl) {
+ std::vector<RtpExtension> old_extensions = MakeUniqueExtensions();
+ std::vector<RtpExtension> new_extensions = old_extensions;
+ // Change first extension to something not generated by MakeUniqueExtensions
+ new_extensions[0].id = 123;
+
+ EXPECT_FALSE(ValidateRtpExtensions(new_extensions, old_extensions));
+}
+
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsEmptyList) {
std::vector<RtpExtension> extensions;
webrtc::FieldTrialBasedConfig trials;
std::vector<webrtc::RtpExtension> filtered =
@@ -108,7 +135,7 @@
EXPECT_EQ(0u, filtered.size());
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_IncludeOnlySupported) {
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsIncludeOnlySupported) {
std::vector<RtpExtension> extensions = MakeUniqueExtensions();
webrtc::FieldTrialBasedConfig trials;
std::vector<webrtc::RtpExtension> filtered =
@@ -118,7 +145,7 @@
EXPECT_EQ("i", filtered[1].uri);
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_SortedByName_1) {
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsSortedByName1) {
std::vector<RtpExtension> extensions = MakeUniqueExtensions();
webrtc::FieldTrialBasedConfig trials;
std::vector<webrtc::RtpExtension> filtered =
@@ -127,7 +154,7 @@
EXPECT_TRUE(IsSorted(filtered));
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_SortedByName_2) {
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsSortedByName2) {
std::vector<RtpExtension> extensions = MakeUniqueExtensions();
webrtc::FieldTrialBasedConfig trials;
std::vector<webrtc::RtpExtension> filtered =
@@ -136,7 +163,7 @@
EXPECT_TRUE(IsSorted(filtered));
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_DontRemoveRedundant) {
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsDontRemoveRedundant) {
std::vector<RtpExtension> extensions = MakeRedundantExtensions();
webrtc::FieldTrialBasedConfig trials;
std::vector<webrtc::RtpExtension> filtered =
@@ -146,7 +173,7 @@
EXPECT_EQ(filtered[0].uri, filtered[1].uri);
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundant) {
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsRemoveRedundant) {
std::vector<RtpExtension> extensions = MakeRedundantExtensions();
webrtc::FieldTrialBasedConfig trials;
std::vector<webrtc::RtpExtension> filtered =
@@ -156,7 +183,7 @@
EXPECT_NE(filtered[0].uri, filtered[1].uri);
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundantEncrypted_1) {
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsRemoveRedundantEncrypted1) {
std::vector<RtpExtension> extensions;
extensions.push_back(webrtc::RtpExtension("b", 1));
extensions.push_back(webrtc::RtpExtension("b", 2, true));
@@ -173,7 +200,7 @@
EXPECT_NE(filtered[1].uri, filtered[2].uri);
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundantEncrypted_2) {
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsRemoveRedundantEncrypted2) {
std::vector<RtpExtension> extensions;
extensions.push_back(webrtc::RtpExtension("b", 1, true));
extensions.push_back(webrtc::RtpExtension("b", 2));
@@ -190,7 +217,7 @@
EXPECT_NE(filtered[1].uri, filtered[2].uri);
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundantBwe_1) {
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsRemoveRedundantBwe1) {
webrtc::test::ScopedFieldTrials override_field_trials_(
"WebRTC-FilterAbsSendTimeExtension/Enabled/");
webrtc::FieldTrialBasedConfig trials;
@@ -209,7 +236,7 @@
}
TEST(WebRtcMediaEngineTest,
- FilterRtpExtensions_RemoveRedundantBwe_1_KeepAbsSendTime) {
+ FilterRtpExtensionsRemoveRedundantBwe1KeepAbsSendTime) {
std::vector<RtpExtension> extensions;
extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumberUri, 3));
@@ -226,7 +253,7 @@
EXPECT_EQ(RtpExtension::kAbsSendTimeUri, filtered[1].uri);
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundantBweEncrypted_1) {
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsRemoveRedundantBweEncrypted1) {
webrtc::test::ScopedFieldTrials override_field_trials_(
"WebRTC-FilterAbsSendTimeExtension/Enabled/");
webrtc::FieldTrialBasedConfig trials;
@@ -251,7 +278,7 @@
}
TEST(WebRtcMediaEngineTest,
- FilterRtpExtensions_RemoveRedundantBweEncrypted_1_KeepAbsSendTime) {
+ FilterRtpExtensionsRemoveRedundantBweEncrypted1KeepAbsSendTime) {
std::vector<RtpExtension> extensions;
extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumberUri, 3));
@@ -274,7 +301,7 @@
EXPECT_NE(filtered[0].encrypt, filtered[1].encrypt);
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundantBwe_2) {
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsRemoveRedundantBwe2) {
std::vector<RtpExtension> extensions;
extensions.push_back(RtpExtension(RtpExtension::kTimestampOffsetUri, 1));
extensions.push_back(RtpExtension(RtpExtension::kAbsSendTimeUri, 14));
@@ -286,7 +313,7 @@
EXPECT_EQ(RtpExtension::kAbsSendTimeUri, filtered[0].uri);
}
-TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundantBwe_3) {
+TEST(WebRtcMediaEngineTest, FilterRtpExtensionsRemoveRedundantBwe3) {
std::vector<RtpExtension> extensions;
extensions.push_back(RtpExtension(RtpExtension::kTimestampOffsetUri, 2));
extensions.push_back(RtpExtension(RtpExtension::kTimestampOffsetUri, 14));
diff --git a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_video_engine.cc b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_video_engine.cc
index ea2298b..39e3cc5 100644
--- a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_video_engine.cc
+++ b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_video_engine.cc
@@ -808,7 +808,7 @@
const VideoSendParameters& params,
ChangedSendParameters* changed_params) const {
if (!ValidateCodecFormats(params.codecs) ||
- !ValidateRtpExtensions(params.extensions)) {
+ !ValidateRtpExtensions(params.extensions, send_rtp_extensions_)) {
return false;
}
@@ -843,7 +843,7 @@
std::vector<webrtc::RtpExtension> filtered_extensions = FilterRtpExtensions(
params.extensions, webrtc::RtpExtension::IsSupportedForVideo, true,
call_->trials());
- if (!send_rtp_extensions_ || (*send_rtp_extensions_ != filtered_extensions)) {
+ if (send_rtp_extensions_ != filtered_extensions) {
changed_params->rtp_header_extensions =
absl::optional<std::vector<webrtc::RtpExtension>>(filtered_extensions);
}
@@ -990,7 +990,7 @@
SetExtmapAllowMixed(*changed_params.extmap_allow_mixed);
}
if (changed_params.rtp_header_extensions) {
- send_rtp_extensions_ = changed_params.rtp_header_extensions;
+ send_rtp_extensions_ = *changed_params.rtp_header_extensions;
}
if (changed_params.send_codec || changed_params.max_bandwidth_bps) {
@@ -1167,7 +1167,7 @@
const VideoRecvParameters& params,
ChangedRecvParameters* changed_params) const {
if (!ValidateCodecFormats(params.codecs) ||
- !ValidateRtpExtensions(params.extensions)) {
+ !ValidateRtpExtensions(params.extensions, recv_rtp_extensions_)) {
return false;
}
diff --git a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_video_engine.h b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_video_engine.h
index 9b8ba0b..6602319 100644
--- a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_video_engine.h
+++ b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_video_engine.h
@@ -601,7 +601,7 @@
std::vector<VideoCodecSettings> negotiated_codecs_
RTC_GUARDED_BY(thread_checker_);
- absl::optional<std::vector<webrtc::RtpExtension>> send_rtp_extensions_
+ std::vector<webrtc::RtpExtension> send_rtp_extensions_
RTC_GUARDED_BY(thread_checker_);
webrtc::VideoEncoderFactory* const encoder_factory_
diff --git a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_voice_engine.cc b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_voice_engine.cc
index 448ad35..7ce5063 100644
--- a/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_voice_engine.cc
+++ b/Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/webrtc_voice_engine.cc
@@ -1384,7 +1384,7 @@
return false;
}
- if (!ValidateRtpExtensions(params.extensions)) {
+ if (!ValidateRtpExtensions(params.extensions, send_rtp_extensions_)) {
return false;
}
@@ -1430,7 +1430,7 @@
return false;
}
- if (!ValidateRtpExtensions(params.extensions)) {
+ if (!ValidateRtpExtensions(params.extensions, recv_rtp_extensions_)) {
return false;
}
std::vector<webrtc::RtpExtension> filtered_extensions = FilterRtpExtensions(
diff --git a/Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map.cc b/Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map.cc
index 541e570..0584823 100644
--- a/Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map.cc
+++ b/Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map.cc
@@ -161,7 +161,12 @@
<< static_cast<int>(registered_type);
return false;
}
- RTC_DCHECK(!IsRegistered(type));
+ if (IsRegistered(type)) {
+ RTC_LOG(LS_WARNING) << "Illegal reregistration for uri: " << uri
+ << " is previously registered with id " << GetId(type)
+ << " and cannot be reregistered with id " << id;
+ return false;
+ }
// There is a run-time check above id fits into uint8_t.
ids_[type] = static_cast<uint8_t>(id);
diff --git a/Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map_unittest.cc b/Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map_unittest.cc
index 91bb252..0326b45 100644
--- a/Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map_unittest.cc
+++ b/Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_header_extension_map_unittest.cc
@@ -106,4 +106,10 @@
EXPECT_EQ(3, map.GetId(TransmissionOffset::kId));
}
+TEST(RtpHeaderExtensionTest, RemapFails) {
+ RtpHeaderExtensionMap map;
+ EXPECT_TRUE(map.Register<TransmissionOffset>(3));
+ EXPECT_FALSE(map.Register<TransmissionOffset>(4));
+}
+
} // namespace webrtc
