zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reland "ice server parsing: return RTCError with more details"

Browse Source 
This is a reland of commit c4b0bde7f2daabc4e0667fb73d096d1cf0819226
which changes the name of the new method and adds a deprecated
backward compatible variant with the old name.

Original change's description:
> ice server parsing: return RTCError with more details
>
> surfacing those errors to the API (without specific details) instead of just the logging.
>
> BUG=webrtc:14539
>
> Change-Id: Id907ebeb08b96b0e4225a016a37a12d58889091b
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278340
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Philipp Hancke <phancke@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#38356}

Bug: webrtc:14539
Change-Id: I0a5482e123f25867582d62101b31ed207b95ea1a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278962
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38364}
This commit is contained in:
Philipp Hancke
2022-10-12 10:16:14 +02:00
committed by WebRTC LUCI CQ
parent 77ec9f5015
commit 633dc2fd10
4 changed files with 81 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

93
pc/ice_server_parsing.cc
View File

@ -155,7 +155,7 @@ std::tuple<bool, absl::string_view, int> ParseHostnameAndPortFromString(
// Adds a STUN or TURN server to the appropriate list, // Adds a STUN or TURN server to the appropriate list,
// by parsing `url` and using the username/password in `server`. // by parsing `url` and using the username/password in `server`.
RTCErrorType ParseIceServerUrl( RTCError ParseIceServerUrl(
const PeerConnectionInterface::IceServer& server, const PeerConnectionInterface::IceServer& server,
absl::string_view url, absl::string_view url,
cricket::ServerAddresses* stun_servers, cricket::ServerAddresses* stun_servers,
@ -186,20 +186,24 @@ RTCErrorType ParseIceServerUrl(
std::vector<absl::string_view> transport_tokens = std::vector<absl::string_view> transport_tokens =
rtc::split(tokens[1], '='); rtc::split(tokens[1], '=');
if (transport_tokens[0] != kTransport) { if (transport_tokens[0] != kTransport) {
RTC_LOG(LS_WARNING) << "Invalid transport parameter key."; LOG_AND_RETURN_ERROR(
return RTCErrorType::SYNTAX_ERROR; RTCErrorType::SYNTAX_ERROR,
"ICE server parsing failed: Invalid transport parameter key.");
} }
if (transport_tokens.size() < 2) { if (transport_tokens.size() < 2) {
RTC_LOG(LS_WARNING) << "Transport parameter missing value."; LOG_AND_RETURN_ERROR(
return RTCErrorType::SYNTAX_ERROR; RTCErrorType::SYNTAX_ERROR,
"ICE server parsing failed: Transport parameter missing value.");
} }
absl::optional<cricket::ProtocolType> proto = absl::optional<cricket::ProtocolType> proto =
cricket::StringToProto(transport_tokens[1]); cricket::StringToProto(transport_tokens[1]);
if (!proto || if (!proto ||
(*proto != cricket::PROTO_UDP && *proto != cricket::PROTO_TCP)) { (*proto != cricket::PROTO_UDP && *proto != cricket::PROTO_TCP)) {
RTC_LOG(LS_WARNING) << "Transport parameter should always be udp or tcp."; LOG_AND_RETURN_ERROR(
return RTCErrorType::SYNTAX_ERROR; RTCErrorType::SYNTAX_ERROR,
"ICE server parsing failed: Transport parameter should "
"always be udp or tcp.");
} }
turn_transport_type = *proto; turn_transport_type = *proto;
} }
@ -207,8 +211,10 @@ RTCErrorType ParseIceServerUrl(
auto [service_type, hoststring] = auto [service_type, hoststring] =
GetServiceTypeAndHostnameFromUri(uri_without_transport); GetServiceTypeAndHostnameFromUri(uri_without_transport);
if (service_type == ServiceType::INVALID) { if (service_type == ServiceType::INVALID) {
RTC_LOG(LS_WARNING) << "Invalid transport parameter in ICE URI: " << url; RTC_LOG(LS_ERROR) << "Invalid transport parameter in ICE URI: " << url;
return RTCErrorType::SYNTAX_ERROR; LOG_AND_RETURN_ERROR(
RTCErrorType::SYNTAX_ERROR,
"ICE server parsing failed: Invalid transport parameter in ICE URI");
} }
// GetServiceTypeAndHostnameFromUri should never give an empty hoststring // GetServiceTypeAndHostnameFromUri should never give an empty hoststring
@ -221,22 +227,25 @@ RTCErrorType ParseIceServerUrl(
} }
if (hoststring.find('@') != absl::string_view::npos) { if (hoststring.find('@') != absl::string_view::npos) {
RTC_LOG(LS_WARNING) << "Invalid url: " << uri_without_transport; RTC_LOG(LS_ERROR) << "Invalid url with long deprecated user@host syntax: "
RTC_LOG(LS_WARNING) << uri_without_transport;
<< "Note that user-info@ in turn:-urls is long-deprecated."; LOG_AND_RETURN_ERROR(RTCErrorType::SYNTAX_ERROR,
return RTCErrorType::SYNTAX_ERROR; "ICE server parsing failed: Invalid url with long "
"deprecated user@host syntax");
} }
auto [success, address, port] = auto [success, address, port] =
ParseHostnameAndPortFromString(hoststring, default_port); ParseHostnameAndPortFromString(hoststring, default_port);
if (!success) { if (!success) {
RTC_LOG(LS_WARNING) << "Invalid hostname format: " << uri_without_transport; RTC_LOG(LS_ERROR) << "Invalid hostname format: " << uri_without_transport;
return RTCErrorType::SYNTAX_ERROR; LOG_AND_RETURN_ERROR(RTCErrorType::SYNTAX_ERROR,
"ICE server parsing failed: Invalid hostname format");
} }
if (port <= 0 || port > 0xffff) { if (port <= 0 || port > 0xffff) {
RTC_LOG(LS_WARNING) << "Invalid port: " << port; RTC_LOG(LS_ERROR) << "Invalid port: " << port;
return RTCErrorType::SYNTAX_ERROR; LOG_AND_RETURN_ERROR(RTCErrorType::SYNTAX_ERROR,
"ICE server parsing failed: Invalid port");
} }
switch (service_type) { switch (service_type) {
@ -249,8 +258,10 @@ RTCErrorType ParseIceServerUrl(
if (server.username.empty() || server.password.empty()) { if (server.username.empty() || server.password.empty()) {
// The WebRTC spec requires throwing an InvalidAccessError when username // The WebRTC spec requires throwing an InvalidAccessError when username
// or credential are ommitted; this is the native equivalent. // or credential are ommitted; this is the native equivalent.
RTC_LOG(LS_WARNING) << "TURN server with empty username or password"; LOG_AND_RETURN_ERROR(
return RTCErrorType::INVALID_PARAMETER; RTCErrorType::INVALID_PARAMETER,
"ICE server parsing failed: TURN server with empty "
"username or password");
} }
// If the hostname field is not empty, then the server address must be // If the hostname field is not empty, then the server address must be
// the resolved IP for that host, the hostname is needed later for TLS // the resolved IP for that host, the hostname is needed later for TLS
@ -263,10 +274,11 @@ RTCErrorType ParseIceServerUrl(
if (!IPFromString(address, &ip)) { if (!IPFromString(address, &ip)) {
// When hostname is set, the server address must be a // When hostname is set, the server address must be a
// resolved ip address. // resolved ip address.
RTC_LOG(LS_WARNING) LOG_AND_RETURN_ERROR(
<< "IceServer has hostname field set, but URI does not " RTCErrorType::INVALID_PARAMETER,
"contain an IP address."; "ICE server parsing failed: "
return RTCErrorType::INVALID_PARAMETER; "IceServer has hostname field set, but URI does not "
"contain an IP address.");
} }
socket_address.SetResolvedIP(ip); socket_address.SetResolvedIP(ip);
} }
@ -287,15 +299,16 @@ RTCErrorType ParseIceServerUrl(
default: default:
// We shouldn't get to this point with an invalid service_type, we should // We shouldn't get to this point with an invalid service_type, we should
// have returned an error already. // have returned an error already.
RTC_DCHECK_NOTREACHED() << "Unexpected service type"; LOG_AND_RETURN_ERROR(
return RTCErrorType::INTERNAL_ERROR; RTCErrorType::INTERNAL_ERROR,
"ICE server parsing failed: Unexpected service type");
} }
return RTCErrorType::NONE; return RTCError::OK();
} }
} // namespace } // namespace
RTCErrorType ParseIceServers( RTCError ParseIceServersOrError(
const PeerConnectionInterface::IceServers& servers, const PeerConnectionInterface::IceServers& servers,
cricket::ServerAddresses* stun_servers, cricket::ServerAddresses* stun_servers,
std::vector<cricket::RelayServerConfig>* turn_servers) { std::vector<cricket::RelayServerConfig>* turn_servers) {
@ -303,25 +316,26 @@ RTCErrorType ParseIceServers(
if (!server.urls.empty()) { if (!server.urls.empty()) {
for (const std::string& url : server.urls) { for (const std::string& url : server.urls) {
if (url.empty()) { if (url.empty()) {
RTC_LOG(LS_WARNING) << "Empty uri."; LOG_AND_RETURN_ERROR(RTCErrorType::SYNTAX_ERROR,
return RTCErrorType::SYNTAX_ERROR; "ICE server parsing failed: Empty uri.");
} }
RTCErrorType err = RTCError err =
ParseIceServerUrl(server, url, stun_servers, turn_servers); ParseIceServerUrl(server, url, stun_servers, turn_servers);
if (err != RTCErrorType::NONE) { if (!err.ok()) {
return err; return err;
} }
} }
} else if (!server.uri.empty()) { } else if (!server.uri.empty()) {
// Fallback to old .uri if new .urls isn't present. // Fallback to old .uri if new .urls isn't present.
RTCErrorType err = RTCError err =
ParseIceServerUrl(server, server.uri, stun_servers, turn_servers); ParseIceServerUrl(server, server.uri, stun_servers, turn_servers);
if (err != RTCErrorType::NONE) {
if (!err.ok()) {
return err; return err;
} }
} else { } else {
RTC_LOG(LS_WARNING) << "Empty uri."; LOG_AND_RETURN_ERROR(RTCErrorType::SYNTAX_ERROR,
return RTCErrorType::SYNTAX_ERROR; "ICE server parsing failed: Empty uri.");
} }
} }
// Candidates must have unique priorities, so that connectivity checks // Candidates must have unique priorities, so that connectivity checks
@ -331,7 +345,14 @@ RTCErrorType ParseIceServers(
// First in the list gets highest priority. // First in the list gets highest priority.
turn_server.priority = priority--; turn_server.priority = priority--;
} }
return RTCErrorType::NONE; return RTCError::OK();
}
RTCErrorType ParseIceServers(
const PeerConnectionInterface::IceServers& servers,
cricket::ServerAddresses* stun_servers,
std::vector<cricket::RelayServerConfig>* turn_servers) {
return ParseIceServersOrError(servers, stun_servers, turn_servers).type();
} }
} // namespace webrtc } // namespace webrtc

