Catching more errors when parsing ICE server URLs.
Every malformed URL should now produce an error message in JS, rather than
silently failing and possibly printing a warning message to the console (and
possibly crashing).
Also added some unit tests, and made "ParseIceServers" public.
BUG=445002
Review URL: https://codereview.webrtc.org/1344143002
Cr-Commit-Position: refs/heads/master@{#10186}
diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc
index 36875a6..bf9a80d 100644
--- a/talk/app/webrtc/peerconnection.cc
+++ b/talk/app/webrtc/peerconnection.cc
@@ -28,6 +28,7 @@
#include "talk/app/webrtc/peerconnection.h"
#include <vector>
+#include <cctype> // for isdigit
#include "talk/app/webrtc/dtmfsender.h"
#include "talk/app/webrtc/jsepicecandidate.h"
@@ -45,6 +46,12 @@
namespace {
using webrtc::PeerConnectionInterface;
+using webrtc::StunConfigurations;
+using webrtc::TurnConfigurations;
+typedef webrtc::PortAllocatorFactoryInterface::StunConfiguration
+ StunConfiguration;
+typedef webrtc::PortAllocatorFactoryInterface::TurnConfiguration
+ TurnConfiguration;
// The min number of tokens must present in Turn host uri.
// e.g. user@turn.example.org
@@ -59,16 +66,20 @@
static const char kTcpTransportType[] = "tcp";
// NOTE: Must be in the same order as the ServiceType enum.
-static const char* kValidIceServiceTypes[] = {
- "stun", "stuns", "turn", "turns", "invalid" };
+static const char* kValidIceServiceTypes[] = {"stun", "stuns", "turn", "turns"};
+// NOTE: A loop below assumes that the first value of this enum is 0 and all
+// other values are incremental.
enum ServiceType {
- STUN, // Indicates a STUN server.
- STUNS, // Indicates a STUN server used with a TLS session.
- TURN, // Indicates a TURN server
- TURNS, // Indicates a TURN server used with a TLS session.
- INVALID, // Unknown.
+ STUN = 0, // Indicates a STUN server.
+ STUNS, // Indicates a STUN server used with a TLS session.
+ TURN, // Indicates a TURN server
+ TURNS, // Indicates a TURN server used with a TLS session.
+ INVALID, // Unknown.
};
+static_assert(INVALID == ARRAY_SIZE(kValidIceServiceTypes),
+ "kValidIceServiceTypes must have as many strings as ServiceType "
+ "has values.");
enum {
MSG_SET_SESSIONDESCRIPTION_SUCCESS = 0,
@@ -100,7 +111,7 @@
// scheme = "stun" / "stuns"
// stun-host = IP-literal / IPv4address / reg-name
// stun-port = *DIGIT
-
+//
// draft-petithuguenin-behave-turn-uris-01
// turnURI = scheme ":" turn-host [ ":" turn-port ]
// turn-host = username@IP-literal / IPv4address / reg-name
@@ -108,12 +119,17 @@
ServiceType* service_type,
std::string* hostname) {
const std::string::size_type colonpos = in_str.find(':');
- if (colonpos == std::string::npos || (colonpos + 1) == in_str.length()) {
+ if (colonpos == std::string::npos) {
+ LOG(LS_WARNING) << "Missing ':' in ICE URI: " << in_str;
return false;
}
- std::string type = in_str.substr(0, colonpos);
+ if ((colonpos + 1) == in_str.length()) {
+ LOG(LS_WARNING) << "Empty hostname in ICE URI: " << in_str;
+ return false;
+ }
+ *service_type = INVALID;
for (size_t i = 0; i < ARRAY_SIZE(kValidIceServiceTypes); ++i) {
- if (type.compare(kValidIceServiceTypes[i]) == 0) {
+ if (in_str.compare(0, colonpos, kValidIceServiceTypes[i]) == 0) {
*service_type = static_cast<ServiceType>(i);
break;
}
@@ -125,52 +141,59 @@
return true;
}
+bool ParsePort(const std::string& in_str, int* port) {
+ // Make sure port only contains digits. FromString doesn't check this.
+ for (const char& c : in_str) {
+ if (!std::isdigit(c)) {
+ return false;
+ }
+ }
+ return rtc::FromString(in_str, port);
+}
+
// This method parses IPv6 and IPv4 literal strings, along with hostnames in
// standard hostname:port format.
// Consider following formats as correct.
// |hostname:port|, |[IPV6 address]:port|, |IPv4 address|:port,
-// |hostname|, |[IPv6 address]|, |IPv4 address|
+// |hostname|, |[IPv6 address]|, |IPv4 address|.
bool ParseHostnameAndPortFromString(const std::string& in_str,
std::string* host,
int* port) {
+ RTC_DCHECK(host->empty());
if (in_str.at(0) == '[') {
std::string::size_type closebracket = in_str.rfind(']');
if (closebracket != std::string::npos) {
- *host = in_str.substr(1, closebracket - 1);
std::string::size_type colonpos = in_str.find(':', closebracket);
if (std::string::npos != colonpos) {
- if (!rtc::FromString(
- in_str.substr(closebracket + 2, std::string::npos), port)) {
+ if (!ParsePort(in_str.substr(closebracket + 2, std::string::npos),
+ port)) {
return false;
}
}
+ *host = in_str.substr(1, closebracket - 1);
} else {
return false;
}
} else {
std::string::size_type colonpos = in_str.find(':');
if (std::string::npos != colonpos) {
- *host = in_str.substr(0, colonpos);
- if (!rtc::FromString(
- in_str.substr(colonpos + 1, std::string::npos), port)) {
+ if (!ParsePort(in_str.substr(colonpos + 1, std::string::npos), port)) {
return false;
}
+ *host = in_str.substr(0, colonpos);
} else {
*host = in_str;
}
}
- return true;
+ return !host->empty();
}
-typedef webrtc::PortAllocatorFactoryInterface::StunConfiguration
- StunConfiguration;
-typedef webrtc::PortAllocatorFactoryInterface::TurnConfiguration
- TurnConfiguration;
-
+// Adds a StunConfiguration or TurnConfiguration to the appropriate list,
+// by parsing |url| and using the username/password in |server|.
bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server,
const std::string& url,
- std::vector<StunConfiguration>* stun_config,
- std::vector<TurnConfiguration>* turn_config) {
+ StunConfigurations* stun_config,
+ TurnConfigurations* turn_config) {
// draft-nandakumar-rtcweb-stun-uri-01
// stunURI = scheme ":" stun-host [ ":" stun-port ]
// scheme = "stun" / "stuns"
@@ -185,9 +208,11 @@
// transport-ext = 1*unreserved
// turn-host = IP-literal / IPv4address / reg-name
// turn-port = *DIGIT
+ RTC_DCHECK(stun_config != nullptr);
+ RTC_DCHECK(turn_config != nullptr);
std::vector<std::string> tokens;
std::string turn_transport_type = kUdpTransportType;
- ASSERT(!url.empty());
+ RTC_DCHECK(!url.empty());
rtc::tokenize(url, '?', &tokens);
std::string uri_without_transport = tokens[0];
// Let's look into transport= param, if it exists.
@@ -199,32 +224,38 @@
// letters.
if (tokens[1] != kUdpTransportType && tokens[1] != kTcpTransportType) {
LOG(LS_WARNING) << "Transport param should always be udp or tcp.";
- return true;
+ return false;
}
turn_transport_type = tokens[1];
}
}
std::string hoststring;
- ServiceType service_type = INVALID;
+ ServiceType service_type;
if (!GetServiceTypeAndHostnameFromUri(uri_without_transport,
&service_type,
&hoststring)) {
- LOG(LS_WARNING) << "Invalid transport parameter in ICE URI: "
- << uri_without_transport;
- return true;
+ LOG(LS_WARNING) << "Invalid transport parameter in ICE URI: " << url;
+ return false;
}
- ASSERT(!hoststring.empty());
+ // GetServiceTypeAndHostnameFromUri should never give an empty hoststring
+ RTC_DCHECK(!hoststring.empty());
// Let's break hostname.
tokens.clear();
- rtc::tokenize(hoststring, '@', &tokens);
- ASSERT(!tokens.empty());
+ rtc::tokenize_with_empty_tokens(hoststring, '@', &tokens);
+
std::string username(server.username);
- // TODO(pthatcher): What's the right thing to do if tokens.size() is >2?
- // E.g. a string like "foo@bar@bat".
- if (tokens.size() >= kTurnHostTokensNum) {
+ if (tokens.size() > kTurnHostTokensNum) {
+ LOG(LS_WARNING) << "Invalid user@hostname format: " << hoststring;
+ return false;
+ }
+ if (tokens.size() == kTurnHostTokensNum) {
+ if (tokens[0].empty() || tokens[1].empty()) {
+ LOG(LS_WARNING) << "Invalid user@hostname format: " << hoststring;
+ return false;
+ }
username.assign(rtc::s_url_decode(tokens[0]));
hoststring = tokens[1];
} else {
@@ -239,13 +270,13 @@
std::string address;
if (!ParseHostnameAndPortFromString(hoststring, &address, &port)) {
- LOG(WARNING) << "Invalid Hostname format: " << uri_without_transport;
- return true;
+ LOG(WARNING) << "Invalid hostname format: " << uri_without_transport;
+ return false;
}
if (port <= 0 || port > 0xffff) {
LOG(WARNING) << "Invalid port: " << port;
- return true;
+ return false;
}
switch (service_type) {
@@ -255,18 +286,7 @@
break;
case TURN:
case TURNS: {
- if (username.empty()) {
- // Turn url example from the spec |url:"turn:user@turn.example.org"|.
- std::vector<std::string> turn_tokens;
- rtc::tokenize(address, '@', &turn_tokens);
- if (turn_tokens.size() == kTurnHostTokensNum) {
- username.assign(rtc::s_url_decode(turn_tokens[0]));
- address = turn_tokens[1];
- }
- }
-
bool secure = (service_type == TURNS);
-
turn_config->push_back(TurnConfiguration(address, port,
username,
server.password,
@@ -282,15 +302,19 @@
return true;
}
+} // namespace
+
+namespace webrtc {
+
bool ParseIceServers(const PeerConnectionInterface::IceServers& servers,
- std::vector<StunConfiguration>* stun_config,
- std::vector<TurnConfiguration>* turn_config) {
+ StunConfigurations* stun_config,
+ TurnConfigurations* turn_config) {
for (const webrtc::PeerConnectionInterface::IceServer& server : servers) {
if (!server.urls.empty()) {
for (const std::string& url : server.urls) {
if (url.empty()) {
- LOG(WARNING) << "Empty uri.";
- continue;
+ LOG(LS_ERROR) << "Empty uri.";
+ return false;
}
if (!ParseIceServerUrl(server, url, stun_config, turn_config)) {
return false;
@@ -302,7 +326,8 @@
return false;
}
} else {
- LOG(WARNING) << "Empty uri.";
+ LOG(LS_ERROR) << "Empty uri.";
+ return false;
}
}
return true;
@@ -324,10 +349,6 @@
return true;
}
-} // namespace
-
-namespace webrtc {
-
PeerConnection::PeerConnection(PeerConnectionFactory* factory)
: factory_(factory),
observer_(NULL),
@@ -339,7 +360,7 @@
}
PeerConnection::~PeerConnection() {
- ASSERT(signaling_thread()->IsCurrent());
+ RTC_DCHECK(signaling_thread()->IsCurrent());
if (mediastream_signaling_) {
mediastream_signaling_->TearDown();
}
@@ -359,7 +380,7 @@
PortAllocatorFactoryInterface* allocator_factory,
rtc::scoped_ptr<DtlsIdentityStoreInterface> dtls_identity_store,
PeerConnectionObserver* observer) {
- ASSERT(observer != NULL);
+ RTC_DCHECK(observer != NULL);
if (!observer)
return false;
observer_ = observer;
@@ -498,7 +519,7 @@
bool PeerConnection::GetStats(StatsObserver* observer,
MediaStreamTrackInterface* track,
StatsOutputLevel level) {
- ASSERT(signaling_thread()->IsCurrent());
+ RTC_DCHECK(signaling_thread()->IsCurrent());
if (!VERIFY(observer != NULL)) {
LOG(LS_ERROR) << "GetStats - observer is NULL.";
return false;
@@ -820,7 +841,7 @@
break;
}
default:
- ASSERT(false && "Not implemented");
+ RTC_DCHECK(false && "Not implemented");
break;
}
}
@@ -927,7 +948,7 @@
void PeerConnection::OnIceConnectionChange(
PeerConnectionInterface::IceConnectionState new_state) {
- ASSERT(signaling_thread()->IsCurrent());
+ RTC_DCHECK(signaling_thread()->IsCurrent());
// After transitioning to "closed", ignore any additional states from
// WebRtcSession (such as "disconnected").
if (ice_connection_state_ == kIceConnectionClosed) {
@@ -939,7 +960,7 @@
void PeerConnection::OnIceGatheringChange(
PeerConnectionInterface::IceGatheringState new_state) {
- ASSERT(signaling_thread()->IsCurrent());
+ RTC_DCHECK(signaling_thread()->IsCurrent());
if (IsClosed()) {
return;
}
@@ -948,17 +969,17 @@
}
void PeerConnection::OnIceCandidate(const IceCandidateInterface* candidate) {
- ASSERT(signaling_thread()->IsCurrent());
+ RTC_DCHECK(signaling_thread()->IsCurrent());
observer_->OnIceCandidate(candidate);
}
void PeerConnection::OnIceComplete() {
- ASSERT(signaling_thread()->IsCurrent());
+ RTC_DCHECK(signaling_thread()->IsCurrent());
observer_->OnIceComplete();
}
void PeerConnection::OnIceConnectionReceivingChange(bool receiving) {
- ASSERT(signaling_thread()->IsCurrent());
+ RTC_DCHECK(signaling_thread()->IsCurrent());
observer_->OnIceConnectionReceivingChange(receiving);
}
diff --git a/talk/app/webrtc/peerconnection.h b/talk/app/webrtc/peerconnection.h
index e996132..8a87019 100644
--- a/talk/app/webrtc/peerconnection.h
+++ b/talk/app/webrtc/peerconnection.h
@@ -48,6 +48,12 @@
typedef std::vector<PortAllocatorFactoryInterface::TurnConfiguration>
TurnConfigurations;
+// Parses the URLs for each server in |servers| to build |stun_config| and
+// |turn_config|.
+bool ParseIceServers(const PeerConnectionInterface::IceServers& servers,
+ StunConfigurations* stun_config,
+ TurnConfigurations* turn_config);
+
// PeerConnection implements the PeerConnectionInterface interface.
// It uses MediaStreamSignaling and WebRtcSession to implement
// the PeerConnection functionality.
diff --git a/talk/app/webrtc/peerconnection_unittest.cc b/talk/app/webrtc/peerconnection_unittest.cc
index d593f88..c8b0c8c 100644
--- a/talk/app/webrtc/peerconnection_unittest.cc
+++ b/talk/app/webrtc/peerconnection_unittest.cc
@@ -37,6 +37,7 @@
#include "talk/app/webrtc/fakeportallocatorfactory.h"
#include "talk/app/webrtc/localaudiosource.h"
#include "talk/app/webrtc/mediastreaminterface.h"
+#include "talk/app/webrtc/peerconnection.h"
#include "talk/app/webrtc/peerconnectionfactory.h"
#include "talk/app/webrtc/peerconnectioninterface.h"
#include "talk/app/webrtc/test/fakeaudiocapturemodule.h"
@@ -1630,4 +1631,180 @@
LocalP2PTest();
}
+class IceServerParsingTest : public testing::Test {
+ public:
+ // Convenience for parsing a single URL.
+ bool ParseUrl(const std::string& url) {
+ return ParseUrl(url, std::string(), std::string());
+ }
+
+ bool ParseUrl(const std::string& url,
+ const std::string& username,
+ const std::string& password) {
+ PeerConnectionInterface::IceServers servers;
+ PeerConnectionInterface::IceServer server;
+ server.urls.push_back(url);
+ server.username = username;
+ server.password = password;
+ servers.push_back(server);
+ return webrtc::ParseIceServers(servers, &stun_configurations_,
+ &turn_configurations_);
+ }
+
+ protected:
+ webrtc::StunConfigurations stun_configurations_;
+ webrtc::TurnConfigurations turn_configurations_;
+};
+
+// Make sure all STUN/TURN prefixes are parsed correctly.
+TEST_F(IceServerParsingTest, ParseStunPrefixes) {
+ EXPECT_TRUE(ParseUrl("stun:hostname"));
+ EXPECT_EQ(1U, stun_configurations_.size());
+ EXPECT_EQ(0U, turn_configurations_.size());
+ stun_configurations_.clear();
+
+ EXPECT_TRUE(ParseUrl("stuns:hostname"));
+ EXPECT_EQ(1U, stun_configurations_.size());
+ EXPECT_EQ(0U, turn_configurations_.size());
+ stun_configurations_.clear();
+
+ EXPECT_TRUE(ParseUrl("turn:hostname"));
+ EXPECT_EQ(0U, stun_configurations_.size());
+ EXPECT_EQ(1U, turn_configurations_.size());
+ EXPECT_FALSE(turn_configurations_[0].secure);
+ turn_configurations_.clear();
+
+ EXPECT_TRUE(ParseUrl("turns:hostname"));
+ EXPECT_EQ(0U, stun_configurations_.size());
+ EXPECT_EQ(1U, turn_configurations_.size());
+ EXPECT_TRUE(turn_configurations_[0].secure);
+ turn_configurations_.clear();
+
+ // invalid prefixes
+ EXPECT_FALSE(ParseUrl("stunn:hostname"));
+ EXPECT_FALSE(ParseUrl(":hostname"));
+ EXPECT_FALSE(ParseUrl(":"));
+ EXPECT_FALSE(ParseUrl(""));
+}
+
+TEST_F(IceServerParsingTest, VerifyDefaults) {
+ // TURNS defaults
+ EXPECT_TRUE(ParseUrl("turns:hostname"));
+ EXPECT_EQ(1U, turn_configurations_.size());
+ EXPECT_EQ(5349, turn_configurations_[0].server.port());
+ EXPECT_EQ("tcp", turn_configurations_[0].transport_type);
+ turn_configurations_.clear();
+
+ // TURN defaults
+ EXPECT_TRUE(ParseUrl("turn:hostname"));
+ EXPECT_EQ(1U, turn_configurations_.size());
+ EXPECT_EQ(3478, turn_configurations_[0].server.port());
+ EXPECT_EQ("udp", turn_configurations_[0].transport_type);
+ turn_configurations_.clear();
+
+ // STUN defaults
+ EXPECT_TRUE(ParseUrl("stun:hostname"));
+ EXPECT_EQ(1U, stun_configurations_.size());
+ EXPECT_EQ(3478, stun_configurations_[0].server.port());
+ stun_configurations_.clear();
+}
+
+// Check that the 6 combinations of IPv4/IPv6/hostname and with/without port
+// can be parsed correctly.
+TEST_F(IceServerParsingTest, ParseHostnameAndPort) {
+ EXPECT_TRUE(ParseUrl("stun:1.2.3.4:1234"));
+ EXPECT_EQ(1U, stun_configurations_.size());
+ EXPECT_EQ("1.2.3.4", stun_configurations_[0].server.hostname());
+ EXPECT_EQ(1234, stun_configurations_[0].server.port());
+ stun_configurations_.clear();
+
+ EXPECT_TRUE(ParseUrl("stun:[1:2:3:4:5:6:7:8]:4321"));
+ EXPECT_EQ(1U, stun_configurations_.size());
+ EXPECT_EQ("1:2:3:4:5:6:7:8", stun_configurations_[0].server.hostname());
+ EXPECT_EQ(4321, stun_configurations_[0].server.port());
+ stun_configurations_.clear();
+
+ EXPECT_TRUE(ParseUrl("stun:hostname:9999"));
+ EXPECT_EQ(1U, stun_configurations_.size());
+ EXPECT_EQ("hostname", stun_configurations_[0].server.hostname());
+ EXPECT_EQ(9999, stun_configurations_[0].server.port());
+ stun_configurations_.clear();
+
+ EXPECT_TRUE(ParseUrl("stun:1.2.3.4"));
+ EXPECT_EQ(1U, stun_configurations_.size());
+ EXPECT_EQ("1.2.3.4", stun_configurations_[0].server.hostname());
+ EXPECT_EQ(3478, stun_configurations_[0].server.port());
+ stun_configurations_.clear();
+
+ EXPECT_TRUE(ParseUrl("stun:[1:2:3:4:5:6:7:8]"));
+ EXPECT_EQ(1U, stun_configurations_.size());
+ EXPECT_EQ("1:2:3:4:5:6:7:8", stun_configurations_[0].server.hostname());
+ EXPECT_EQ(3478, stun_configurations_[0].server.port());
+ stun_configurations_.clear();
+
+ EXPECT_TRUE(ParseUrl("stun:hostname"));
+ EXPECT_EQ(1U, stun_configurations_.size());
+ EXPECT_EQ("hostname", stun_configurations_[0].server.hostname());
+ EXPECT_EQ(3478, stun_configurations_[0].server.port());
+ stun_configurations_.clear();
+
+ // Try some invalid hostname:port strings.
+ EXPECT_FALSE(ParseUrl("stun:hostname:99a99"));
+ EXPECT_FALSE(ParseUrl("stun:hostname:-1"));
+ EXPECT_FALSE(ParseUrl("stun:hostname:"));
+ EXPECT_FALSE(ParseUrl("stun:[1:2:3:4:5:6:7:8]junk:1000"));
+ EXPECT_FALSE(ParseUrl("stun::5555"));
+ EXPECT_FALSE(ParseUrl("stun:"));
+}
+
+// Test parsing the "?transport=xxx" part of the URL.
+TEST_F(IceServerParsingTest, ParseTransport) {
+ EXPECT_TRUE(ParseUrl("turn:hostname:1234?transport=tcp"));
+ EXPECT_EQ(1U, turn_configurations_.size());
+ EXPECT_EQ("tcp", turn_configurations_[0].transport_type);
+ turn_configurations_.clear();
+
+ EXPECT_TRUE(ParseUrl("turn:hostname?transport=udp"));
+ EXPECT_EQ(1U, turn_configurations_.size());
+ EXPECT_EQ("udp", turn_configurations_[0].transport_type);
+ turn_configurations_.clear();
+
+ EXPECT_FALSE(ParseUrl("turn:hostname?transport=invalid"));
+}
+
+// Test parsing ICE username contained in URL.
+TEST_F(IceServerParsingTest, ParseUsername) {
+ EXPECT_TRUE(ParseUrl("turn:user@hostname"));
+ EXPECT_EQ(1U, turn_configurations_.size());
+ EXPECT_EQ("user", turn_configurations_[0].username);
+ turn_configurations_.clear();
+
+ EXPECT_FALSE(ParseUrl("turn:@hostname"));
+ EXPECT_FALSE(ParseUrl("turn:username@"));
+ EXPECT_FALSE(ParseUrl("turn:@"));
+ EXPECT_FALSE(ParseUrl("turn:user@name@hostname"));
+}
+
+// Test that username and password from IceServer is copied into the resulting
+// TurnConfiguration.
+TEST_F(IceServerParsingTest, CopyUsernameAndPasswordFromIceServer) {
+ EXPECT_TRUE(ParseUrl("turn:hostname", "username", "password"));
+ EXPECT_EQ(1U, turn_configurations_.size());
+ EXPECT_EQ("username", turn_configurations_[0].username);
+ EXPECT_EQ("password", turn_configurations_[0].password);
+}
+
+// Ensure that if a server has multiple URLs, each one is parsed.
+TEST_F(IceServerParsingTest, ParseMultipleUrls) {
+ PeerConnectionInterface::IceServers servers;
+ PeerConnectionInterface::IceServer server;
+ server.urls.push_back("stun:hostname");
+ server.urls.push_back("turn:hostname");
+ servers.push_back(server);
+ EXPECT_TRUE(webrtc::ParseIceServers(servers, &stun_configurations_,
+ &turn_configurations_));
+ EXPECT_EQ(1U, stun_configurations_.size());
+ EXPECT_EQ(1U, turn_configurations_.size());
+}
+
#endif // if !defined(THREAD_SANITIZER)
diff --git a/talk/app/webrtc/peerconnectionfactory_unittest.cc b/talk/app/webrtc/peerconnectionfactory_unittest.cc
index 8cf08f5..f1d5353 100644
--- a/talk/app/webrtc/peerconnectionfactory_unittest.cc
+++ b/talk/app/webrtc/peerconnectionfactory_unittest.cc
@@ -79,8 +79,6 @@
static const char kStunIceServerWithIPv6Address[] = "stun:[2401:fa00:4::]:1234";
static const char kStunIceServerWithIPv6AddressWithoutPort[] =
"stun:[2401:fa00:4::]";
-static const char kStunIceServerWithInvalidIPv6Address[] =
- "stun:[2401:fa00:4:::3478";
static const char kTurnIceServerWithIPv6Address[] =
"turn:test@[2401:fa00:4::]:1234";
@@ -208,7 +206,6 @@
TEST_F(PeerConnectionFactoryTest, CreatePCUsingIceServersUrls) {
PeerConnectionInterface::RTCConfiguration config;
webrtc::PeerConnectionInterface::IceServer ice_server;
- ice_server.urls.push_back(""); // Empty URLs should be ignored.
ice_server.urls.push_back(kStunIceServer);
ice_server.urls.push_back(kTurnIceServer);
ice_server.urls.push_back(kTurnIceServerWithTransport);
@@ -368,8 +365,6 @@
config.servers.push_back(ice_server);
ice_server.uri = kStunIceServerWithIPv6AddressWithoutPort;
config.servers.push_back(ice_server);
- ice_server.uri = kStunIceServerWithInvalidIPv6Address;
- config.servers.push_back(ice_server);
ice_server.uri = kTurnIceServerWithIPv6Address;
ice_server.password = kTurnPassword;
config.servers.push_back(ice_server);
diff --git a/talk/app/webrtc/peerconnectioninterface_unittest.cc b/talk/app/webrtc/peerconnectioninterface_unittest.cc
index 03ceaf2..5e135df 100644
--- a/talk/app/webrtc/peerconnectioninterface_unittest.cc
+++ b/talk/app/webrtc/peerconnectioninterface_unittest.cc
@@ -247,9 +247,11 @@
webrtc::MediaConstraintsInterface* constraints) {
PeerConnectionInterface::IceServer server;
PeerConnectionInterface::IceServers servers;
- server.uri = uri;
- server.password = password;
- servers.push_back(server);
+ if (!uri.empty()) {
+ server.uri = uri;
+ server.password = password;
+ servers.push_back(server);
+ }
port_allocator_factory_ = FakePortAllocatorFactory::Create();
@@ -281,6 +283,21 @@
EXPECT_EQ(PeerConnectionInterface::kStable, observer_.state_);
}
+ void CreatePeerConnectionExpectFail(const std::string& uri) {
+ PeerConnectionInterface::IceServer server;
+ PeerConnectionInterface::IceServers servers;
+ server.uri = uri;
+ servers.push_back(server);
+
+ scoped_ptr<webrtc::DtlsIdentityStoreInterface> dtls_identity_store;
+ port_allocator_factory_ = FakePortAllocatorFactory::Create();
+ scoped_refptr<PeerConnectionInterface> pc;
+ pc = pc_factory_->CreatePeerConnection(
+ servers, nullptr, port_allocator_factory_.get(),
+ dtls_identity_store.Pass(), &observer_);
+ ASSERT_EQ(nullptr, pc);
+ }
+
void CreatePeerConnectionWithDifferentConfigurations() {
CreatePeerConnection(kStunAddressOnly, "", NULL);
EXPECT_EQ(1u, port_allocator_factory_->stun_configs().size());
@@ -290,17 +307,9 @@
EXPECT_EQ(kDefaultStunPort,
port_allocator_factory_->stun_configs()[0].server.port());
- CreatePeerConnection(kStunInvalidPort, "", NULL);
- EXPECT_EQ(0u, port_allocator_factory_->stun_configs().size());
- EXPECT_EQ(0u, port_allocator_factory_->turn_configs().size());
-
- CreatePeerConnection(kStunAddressPortAndMore1, "", NULL);
- EXPECT_EQ(0u, port_allocator_factory_->stun_configs().size());
- EXPECT_EQ(0u, port_allocator_factory_->turn_configs().size());
-
- CreatePeerConnection(kStunAddressPortAndMore2, "", NULL);
- EXPECT_EQ(0u, port_allocator_factory_->stun_configs().size());
- EXPECT_EQ(0u, port_allocator_factory_->turn_configs().size());
+ CreatePeerConnectionExpectFail(kStunInvalidPort);
+ CreatePeerConnectionExpectFail(kStunAddressPortAndMore1);
+ CreatePeerConnectionExpectFail(kStunAddressPortAndMore2);
CreatePeerConnection(kTurnIceServerUri, kTurnPassword, NULL);
EXPECT_EQ(0u, port_allocator_factory_->stun_configs().size());