From d9ac87ed65aa12b93e0b70f5e8560562c850fde0 Mon Sep 17 00:00:00 2001
From: Hugo Benichi <hugobenichi@google.com>
Date: Thu, 6 Apr 2017 14:36:39 +0900
Subject: [PATCH] NetworkMonitor: improve captive portal validation logs

This patch adds probe type and url to all validation logs about web
probes sent for captive portal detection.

Test: built, flashed, looked at $ adb shell dumpsys connectivity
Bug: 36830206, 36532213
Change-Id: Iee2caaf6664f3d097f9d1888ccc5fece0a95994c
---
 .../server/connectivity/NetworkMonitor.java   | 35 +++++++++++--------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java
index 97669d242f063..6cf8f37b76045 100644
--- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java
+++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java
@@ -301,6 +301,11 @@ public class NetworkMonitor extends StateMachine {
         if (DBG) Log.d(TAG + "/" + mNetworkAgentInfo.name(), s);
     }
 
+    private void validationLog(int probeType, Object url, String msg) {
+        String probeName = ValidationProbeEvent.getProbeName(probeType);
+        validationLog(String.format("%s %s %s", probeName, url, msg));
+    }
+
     private void validationLog(String s) {
         if (DBG) log(s);
         validationLogs.log(s);
@@ -752,20 +757,19 @@ public class NetworkMonitor extends StateMachine {
         String connectInfo;
         try {
             InetAddress[] addresses = mNetworkAgentInfo.network.getAllByName(host);
-            result = ValidationProbeEvent.DNS_SUCCESS;
-            StringBuffer buffer = new StringBuffer(host).append("=");
+            StringBuffer buffer = new StringBuffer();
             for (InetAddress address : addresses) {
-                buffer.append(address.getHostAddress());
-                if (address != addresses[addresses.length-1]) buffer.append(",");
+                buffer.append(',').append(address.getHostAddress());
             }
-            connectInfo = buffer.toString();
+            result = ValidationProbeEvent.DNS_SUCCESS;
+            connectInfo = "OK " + buffer.substring(1);
         } catch (UnknownHostException e) {
             result = ValidationProbeEvent.DNS_FAILURE;
-            connectInfo = host;
+            connectInfo = "FAIL";
         }
         final long latency = watch.stop();
-        String resultString = (ValidationProbeEvent.DNS_SUCCESS == result) ? "OK" : "FAIL";
-        validationLog(String.format("%s %s %dms, %s", name, resultString, latency, connectInfo));
+        validationLog(ValidationProbeEvent.PROBE_DNS, host,
+                String.format("%dms %s", latency, connectInfo));
         logValidationProbe(latency, ValidationProbeEvent.PROBE_DNS, result);
     }
 
@@ -787,7 +791,7 @@ public class NetworkMonitor extends StateMachine {
             urlConnection.setUseCaches(false);
             final String userAgent = getCaptivePortalUserAgent(mContext);
             if (userAgent != null) {
-               urlConnection.setRequestProperty("User-Agent", userAgent);
+                urlConnection.setRequestProperty("User-Agent", userAgent);
             }
             // cannot read request header after connection
             String requestHeader = urlConnection.getRequestProperties().toString();
@@ -801,8 +805,7 @@ public class NetworkMonitor extends StateMachine {
             // Time how long it takes to get a response to our request
             long responseTimestamp = SystemClock.elapsedRealtime();
 
-            validationLog(ValidationProbeEvent.getProbeName(probeType) + " " + url +
-                    " time=" + (responseTimestamp - requestTimestamp) + "ms" +
+            validationLog(probeType, url, "time=" + (responseTimestamp - requestTimestamp) + "ms" +
                     " ret=" + httpResponseCode +
                     " request=" + requestHeader +
                     " headers=" + urlConnection.getHeaderFields());
@@ -814,27 +817,29 @@ public class NetworkMonitor extends StateMachine {
             // proxy server.
             if (httpResponseCode == 200) {
                 if (probeType == ValidationProbeEvent.PROBE_PAC) {
-                    validationLog("PAC fetch 200 response interpreted as 204 response.");
+                    validationLog(
+                            probeType, url, "PAC fetch 200 response interpreted as 204 response.");
                     httpResponseCode = 204;
                 } else if (urlConnection.getContentLengthLong() == 0) {
                     // Consider 200 response with "Content-length=0" to not be a captive portal.
                     // There's no point in considering this a captive portal as the user cannot
                     // sign-in to an empty page. Probably the result of a broken transparent proxy.
                     // See http://b/9972012.
-                    validationLog(
+                    validationLog(probeType, url,
                         "200 response with Content-length=0 interpreted as 204 response.");
                     httpResponseCode = 204;
                 } else if (urlConnection.getContentLengthLong() == -1) {
                     // When no Content-length (default value == -1), attempt to read a byte from the
                     // response. Do not use available() as it is unreliable. See http://b/33498325.
                     if (urlConnection.getInputStream().read() == -1) {
-                        validationLog("Empty 200 response interpreted as 204 response.");
+                        validationLog(
+                                probeType, url, "Empty 200 response interpreted as 204 response.");
                         httpResponseCode = 204;
                     }
                 }
             }
         } catch (IOException e) {
-            validationLog("Probably not a portal: exception " + e);
+            validationLog(probeType, url, "Probably not a portal: exception " + e);
             if (httpResponseCode == 599) {
                 // TODO: Ping gateway and DNS server and log results.
             }