7
pc/ice_server_parsing.h
View File

@ -27,7 +27,12 @@ namespace webrtc {
// //
// Intended to be used to convert/validate the servers passed into a // Intended to be used to convert/validate the servers passed into a
// PeerConnection through RTCConfiguration. // PeerConnection through RTCConfiguration.
RTC_EXPORT RTCErrorType RTC_EXPORT RTCError
ParseIceServersOrError(const PeerConnectionInterface::IceServers& servers,
cricket::ServerAddresses* stun_servers,
std::vector<cricket::RelayServerConfig>* turn_servers);
[[deprecated("use ParseIceServersOrError")]] RTC_EXPORT RTCErrorType
ParseIceServers(const PeerConnectionInterface::IceServers& servers, ParseIceServers(const PeerConnectionInterface::IceServers& servers,
cricket::ServerAddresses* stun_servers, cricket::ServerAddresses* stun_servers,
std::vector<cricket::RelayServerConfig>* turn_servers); std::vector<cricket::RelayServerConfig>* turn_servers);

16
pc/ice_server_parsing_unittest.cc
View File

@ -62,8 +62,9 @@ class IceServerParsingTest : public ::testing::Test {
server.tls_cert_policy = tls_certificate_policy; server.tls_cert_policy = tls_certificate_policy;
server.hostname = hostname; server.hostname = hostname;
servers.push_back(server); servers.push_back(server);
return webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_) == return webrtc::ParseIceServersOrError(servers, &stun_servers_,
webrtc::RTCErrorType::NONE; &turn_servers_)
.ok();
} }
protected: protected:
@ -229,8 +230,9 @@ TEST_F(IceServerParsingTest, ParseMultipleUrls) {
server.username = "foo"; server.username = "foo";
server.password = "bar"; server.password = "bar";
servers.push_back(server); servers.push_back(server);
EXPECT_EQ(webrtc::RTCErrorType::NONE, EXPECT_TRUE(
webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_)); webrtc::ParseIceServersOrError(servers, &stun_servers_, &turn_servers_)
.ok());
EXPECT_EQ(1U, stun_servers_.size()); EXPECT_EQ(1U, stun_servers_.size());
EXPECT_EQ(1U, turn_servers_.size()); EXPECT_EQ(1U, turn_servers_.size());
} }
@ -245,8 +247,10 @@ TEST_F(IceServerParsingTest, TurnServerPrioritiesUnique) {
server.username = "foo"; server.username = "foo";
server.password = "bar"; server.password = "bar";
servers.push_back(server); servers.push_back(server);
EXPECT_EQ(webrtc::RTCErrorType::NONE,
webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_)); EXPECT_TRUE(
webrtc::ParseIceServersOrError(servers, &stun_servers_, &turn_servers_)
.ok());
EXPECT_EQ(2U, turn_servers_.size()); EXPECT_EQ(2U, turn_servers_.size());
EXPECT_NE(turn_servers_[0].priority, turn_servers_[1].priority); EXPECT_NE(turn_servers_[0].priority, turn_servers_[1].priority);
} }

