From 2e43e45e1e108fa4ac9ee66da7de7a4447e7bad7 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Thu, 13 Apr 2017 12:43:49 +0000 Subject: [PATCH] Revert "Captive portal: rotate fallback urls" After discussion in the cl we agreed to use "|" as a separator. However String.split() input arg is a regex and not a literal string, so that "|" will actually split the urls characters per characters. Will revert and resubmit with a comma. This reverts commit 0908daaaf00e6b56ebed0a0fce9c3e3fe183a06b. Change-Id: Ifab25e41bec806fbc1d2c13ffd81d4ad91598c89 --- core/java/android/provider/Settings.java | 9 ---- .../server/connectivity/NetworkMonitor.java | 46 ++++--------------- 2 files changed, 9 insertions(+), 46 deletions(-) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 60acbafe5895f..8e55f4b804cfc 100755 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -8182,15 +8182,6 @@ public final class Settings { */ public static final String CAPTIVE_PORTAL_FALLBACK_URL = "captive_portal_fallback_url"; - /** - * A "|" separated list of URLs used for captive portal detection in addition to the - * fallback HTTP url associated with the CAPTIVE_PORTAL_FALLBACK_URL settings. - * - * @hide - */ - public static final String CAPTIVE_PORTAL_OTHER_FALLBACK_URLS = - "captive_portal_other_fallback_urls"; - /** * Whether to use HTTPS for network validation. This is enabled by default and the setting * needs to be set to 0 to disable it. This setting is a misnomer because captive portals diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java index f5b2b77765f71..3f2ca1440508d 100644 --- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java +++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java @@ -70,7 +70,6 @@ import java.net.InetAddress; import java.net.MalformedURLException; import java.net.URL; import java.net.UnknownHostException; -import java.util.ArrayList; import java.util.List; import java.util.Random; import java.util.concurrent.CountDownLatch; @@ -91,8 +90,6 @@ public class NetworkMonitor extends StateMachine { private static final String DEFAULT_HTTP_URL = "http://connectivitycheck.gstatic.com/generate_204"; private static final String DEFAULT_FALLBACK_URL = "http://www.google.com/gen_204"; - private static final String DEFAULT_OTHER_FALLBACK_URLS = - "http://play.googleapis.com/generate_204"; private static final String DEFAULT_USER_AGENT = "Mozilla/5.0 (X11; Linux x86_64) " + "AppleWebKit/537.36 (KHTML, like Gecko) " + "Chrome/52.0.2743.82 Safari/537.36"; @@ -266,8 +263,7 @@ public class NetworkMonitor extends StateMachine { private final String mCaptivePortalUserAgent; private final URL mCaptivePortalHttpsUrl; private final URL mCaptivePortalHttpUrl; - private final URL[] mCaptivePortalFallbackUrls; - private int mNextFallbackUrlIndex = 0; + private final URL mCaptivePortalFallbackUrl; public NetworkMonitor(Context context, Handler handler, NetworkAgentInfo networkAgentInfo, NetworkRequest defaultRequest) { @@ -306,7 +302,7 @@ public class NetworkMonitor extends StateMachine { mCaptivePortalUserAgent = getCaptivePortalUserAgent(context); mCaptivePortalHttpsUrl = makeURL(getCaptivePortalServerHttpsUrl(context)); mCaptivePortalHttpUrl = makeURL(getCaptivePortalServerHttpUrl(context)); - mCaptivePortalFallbackUrls = makeCaptivePortalFallbackUrls(context); + mCaptivePortalFallbackUrl = makeURL(getCaptivePortalFallbackUrl(context)); start(); } @@ -454,7 +450,7 @@ public class NetworkMonitor extends StateMachine { intent.putExtra(ConnectivityManager.EXTRA_CAPTIVE_PORTAL_URL, mLastPortalProbeResult.detectUrl); intent.putExtra(ConnectivityManager.EXTRA_CAPTIVE_PORTAL_USER_AGENT, - mCaptivePortalUserAgent); + getCaptivePortalUserAgent(mContext)); intent.setFlags( Intent.FLAG_ACTIVITY_BROUGHT_TO_FRONT | Intent.FLAG_ACTIVITY_NEW_TASK); mContext.startActivityAsUser(intent, UserHandle.CURRENT); @@ -670,24 +666,9 @@ public class NetworkMonitor extends StateMachine { return getSetting(context, Settings.Global.CAPTIVE_PORTAL_HTTP_URL, DEFAULT_HTTP_URL); } - private URL[] makeCaptivePortalFallbackUrls(Context context) { - String separator = "|"; - String firstUrl = getSetting(context, + private static String getCaptivePortalFallbackUrl(Context context) { + return getSetting(context, Settings.Global.CAPTIVE_PORTAL_FALLBACK_URL, DEFAULT_FALLBACK_URL); - String joinedUrls = firstUrl + separator + getSetting(context, - Settings.Global.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS, DEFAULT_OTHER_FALLBACK_URLS); - List urls = new ArrayList<>(); - for (String s : joinedUrls.split(separator)) { - URL u = makeURL(s); - if (u == null) { - continue; - } - urls.add(u); - } - if (urls.isEmpty()) { - Log.e(TAG, String.format("could not create any url from %s", joinedUrls)); - } - return urls.toArray(new URL[urls.size()]); } private static String getCaptivePortalUserAgent(Context context) { @@ -699,15 +680,6 @@ public class NetworkMonitor extends StateMachine { return value != null ? value : defaultValue; } - private URL nextFallbackUrl() { - if (mCaptivePortalFallbackUrls.length == 0) { - return null; - } - int idx = Math.abs(mNextFallbackUrlIndex) % mCaptivePortalFallbackUrls.length; - mNextFallbackUrlIndex += new Random().nextInt(); // randomely change url without memory. - return mCaptivePortalFallbackUrls[idx]; - } - @VisibleForTesting protected CaptivePortalProbeResult isCaptivePortal() { if (!mIsCaptivePortalCheckEnabled) { @@ -718,6 +690,7 @@ public class NetworkMonitor extends StateMachine { 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: @@ -754,7 +727,7 @@ public class NetworkMonitor extends StateMachine { if (pacUrl != null) { result = sendDnsAndHttpProbes(null, pacUrl, ValidationProbeEvent.PROBE_PAC); } else if (mUseHttps) { - result = sendParallelHttpProbes(proxyInfo, httpsUrl, httpUrl); + result = sendParallelHttpProbes(proxyInfo, httpsUrl, httpUrl, fallbackUrl); } else { result = sendDnsAndHttpProbes(proxyInfo, httpUrl, ValidationProbeEvent.PROBE_HTTP); } @@ -888,7 +861,7 @@ public class NetworkMonitor extends StateMachine { } private CaptivePortalProbeResult sendParallelHttpProbes( - ProxyInfo proxy, URL httpsUrl, URL httpUrl) { + 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); @@ -947,8 +920,7 @@ public class NetworkMonitor extends StateMachine { if (httpsResult.isPortal() || httpsResult.isSuccessful()) { return httpsResult; } - // If a fallback url exists, use a fallback probe to try again portal detection. - URL fallbackUrl = nextFallbackUrl(); + // If a fallback url is specified, use a fallback probe to try again portal detection. if (fallbackUrl != null) { CaptivePortalProbeResult result = sendHttpProbe(fallbackUrl, ValidationProbeEvent.PROBE_FALLBACK);