From a857698d549df4977d38555af220e7839e484cfc Mon Sep 17 00:00:00 2001 From: Guido Urdaneta Date: Tue, 9 Apr 2019 17:12:23 +0000 Subject: [PATCH] Revert "Adding a restriction for legal RID values." This reverts commit 07f3279a730980583403b78c3762c5d246d1d9be. Reason for revert: Suspect of producing consistent failure in some Chrome trybots, blocking rolls. Failed test: external/wpt/webrtc/RTCPeerConnection-addTransceiver.https.html First failure: https://ci.chromium.org/p/chromium/builders/try/linux-rel/64597 Original change's description: > Adding a restriction for legal RID values. > > According to the spec, RID values should be constrained to only > alpha-numeric values. This was not enforced in our implementation to > allow for more flexibility. > It has been brought to our attention that some values that we currently > consider legal (such as the '~', '=' ';' characters) might cause confusion > with the simulcast syntax that uses these characters to indicate other > meanings. > What's worse, is that some characters, when used in RIDs (such as > \u{1f937} \u{1f4a9} and \u{1f926}) cause uncontrollable laughter for some > users which might also be a health hazard. > This change resolves these issues by restricting RIDs to alpha-numeric. > > Bug: webrtc:10491 > Change-Id: I16e262c87525d0289764beacd098e1525a355463 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132061 > Reviewed-by: Steve Anton > Commit-Queue: Amit Hilbuch > Cr-Commit-Position: refs/heads/master@{#27499} TBR=steveanton@webrtc.org,amithi@webrtc.org Change-Id: I89f9d8a8d3fa82de8a7d429f11ad7cc30812ba7c No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:10491 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132244 Reviewed-by: Guido Urdaneta Commit-Queue: Guido Urdaneta Cr-Commit-Position: refs/heads/master@{#27525} --- pc/BUILD.gn | 1 - pc/peer_connection.cc | 11 ----------- pc/peer_connection_simulcast_unittest.cc | 12 ------------ pc/sdp_serializer.cc | 5 ----- pc/sdp_serializer_unittest.cc | 4 ---- 5 files changed, 33 deletions(-) diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 48c864b179..a913f42360 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -208,7 +208,6 @@ rtc_static_library("peerconnection") { "../logging:rtc_event_log_impl_output", "../media:rtc_data", "../media:rtc_media_base", - "../modules/rtp_rtcp:rtp_rtcp_format", "../p2p:rtc_p2p", "../rtc_base", "../rtc_base:checks", diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index e7a1a11157..acf5e7251d 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -24,14 +24,11 @@ #include "api/jsep_session_description.h" #include "api/media_stream_proxy.h" #include "api/media_stream_track_proxy.h" -#include "api/rtc_error.h" -#include "api/rtp_parameters.h" #include "api/uma_metrics.h" #include "call/call.h" #include "logging/rtc_event_log/ice_logger.h" #include "logging/rtc_event_log/output/rtc_event_log_output_file.h" #include "logging/rtc_event_log/rtc_event_log.h" -#include "media/base/rid_description.h" #include "media/sctp/sctp_transport.h" #include "pc/audio_rtp_receiver.h" #include "pc/audio_track.h" @@ -1550,14 +1547,6 @@ PeerConnection::AddTransceiver( "RIDs must be provided for either all or none of the send encodings."); } - if (num_rids > 0 && absl::c_any_of(init.send_encodings, - [](const RtpEncodingParameters& encoding) { - return !IsLegalRsidName(encoding.rid); - })) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, - "Invalid RID value provided."); - } - if (absl::c_any_of(init.send_encodings, [](const RtpEncodingParameters& encoding) { return encoding.ssrc.has_value(); diff --git a/pc/peer_connection_simulcast_unittest.cc b/pc/peer_connection_simulcast_unittest.cc index 7618ce204f..3efc6cbf44 100644 --- a/pc/peer_connection_simulcast_unittest.cc +++ b/pc/peer_connection_simulcast_unittest.cc @@ -15,8 +15,6 @@ #include "api/audio_codecs/builtin_audio_decoder_factory.h" #include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "api/create_peerconnection_factory.h" -#include "api/media_types.h" -#include "api/rtc_error.h" #include "api/rtp_transceiver_interface.h" #include "api/uma_metrics.h" #include "api/video_codecs/builtin_video_decoder_factory.h" @@ -256,16 +254,6 @@ TEST_F(PeerConnectionSimulcastTests, MustSupplyAllOrNoRidsInSimulcast) { EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, error.error().type()); } -// Validates that an error is returned when illegal RIDs are supplied. -TEST_F(PeerConnectionSimulcastTests, ChecksForIllegalRidValues) { - auto pc_wrapper = CreatePeerConnectionWrapper(); - auto pc = pc_wrapper->pc(); - auto layers = CreateLayers({"f", "h", "~q"}, true); - auto init = CreateTransceiverInit(layers); - auto error = pc->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init); - EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, error.error().type()); -} - // Validates that a single RID is removed from the encoding layer. TEST_F(PeerConnectionSimulcastTests, SingleRidIsRemovedFromSessionDescription) { auto pc = CreatePeerConnectionWrapper(); diff --git a/pc/sdp_serializer.cc b/pc/sdp_serializer.cc index 7ebaffda86..482dd07cdc 100644 --- a/pc/sdp_serializer.cc +++ b/pc/sdp_serializer.cc @@ -16,7 +16,6 @@ #include "absl/algorithm/container.h" #include "api/jsep.h" -#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/checks.h" #include "rtc_base/string_encode.h" #include "rtc_base/string_to_number.h" @@ -329,10 +328,6 @@ RTCErrorOr SdpSerializer::DeserializeRidDescription( return ParseError("Invalid RID Description format. Too many arguments."); } - if (!IsLegalRsidName(tokens[0])) { - return ParseError("Invalid RID value: " + tokens[0] + "."); - } - if (tokens[1] != kSendDirection && tokens[1] != kReceiveDirection) { return ParseError("Invalid RID direction. Supported values: send / recv."); } diff --git a/pc/sdp_serializer_unittest.cc b/pc/sdp_serializer_unittest.cc index 726d01dc94..e655f229ca 100644 --- a/pc/sdp_serializer_unittest.cc +++ b/pc/sdp_serializer_unittest.cc @@ -469,10 +469,6 @@ const char* kRidDescriptionMalformedStrings[] = { "1 send pt=", "1 send pt=abc", "1 recv ;;", - "~1 recv", - "1$2 send", - "1=2 send", - "1* send", }; INSTANTIATE_TEST_SUITE_P(RidDescriptionDeserializationErrors,