From b03272c9b5d1b5c71ae591f0e38b1d139d11f19f Mon Sep 17 00:00:00 2001
From: Hugo Benichi <hugobenichi@google.com>
Date: Mon, 10 Apr 2017 22:45:13 +0900
Subject: [PATCH] Captive portal: fix probe urls at network creation

This patch changes how url configuration values for captive portal
detection are read: instead of re-reading the settings at every probe
detection, the settings are read once when the NetworkMonitor associated
to a network is created.

If the settings are updated, the new values are picked up by new
networks, but not by existing networks. Since captive portal detection
is most important when joining WiFi networks, the newest available
settings values will still be used.

This change prepares introducing further changes for rotating probe
urls. Especially it helps making the detection strategy stateful with
respect to the lifecycle of a single NetworkMonitor.

Test: built, flashed, tested manually with various portal networks
Bug: 36532213
Change-Id: I71cc8bb8b996462f27f50798d67bceee5ffb898d
---
 .../server/connectivity/NetworkMonitor.java   | 30 ++++++++++++-------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java
index 6cf8f37b76045..3f2ca1440508d 100644
--- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java
+++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java
@@ -259,6 +259,12 @@ public class NetworkMonitor extends StateMachine {
     // This variable is set before transitioning to the mCaptivePortalState.
     private CaptivePortalProbeResult mLastPortalProbeResult = CaptivePortalProbeResult.FAILED;
 
+    // Configuration values for captive portal detection probes.
+    private final String mCaptivePortalUserAgent;
+    private final URL mCaptivePortalHttpsUrl;
+    private final URL mCaptivePortalHttpUrl;
+    private final URL mCaptivePortalFallbackUrl;
+
     public NetworkMonitor(Context context, Handler handler, NetworkAgentInfo networkAgentInfo,
             NetworkRequest defaultRequest) {
         this(context, handler, networkAgentInfo, defaultRequest, new IpConnectivityLog());
@@ -293,6 +299,11 @@ public class NetworkMonitor extends StateMachine {
         mUseHttps = Settings.Global.getInt(mContext.getContentResolver(),
                 Settings.Global.CAPTIVE_PORTAL_USE_HTTPS, 1) == 1;
 
+        mCaptivePortalUserAgent = getCaptivePortalUserAgent(context);
+        mCaptivePortalHttpsUrl = makeURL(getCaptivePortalServerHttpsUrl(context));
+        mCaptivePortalHttpUrl = makeURL(getCaptivePortalServerHttpUrl(context));
+        mCaptivePortalFallbackUrl = makeURL(getCaptivePortalFallbackUrl(context));
+
         start();
     }
 
@@ -676,7 +687,10 @@ public class NetworkMonitor extends StateMachine {
             return new CaptivePortalProbeResult(204);
         }
 
-        URL pacUrl = null, httpsUrl = null, httpUrl = null, fallbackUrl = null;
+        URL pacUrl = null;
+        URL httpsUrl = mCaptivePortalHttpsUrl;
+        URL httpUrl = mCaptivePortalHttpUrl;
+        URL fallbackUrl = mCaptivePortalFallbackUrl;
 
         // On networks with a PAC instead of fetching a URL that should result in a 204
         // response, we instead simply fetch the PAC script.  This is done for a few reasons:
@@ -703,13 +717,8 @@ public class NetworkMonitor extends StateMachine {
             }
         }
 
-        if (pacUrl == null) {
-            httpsUrl = makeURL(getCaptivePortalServerHttpsUrl(mContext));
-            httpUrl = makeURL(getCaptivePortalServerHttpUrl(mContext));
-            fallbackUrl = makeURL(getCaptivePortalFallbackUrl(mContext));
-            if (httpUrl == null || httpsUrl == null) {
-                return CaptivePortalProbeResult.FAILED;
-            }
+        if ((pacUrl == null) && (httpUrl == null || httpsUrl == null)) {
+            return CaptivePortalProbeResult.FAILED;
         }
 
         long startTime = SystemClock.elapsedRealtime();
@@ -789,9 +798,8 @@ public class NetworkMonitor extends StateMachine {
             urlConnection.setConnectTimeout(SOCKET_TIMEOUT_MS);
             urlConnection.setReadTimeout(SOCKET_TIMEOUT_MS);
             urlConnection.setUseCaches(false);
-            final String userAgent = getCaptivePortalUserAgent(mContext);
-            if (userAgent != null) {
-                urlConnection.setRequestProperty("User-Agent", userAgent);
+            if (mCaptivePortalUserAgent != null) {
+                urlConnection.setRequestProperty("User-Agent", mCaptivePortalUserAgent);
             }
             // cannot read request header after connection
             String requestHeader = urlConnection.getRequestProperties().toString();