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 <steveanton@webrtc.org> > Commit-Queue: Amit Hilbuch <amithi@webrtc.org> > 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 <guidou@webrtc.org> Commit-Queue: Guido Urdaneta <guidou@webrtc.org> Cr-Commit-Position: refs/heads/master@{#27525}
This commit is contained in:

committed by
Commit Bot

parent
5154c5bc01
commit
a857698d54
@ -208,7 +208,6 @@ rtc_static_library("peerconnection") {
|
|||||||
"../logging:rtc_event_log_impl_output",
|
"../logging:rtc_event_log_impl_output",
|
||||||
"../media:rtc_data",
|
"../media:rtc_data",
|
||||||
"../media:rtc_media_base",
|
"../media:rtc_media_base",
|
||||||
"../modules/rtp_rtcp:rtp_rtcp_format",
|
|
||||||
"../p2p:rtc_p2p",
|
"../p2p:rtc_p2p",
|
||||||
"../rtc_base",
|
"../rtc_base",
|
||||||
"../rtc_base:checks",
|
"../rtc_base:checks",
|
||||||
|
@ -24,14 +24,11 @@
|
|||||||
#include "api/jsep_session_description.h"
|
#include "api/jsep_session_description.h"
|
||||||
#include "api/media_stream_proxy.h"
|
#include "api/media_stream_proxy.h"
|
||||||
#include "api/media_stream_track_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 "api/uma_metrics.h"
|
||||||
#include "call/call.h"
|
#include "call/call.h"
|
||||||
#include "logging/rtc_event_log/ice_logger.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/output/rtc_event_log_output_file.h"
|
||||||
#include "logging/rtc_event_log/rtc_event_log.h"
|
#include "logging/rtc_event_log/rtc_event_log.h"
|
||||||
#include "media/base/rid_description.h"
|
|
||||||
#include "media/sctp/sctp_transport.h"
|
#include "media/sctp/sctp_transport.h"
|
||||||
#include "pc/audio_rtp_receiver.h"
|
#include "pc/audio_rtp_receiver.h"
|
||||||
#include "pc/audio_track.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.");
|
"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,
|
if (absl::c_any_of(init.send_encodings,
|
||||||
[](const RtpEncodingParameters& encoding) {
|
[](const RtpEncodingParameters& encoding) {
|
||||||
return encoding.ssrc.has_value();
|
return encoding.ssrc.has_value();
|
||||||
|
@ -15,8 +15,6 @@
|
|||||||
#include "api/audio_codecs/builtin_audio_decoder_factory.h"
|
#include "api/audio_codecs/builtin_audio_decoder_factory.h"
|
||||||
#include "api/audio_codecs/builtin_audio_encoder_factory.h"
|
#include "api/audio_codecs/builtin_audio_encoder_factory.h"
|
||||||
#include "api/create_peerconnection_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/rtp_transceiver_interface.h"
|
||||||
#include "api/uma_metrics.h"
|
#include "api/uma_metrics.h"
|
||||||
#include "api/video_codecs/builtin_video_decoder_factory.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());
|
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.
|
// Validates that a single RID is removed from the encoding layer.
|
||||||
TEST_F(PeerConnectionSimulcastTests, SingleRidIsRemovedFromSessionDescription) {
|
TEST_F(PeerConnectionSimulcastTests, SingleRidIsRemovedFromSessionDescription) {
|
||||||
auto pc = CreatePeerConnectionWrapper();
|
auto pc = CreatePeerConnectionWrapper();
|
||||||
|
@ -16,7 +16,6 @@
|
|||||||
|
|
||||||
#include "absl/algorithm/container.h"
|
#include "absl/algorithm/container.h"
|
||||||
#include "api/jsep.h"
|
#include "api/jsep.h"
|
||||||
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
|
|
||||||
#include "rtc_base/checks.h"
|
#include "rtc_base/checks.h"
|
||||||
#include "rtc_base/string_encode.h"
|
#include "rtc_base/string_encode.h"
|
||||||
#include "rtc_base/string_to_number.h"
|
#include "rtc_base/string_to_number.h"
|
||||||
@ -329,10 +328,6 @@ RTCErrorOr<RidDescription> SdpSerializer::DeserializeRidDescription(
|
|||||||
return ParseError("Invalid RID Description format. Too many arguments.");
|
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) {
|
if (tokens[1] != kSendDirection && tokens[1] != kReceiveDirection) {
|
||||||
return ParseError("Invalid RID direction. Supported values: send / recv.");
|
return ParseError("Invalid RID direction. Supported values: send / recv.");
|
||||||
}
|
}
|
||||||
|
@ -469,10 +469,6 @@ const char* kRidDescriptionMalformedStrings[] = {
|
|||||||
"1 send pt=",
|
"1 send pt=",
|
||||||
"1 send pt=abc",
|
"1 send pt=abc",
|
||||||
"1 recv ;;",
|
"1 recv ;;",
|
||||||
"~1 recv",
|
|
||||||
"1$2 send",
|
|
||||||
"1=2 send",
|
|
||||||
"1* send",
|
|
||||||
};
|
};
|
||||||
|
|
||||||
INSTANTIATE_TEST_SUITE_P(RidDescriptionDeserializationErrors,
|
INSTANTIATE_TEST_SUITE_P(RidDescriptionDeserializationErrors,
|
||||||
|
Reference in New Issue
Block a user