From eb5e9aa2cea166455f4f157cf550e44626ede204 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Wed, 16 Nov 2016 18:18:08 +0900 Subject: [PATCH] DO NOT MERGE NetworkMonitor: send one DNS probe per web probe This patch changes sligthly the two web probes mechanism for captive portal detection and network validation so that DNS resolution is always done for both probes. In general the target web servers of the two parallel HTTP and HTTPS probes are now different. This introduces a bias in the latency measurement of th HTTPS probe since this latency will also include DNS resolution in general. Test: manual verification + $ runtest frameworks-net Bug: 32198726 (cherry picked from commit ab61e7c324b24e46829495bc2597e42ea907c53d) Change-Id: Ic83c3f8f46d32269bca1b90ae192ef648d5df6c3 --- .../server/connectivity/NetworkMonitor.java | 96 ++++++++++--------- 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java index 209ea647922c0..5e98859141697 100644 --- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java +++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java @@ -707,48 +707,13 @@ public class NetworkMonitor extends StateMachine { long startTime = SystemClock.elapsedRealtime(); - // Pre-resolve the captive portal server host so we can log it. - // Only do this if HttpURLConnection is about to, to avoid any potentially - // unnecessary resolution. - String hostToResolve = null; + final CaptivePortalProbeResult result; if (pacUrl != null) { - hostToResolve = pacUrl.getHost(); - } else if (proxyInfo != null) { - hostToResolve = proxyInfo.getHost(); - } else { - hostToResolve = httpUrl.getHost(); - } - - if (!TextUtils.isEmpty(hostToResolve)) { - String probeName = ValidationProbeEvent.getProbeName(ValidationProbeEvent.PROBE_DNS); - final Stopwatch dnsTimer = new Stopwatch().start(); - int dnsResult; - long dnsLatency; - try { - InetAddress[] addresses = mNetworkAgentInfo.network.getAllByName(hostToResolve); - dnsResult = ValidationProbeEvent.DNS_SUCCESS; - dnsLatency = dnsTimer.stop(); - final StringBuffer connectInfo = new StringBuffer(", " + hostToResolve + "="); - for (InetAddress address : addresses) { - connectInfo.append(address.getHostAddress()); - if (address != addresses[addresses.length-1]) connectInfo.append(","); - } - validationLog(probeName + " OK " + dnsLatency + "ms" + connectInfo); - } catch (UnknownHostException e) { - dnsResult = ValidationProbeEvent.DNS_FAILURE; - dnsLatency = dnsTimer.stop(); - validationLog(probeName + " FAIL " + dnsLatency + "ms, " + hostToResolve); - } - logValidationProbe(dnsLatency, ValidationProbeEvent.PROBE_DNS, dnsResult); - } - - CaptivePortalProbeResult result; - if (pacUrl != null) { - result = sendHttpProbe(pacUrl, ValidationProbeEvent.PROBE_PAC); + result = sendDnsAndHttpProbes(null, pacUrl, ValidationProbeEvent.PROBE_PAC); } else if (mUseHttps) { - result = sendParallelHttpProbes(httpsUrl, httpUrl, fallbackUrl); + result = sendParallelHttpProbes(proxyInfo, httpsUrl, httpUrl, fallbackUrl); } else { - result = sendHttpProbe(httpUrl, ValidationProbeEvent.PROBE_HTTP); + result = sendDnsAndHttpProbes(proxyInfo, httpUrl, ValidationProbeEvent.PROBE_HTTP); } long endTime = SystemClock.elapsedRealtime(); @@ -761,8 +726,50 @@ public class NetworkMonitor extends StateMachine { } /** - * Do a URL fetch on a known server to see if we get the data we expect. - * Returns HTTP response code. + * Do a DNS resolution and URL fetch on a known web server to see if we get the data we expect. + * @return a CaptivePortalProbeResult inferred from the HTTP response. + */ + private CaptivePortalProbeResult sendDnsAndHttpProbes(ProxyInfo proxy, URL url, int probeType) { + // Pre-resolve the captive portal server host so we can log it. + // Only do this if HttpURLConnection is about to, to avoid any potentially + // unnecessary resolution. + final String host = (proxy != null) ? proxy.getHost() : url.getHost(); + sendDnsProbe(host); + return sendHttpProbe(url, probeType); + } + + /** Do a DNS resolution of the given server. */ + private void sendDnsProbe(String host) { + if (TextUtils.isEmpty(host)) { + return; + } + + final String name = ValidationProbeEvent.getProbeName(ValidationProbeEvent.PROBE_DNS); + final Stopwatch watch = new Stopwatch().start(); + int result; + String connectInfo; + try { + InetAddress[] addresses = mNetworkAgentInfo.network.getAllByName(host); + result = ValidationProbeEvent.DNS_SUCCESS; + StringBuffer buffer = new StringBuffer(host).append("="); + for (InetAddress address : addresses) { + buffer.append(address.getHostAddress()); + if (address != addresses[addresses.length-1]) buffer.append(","); + } + connectInfo = buffer.toString(); + } catch (UnknownHostException e) { + result = ValidationProbeEvent.DNS_FAILURE; + connectInfo = host; + } + final long latency = watch.stop(); + String resultString = (ValidationProbeEvent.DNS_SUCCESS == result) ? "OK" : "FAIL"; + validationLog(String.format("%s %s %dms, %s", name, resultString, latency, connectInfo)); + logValidationProbe(latency, ValidationProbeEvent.PROBE_DNS, result); + } + + /** + * Do a URL fetch on a known web server to see if we get the data we expect. + * @return a CaptivePortalProbeResult inferred from the HTTP response. */ @VisibleForTesting protected CaptivePortalProbeResult sendHttpProbe(URL url, int probeType) { @@ -829,7 +836,7 @@ public class NetworkMonitor extends StateMachine { } private CaptivePortalProbeResult sendParallelHttpProbes( - URL httpsUrl, URL httpUrl, URL fallbackUrl) { + ProxyInfo proxy, URL httpsUrl, URL httpUrl, URL fallbackUrl) { // Number of probes to wait for. If a probe completes with a conclusive answer // it shortcuts the latch immediately by forcing the count to 0. final CountDownLatch latch = new CountDownLatch(2); @@ -849,9 +856,10 @@ public class NetworkMonitor extends StateMachine { @Override public void run() { if (mIsHttps) { - mResult = sendHttpProbe(httpsUrl, ValidationProbeEvent.PROBE_HTTPS); + mResult = + sendDnsAndHttpProbes(proxy, httpsUrl, ValidationProbeEvent.PROBE_HTTPS); } else { - mResult = sendHttpProbe(httpUrl, ValidationProbeEvent.PROBE_HTTP); + mResult = sendDnsAndHttpProbes(proxy, httpUrl, ValidationProbeEvent.PROBE_HTTP); } if ((mIsHttps && mResult.isSuccessful()) || (!mIsHttps && mResult.isPortal())) { // Stop waiting immediately if https succeeds or if http finds a portal.