[remoting host] Allow route-changes to have invalid IP addresses.
WebRTC sometimes provides selected-candidate-changed notifications with
invalid IP addresses. The host process normally records these in the
system event log, but only if both IPs are valid. With this CL,
route-change events are generated even if the local or peer IP address
is invalid. These appear in the event log as "unknown".
(cherry picked from commit 6cd55a889f8400942308e76eb5c69bf41d3823ca)
Bug: 1128667
Change-Id: I9329092bdb7f7e4cf6d845303f321bff3eb3177b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485576
Commit-Queue: Lambros Lambrou <[email protected]>
Commit-Queue: Jamie Walch <[email protected]>
Auto-Submit: Lambros Lambrou <[email protected]>
Reviewed-by: Jamie Walch <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#818770}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2487813
Reviewed-by: Lambros Lambrou <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#551}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/remoting/host/host_event_logger_posix.cc b/remoting/host/host_event_logger_posix.cc
index 868baa7..abe4a15 100644
--- a/remoting/host/host_event_logger_posix.cc
+++ b/remoting/host/host_event_logger_posix.cc
@@ -88,8 +88,14 @@
Log(base::StringPrintf(
"Channel IP for client: %s ip='%s' host_ip='%s' channel='%s' "
"connection='%s'",
- jid.c_str(), route.remote_address.ToString().c_str(),
- route.local_address.ToString().c_str(), channel_name.c_str(),
+ jid.c_str(),
+ route.remote_address.address().IsValid()
+ ? route.remote_address.ToString().c_str()
+ : "unknown",
+ route.local_address.address().IsValid()
+ ? route.local_address.ToString().c_str()
+ : "unknown",
+ channel_name.c_str(),
protocol::TransportRoute::GetTypeString(route.type).c_str()));
}
diff --git a/remoting/host/host_event_logger_win.cc b/remoting/host/host_event_logger_win.cc
index 67dc453..62a697f 100644
--- a/remoting/host/host_event_logger_win.cc
+++ b/remoting/host/host_event_logger_win.cc
@@ -96,8 +96,12 @@
const protocol::TransportRoute& route) {
std::vector<std::string> strings(5);
strings[0] = jid;
- strings[1] = route.remote_address.ToString();
- strings[2] = route.local_address.ToString();
+ strings[1] = route.remote_address.address().IsValid()
+ ? route.remote_address.ToString()
+ : "unknown";
+ strings[2] = route.local_address.address().IsValid()
+ ? route.local_address.ToString()
+ : "unknown";
strings[3] = channel_name;
strings[4] = protocol::TransportRoute::GetTypeString(route.type);
Log(EVENTLOG_INFORMATION_TYPE, MSG_HOST_CLIENT_ROUTING_CHANGED, strings);
diff --git a/remoting/host/ipc_host_event_logger.cc b/remoting/host/ipc_host_event_logger.cc
index 927d75f..51144eb 100644
--- a/remoting/host/ipc_host_event_logger.cc
+++ b/remoting/host/ipc_host_event_logger.cc
@@ -58,10 +58,19 @@
SerializedTransportRoute serialized_route;
serialized_route.type = route.type;
- serialized_route.remote_ip =
- route.remote_address.address().CopyBytesToVector();
+
+ // If the remote (or local) IP address is invalid, send it over IPC as an
+ // empty vector. The receiving process has a CHECK() that the address is
+ // either valid or empty.
+ if (route.remote_address.address().IsValid()) {
+ serialized_route.remote_ip =
+ route.remote_address.address().CopyBytesToVector();
+ }
serialized_route.remote_port = route.remote_address.port();
- serialized_route.local_ip = route.local_address.address().CopyBytesToVector();
+ if (route.local_address.address().IsValid()) {
+ serialized_route.local_ip =
+ route.local_address.address().CopyBytesToVector();
+ }
serialized_route.local_port = route.local_address.port();
daemon_channel_->Send(new ChromotingNetworkDaemonMsg_ClientRouteChange(
jid, channel_name, serialized_route));
diff --git a/remoting/protocol/webrtc_transport.cc b/remoting/protocol/webrtc_transport.cc
index d08c247..12a351d 100644
--- a/remoting/protocol/webrtc_transport.cc
+++ b/remoting/protocol/webrtc_transport.cc
@@ -1069,15 +1069,25 @@
std::max(CandidateTypeToTransportRouteType(local_candidate.type()),
CandidateTypeToTransportRouteType(remote_candidate.type()));
+ VLOG(0) << "Selected candidate-pair changed, reason = " << event.reason;
+ VLOG(0) << " Local IP = " << local_candidate.address().ToString()
+ << ", type = " << local_candidate.type()
+ << ", protocol = " << local_candidate.protocol();
+ VLOG(0) << " Remote IP = " << remote_candidate.address().ToString()
+ << ", type = " << remote_candidate.type()
+ << ", protocol = " << remote_candidate.protocol();
+
+ // Try to convert local and peer addresses. These may sometimes be invalid,
+ // for example, a "relay" or "prflx" candidate from a relay connection
+ // might have the IP address stripped away by WebRTC - see
+ // http://crbug.com/1128667.
if (!jingle_glue::SocketAddressToIPEndPoint(remote_candidate.address(),
&route.remote_address)) {
- LOG(ERROR) << "Failed to convert peer IP address.";
- return;
+ VLOG(0) << "Peer IP address is invalid.";
}
if (!jingle_glue::SocketAddressToIPEndPoint(local_candidate.address(),
&route.local_address)) {
- LOG(ERROR) << "Failed to convert local IP address.";
- return;
+ VLOG(0) << "Local IP address is invalid.";
}
event_handler_->OnWebrtcTransportRouteChanged(route);
