admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Check the return value of the FromString call and return failure when then value is invalid. I.e. uses

Browse Source 
bool FromString(const std::string& s, T* t)
instead of
T FromString(const std::string& str)

Before this change we will silently continue the parsing and take whatever default value returned by FromString.

TEST=new tests
BUG=2507
R=mallinath@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/11069004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5834 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
wu@webrtc.org 2014-04-02 23:19:09 +00:00
parent e387771b98
commit 5e760e7b94
2 changed files with 163 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

125
talk/app/webrtc/webrtcsdp.cc
View File

@ -588,6 +588,19 @@ static bool CaseInsensitiveFind(std::string str1, std::string str2) {
return str1.find(str2) != std::string::npos;
}
template <class T>
static bool GetValueFromString(const std::string& line,
const std::string& s,
T* t,
SdpParseError* error) {
if (!talk_base::FromString(s, t)) {
std::ostringstream description;
description << "Invalid value: " << s << ".";
return ParseFailed(line, description.str(), error);
}
return true;
}
void CreateTracksFromSsrcInfos(const SsrcInfoVec& ssrc_infos,
StreamParamsVec* tracks) {
ASSERT(tracks != NULL);
@ -1003,11 +1016,20 @@ bool ParseCandidate(const std::string& message, Candidate* candidate,
if (!GetValue(fields[0], kAttributeCandidate, &foundation, error)) {
return false;
}
const int component_id = talk_base::FromString<int>(fields[1]);
int component_id = 0;
if (!GetValueFromString(first_line, fields[1], &component_id, error)) {
return false;
}
const std::string transport = fields[2];
const uint32 priority = talk_base::FromString<uint32>(fields[3]);
uint32 priority = 0;
if (!GetValueFromString(first_line, fields[3], &priority, error)) {
return false;
}
const std::string connection_address = fields[4];
const int port = talk_base::FromString<int>(fields[5]);
int port = 0;
if (!GetValueFromString(first_line, fields[5], &port, error)) {
return false;
}
SocketAddress address(connection_address, port);
cricket::ProtocolType protocol;
@ -1038,8 +1060,12 @@ bool ParseCandidate(const std::string& message, Candidate* candidate,
}
if (fields.size() >= (current_position + 2) &&
fields[current_position] == kAttributeCandidateRport) {
related_address.SetPort(
talk_base::FromString<int>(fields[++current_position]));
int port = 0;
if (!GetValueFromString(
first_line, fields[++current_position], &port, error)) {
return false;
}
related_address.SetPort(port);
++current_position;
}
@ -1055,7 +1081,9 @@ bool ParseCandidate(const std::string& message, Candidate* candidate,
// RFC 5245
// *(SP extension-att-name SP extension-att-value)
if (fields[i] == kAttributeCandidateGeneration) {
generation = talk_base::FromString<uint32>(fields[++i]);
if (!GetValueFromString(first_line, fields[++i], &generation, error)) {
return false;
}
} else if (fields[i] == kAttributeCandidateUsername) {
username = fields[++i];
} else if (fields[i] == kAttributeCandidatePassword) {
@ -1110,7 +1138,10 @@ bool ParseExtmap(const std::string& line, RtpHeaderExtension* extmap,
}
std::vector<std::string> sub_fields;
talk_base::split(value_direction, kSdpDelimiterSlash, &sub_fields);
int value = talk_base::FromString<int>(sub_fields[0]);
int value = 0;
if (!GetValueFromString(line, sub_fields[0], &value, error)) {
return false;
}
*extmap = RtpHeaderExtension(uri, value);
return true;
@ -1541,7 +1572,9 @@ bool GetParameter(const std::string& name,
if (found == params.end()) {
return false;
}
*value = talk_base::FromString<int>(found->second);
if (!talk_base::FromString(found->second, value)) {
return false;
}
return true;
}
@ -2082,7 +2115,11 @@ bool ParseMediaDescription(const std::string& message,
// <fmt>
std::vector<int> codec_preference;
for (size_t j = 3 ; j < fields.size(); ++j) {
codec_preference.push_back(talk_base::FromString<int>(fields[j]));
int pl = 0;
if (!GetValueFromString(line, fields[j], &pl, error)) {
return false;
}
codec_preference.push_back(pl);
}
// Make a temporary TransportDescription based on |session_td|.
@ -2392,19 +2429,6 @@ bool ParseContent(const std::string& message,
}
}
if (IsLineType(line, kLineTypeSessionBandwidth)) {
std::string bandwidth;
if (HasAttribute(line, kApplicationSpecificMaximum)) {
if (!GetValue(line, kApplicationSpecificMaximum, &bandwidth, error)) {
return false;
} else {
media_desc->set_bandwidth(
talk_base::FromString<int>(bandwidth) * 1000);
}
}
continue;
}
// RFC 4566
// b=* (zero or more bandwidth information lines)
if (IsLineType(line, kLineTypeSessionBandwidth)) {
@ -2413,8 +2437,11 @@ bool ParseContent(const std::string& message,
if (!GetValue(line, kApplicationSpecificMaximum, &bandwidth, error)) {
return false;
} else {
media_desc->set_bandwidth(
talk_base::FromString<int>(bandwidth) * 1000);
int b = 0;
if (!GetValueFromString(line, bandwidth, &b, error)) {
return false;
}
media_desc->set_bandwidth(b * 1000);
}
}
continue;
@ -2537,9 +2564,11 @@ bool ParseContent(const std::string& message,
return false;
}
int buffer_latency = 0;
if (!talk_base::FromString(flag_value, &buffer_latency) ||
buffer_latency < 0) {
return ParseFailed(message, "Invalid buffer latency.", error);
if (!GetValueFromString(line, flag_value, &buffer_latency, error)) {
return false;
}
if (buffer_latency < 0) {
return ParseFailed(line, "Buffer latency less than 0.", error);
}
media_desc->set_buffered_mode_latency(buffer_latency);
}
@ -2631,7 +2660,10 @@ bool ParseSsrcAttribute(const std::string& line, SsrcInfoVec* ssrc_infos,
if (!GetValue(field1, kAttributeSsrc, &ssrc_id_s, error)) {
return false;
}
uint32 ssrc_id = talk_base::FromString<uint32>(ssrc_id_s);
uint32 ssrc_id = 0;
if (!GetValueFromString(line, ssrc_id_s, &ssrc_id, error)) {
return false;
}
std::string attribute;
std::string value;
@ -2708,7 +2740,10 @@ bool ParseSsrcGroupAttribute(const std::string& line,
}
std::vector<uint32> ssrcs;
for (size_t i = 1; i < fields.size(); ++i) {
uint32 ssrc = talk_base::FromString<uint32>(fields[i]);
uint32 ssrc = 0;
if (!GetValueFromString(line, fields[i], &ssrc, error)) {
return false;
}
ssrcs.push_back(ssrc);
}
ssrc_groups->push_back(SsrcGroup(semantics, ssrcs));
@ -2731,7 +2766,10 @@ bool ParseCryptoAttribute(const std::string& line,
if (!GetValue(fields[0], kAttributeCrypto, &tag_value, error)) {
return false;
}
int tag = talk_base::FromString<int>(tag_value);
int tag = 0;
if (!GetValueFromString(line, tag_value, &tag, error)) {
return false;
}
const std::string crypto_suite = fields[1];
const std::string key_params = fields[2];
std::string session_params;
@ -2795,7 +2833,10 @@ bool ParseRtpmapAttribute(const std::string& line,
if (!GetValue(fields[0], kAttributeRtpmap, &payload_type_value, error)) {
return false;
}
const int payload_type = talk_base::FromString<int>(payload_type_value);
int payload_type = 0;
if (!GetValueFromString(line, payload_type_value, &payload_type, error)) {
return false;
}
// Set the preference order depending on the order of the pl type in the
// <fmt> of the m-line.
@ -2819,7 +2860,10 @@ bool ParseRtpmapAttribute(const std::string& line,
error);
}
const std::string encoding_name = codec_params[0];
const int clock_rate = talk_base::FromString<int>(codec_params[1]);
int clock_rate = 0;
if (!GetValueFromString(line, codec_params[1], &clock_rate, error)) {
return false;
}
if (media_type == cricket::MEDIA_TYPE_VIDEO) {
VideoContentDescription* video_desc =
static_cast<VideoContentDescription*>(media_desc);
@ -2838,7 +2882,9 @@ bool ParseRtpmapAttribute(const std::string& line,
// additional parameters are needed.
int channels = 1;
if (codec_params.size() == 3) {
channels = talk_base::FromString<int>(codec_params[2]);
if (!GetValueFromString(line, codec_params[2], &channels, error)) {
return false;
}
}
int bitrate = 0;
// The default behavior for ISAC (bitrate == 0) in webrtcvoiceengine.cc
@ -2925,7 +2971,10 @@ bool ParseFmtpAttributes(const std::string& line, const MediaType media_type,
codec_params[name] = value;
}
int int_payload_type = talk_base::FromString<int>(payload_type);
int int_payload_type = 0;
if (!GetValueFromString(line, payload_type, &int_payload_type, error)) {
return false;
}
if (media_type == cricket::MEDIA_TYPE_AUDIO) {
UpdateCodec<AudioContentDescription, cricket::AudioCodec>(
media_desc, int_payload_type, codec_params);
@ -2953,8 +3002,12 @@ bool ParseRtcpFbAttribute(const std::string& line, const MediaType media_type,
error)) {
return false;
}
int payload_type = (payload_type_string == "*") ?
kWildcardPayloadType : talk_base::FromString<int>(payload_type_string);
int payload_type = kWildcardPayloadType;
if (payload_type_string != "*") {
if (!GetValueFromString(line, payload_type_string, &payload_type, error)) {
return false;
}
}
std::string id = rtcp_fb_fields[1];
std::string param = "";
for (std::vector<std::string>::iterator iter = rtcp_fb_fields.begin() + 2;

92
talk/app/webrtc/webrtcsdp_unittest.cc
View File

@ -401,14 +401,31 @@ static void Replace(const std::string& line,
newlines.c_str(), newlines.length(), message);
}
static void ReplaceAndTryToParse(const char* search, const char* replace) {
// Expect fail to parase |bad_sdp| and expect |bad_part| be part of the error
// message.
static void ExpectParseFailure(const std::string& bad_sdp,
const std::string& bad_part) {
JsepSessionDescription desc(kDummyString);
std::string sdp = kSdpFullString;
Replace(search, replace, &sdp);
SdpParseError error;
bool ret = webrtc::SdpDeserialize(sdp, &desc, &error);
bool ret = webrtc::SdpDeserialize(bad_sdp, &desc, &error);
EXPECT_FALSE(ret);
EXPECT_NE(std::string::npos, error.line.find(replace));
EXPECT_NE(std::string::npos, error.line.find(bad_part.c_str()));
}
// Expect fail to parse kSdpFullString if replace |good_part| with |bad_part|.
static void ExpectParseFailure(const char* good_part, const char* bad_part) {
std::string bad_sdp = kSdpFullString;
Replace(good_part, bad_part, &bad_sdp);
ExpectParseFailure(bad_sdp, bad_part);
}
// Expect fail to parse kSdpFullString if add |newlines| after |injectpoint|.
static void ExpectParseFailureWithNewLines(const std::string& injectpoint,
const std::string& newlines,
const std::string& bad_part) {
std::string bad_sdp = kSdpFullString;
InjectAfter(injectpoint, newlines, &bad_sdp);
ExpectParseFailure(bad_sdp, bad_part);
}
static void ReplaceDirection(cricket::MediaContentDirection direction,
@ -2017,28 +2034,67 @@ TEST_F(WebRtcSdpTest, DeserializeBrokenSdp) {
"4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B";
// Broken session description
ReplaceAndTryToParse("v=", kSdpDestroyer);
ReplaceAndTryToParse("o=", kSdpDestroyer);
ReplaceAndTryToParse("s=-", kSdpDestroyer);
ExpectParseFailure("v=", kSdpDestroyer);
ExpectParseFailure("o=", kSdpDestroyer);
ExpectParseFailure("s=-", kSdpDestroyer);
// Broken time description
ReplaceAndTryToParse("t=", kSdpDestroyer);
ExpectParseFailure("t=", kSdpDestroyer);
// Broken media description
ReplaceAndTryToParse("m=audio", "c=IN IP4 74.125.224.39");
ReplaceAndTryToParse("m=video", kSdpDestroyer);
ExpectParseFailure("m=audio", "c=IN IP4 74.125.224.39");
ExpectParseFailure("m=video", kSdpDestroyer);
// Invalid lines
ReplaceAndTryToParse("a=candidate", kSdpInvalidLine1);
ReplaceAndTryToParse("a=candidate", kSdpInvalidLine2);
ReplaceAndTryToParse("a=candidate", kSdpInvalidLine3);
ExpectParseFailure("a=candidate", kSdpInvalidLine1);
ExpectParseFailure("a=candidate", kSdpInvalidLine2);
ExpectParseFailure("a=candidate", kSdpInvalidLine3);
// Bogus fingerprint replacing a=sendrev. We selected this attribute
// because it's orthogonal to what we are replacing and hence
// safe.
ReplaceAndTryToParse("a=sendrecv", kSdpInvalidLine4);
ReplaceAndTryToParse("a=sendrecv", kSdpInvalidLine5);
ReplaceAndTryToParse("a=sendrecv", kSdpInvalidLine6);
ReplaceAndTryToParse("a=sendrecv", kSdpInvalidLine7);
ExpectParseFailure("a=sendrecv", kSdpInvalidLine4);
ExpectParseFailure("a=sendrecv", kSdpInvalidLine5);
ExpectParseFailure("a=sendrecv", kSdpInvalidLine6);
ExpectParseFailure("a=sendrecv", kSdpInvalidLine7);
}
TEST_F(WebRtcSdpTest, DeserializeSdpWithInvalidAttributeValue) {
// ssrc
ExpectParseFailure("a=ssrc:1", "a=ssrc:badvalue");
ExpectParseFailure("a=ssrc-group:FEC 5 6", "a=ssrc-group:FEC badvalue 6");
// crypto
ExpectParseFailure("a=crypto:1 ", "a=crypto:badvalue ");
// rtpmap
ExpectParseFailure("a=rtpmap:111 ", "a=rtpmap:badvalue ");
ExpectParseFailure("opus/48000/2", "opus/badvalue/2");
ExpectParseFailure("opus/48000/2", "opus/48000/badvalue");
// candidate
ExpectParseFailure("1 udp 2130706432", "badvalue udp 2130706432");
ExpectParseFailure("1 udp 2130706432", "1 udp badvalue");
ExpectParseFailure("192.168.1.5 1234", "192.168.1.5 badvalue");
ExpectParseFailure("rport 2346", "rport badvalue");
ExpectParseFailure("rport 2346 generation 2",
"rport 2346 generation badvalue");
// m line
ExpectParseFailure("m=audio 2345 RTP/SAVPF 111 103 104",
"m=audio 2345 RTP/SAVPF 111 badvalue 104");
// bandwidth
ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n",
"b=AS:badvalue\r\n",
"b=AS:badvalue");
// rtcp-fb
ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n",
"a=rtcp-fb:badvalue nack\r\n",
"a=rtcp-fb:badvalue nack");
// extmap
ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n",
"a=extmap:badvalue http://example.com\r\n",
"a=extmap:badvalue http://example.com");
// x-google-buffer-latency
ExpectParseFailureWithNewLines("a=mid:video_content_name\r\n",
"a=x-google-buffer-latency:badvalue\r\n",
"a=x-google-buffer-latency:badvalue");
}
TEST_F(WebRtcSdpTest, DeserializeSdpWithReorderedPltypes) {