16
pc/peer_connection.cc
View File

@ -596,10 +596,10 @@ RTCError PeerConnection::Initialize(
cricket::ServerAddresses stun_servers; cricket::ServerAddresses stun_servers;
std::vector<cricket::RelayServerConfig> turn_servers; std::vector<cricket::RelayServerConfig> turn_servers;
RTCErrorType parse_error = RTCError parse_error = ParseIceServersOrError(configuration.servers,
ParseIceServers(configuration.servers, &stun_servers, &turn_servers); &stun_servers, &turn_servers);
if (parse_error != RTCErrorType::NONE) { if (!parse_error.ok()) {
return RTCError(parse_error, "ICE server parse failed"); return parse_error;
} }
// Add the turn logging id to all turn servers // Add the turn logging id to all turn servers
@ -1519,10 +1519,10 @@ RTCError PeerConnection::SetConfiguration(
// Parse ICE servers before hopping to network thread. // Parse ICE servers before hopping to network thread.
cricket::ServerAddresses stun_servers; cricket::ServerAddresses stun_servers;
std::vector<cricket::RelayServerConfig> turn_servers; std::vector<cricket::RelayServerConfig> turn_servers;
RTCErrorType parse_error = RTCError parse_error = ParseIceServersOrError(configuration.servers,
ParseIceServers(configuration.servers, &stun_servers, &turn_servers); &stun_servers, &turn_servers);
if (parse_error != RTCErrorType::NONE) { if (!parse_error.ok()) {
return RTCError(parse_error, "ICE server parse failed"); return parse_error;
} }
// Add the turn logging id to all turn servers // Add the turn logging id to all turn servers
for (cricket::RelayServerConfig& turn_server : turn_servers) { for (cricket::RelayServerConfig& turn_server : turn_servers) {