Reland "Adding a restriction for legal RID values."

This is a reland of 07f3279a730980583403b78c3762c5d246d1d9be

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

Bug: webrtc:10491
Change-Id: I856581306a9258480ee9184f12b55c2a23dd8636
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/131983
Commit-Queue: Amit Hilbuch <amithi@webrtc.org>
Reviewed-by: Amit Hilbuch <amithi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27530}
This commit is contained in:
Amit Hilbuch
2019-04-08 14:11:57 -07:00
committed by Commit Bot
parent 2af5dcbe9e
commit f4770401dc
5 changed files with 33 additions and 0 deletions

View File

@ -208,6 +208,7 @@ 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",

View File

@ -24,11 +24,14 @@
#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"
@ -1547,6 +1550,14 @@ 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();

View File

@ -15,6 +15,8 @@
#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"
@ -254,6 +256,16 @@ 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();

View File

@ -16,6 +16,7 @@
#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"
@ -328,6 +329,10 @@ RTCErrorOr<RidDescription> 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.");
}

View File

@ -469,6 +469,10 @@ 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,