Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 44ee27b1 authored by Chiachang Wang's avatar Chiachang Wang
Browse files

Update default value of probe url to be a constant 

Default value for probing url should be a constant in
NetworkStack but not an overlayable config due to some
technical limitations. OEMs can mistakenly override configs
that were not designed to be overridden.

Bug: 152730542
Test: atest NetworkStackTests NetworkStackNextTests
Change-Id: I1846958e7c4e8b64ae287718c63e705bb232632a
Merged-In: I1846958e7c4e8b64ae287718c63e705bb232632a
parent 05137ff6

res/values-mcc460/config.xml

+12 −0
Original line number Diff line number Diff line
@@ -5,7 +5,19 @@ 
         general case as this could degrade the user experience (portals not detected properly).

         However in China the default URLs are not accessible in general. The below alternatives

         should allow users to connect to local networks normally. -->

    <!-- default_captive_portal_https_url is not configured as overlayable so

         OEMs that wish to change captive_portal_https_url configuration must

         do so via configuring runtime resource overlay to

         config_captive_portal_https_url and *NOT* by changing or overlaying

         this resource. It will break if the enforcement of overlayable starts.

         -->

    <string name="default_captive_portal_https_url" translatable="false">https://connectivitycheck.gstatic.com/generate_204</string>

    <!-- default_captive_portal_fallback_urls is not configured as overlayable

         so OEMs that wish to change captive_portal_fallback_urls configuration

         must do so via configuring runtime resource overlay to

         config_captive_portal_fallback_urls and *NOT* by changing or overlaying

         this resource. It will break if the enforcement of overlayable starts.

         -->

    <string-array name="default_captive_portal_fallback_urls" translatable="false">

        <item>http://www.googleapis.cn/generate_204</item>

    </string-array>

res/values/config.xml

+18 −12
Original line number Diff line number Diff line
@@ -6,28 +6,34 @@ 
    The overlays must apply to the config_* values, not the default_* values. The default_*

    values are meant to be the default when no other configuration is specified.

    -->



    <!-- DNS probe timeout for network validation. Enough for 3 DNS queries 5 seconds apart. -->

    <integer name="default_captive_portal_dns_probe_timeout">12500</integer>



    <!-- HTTP URL for network validation, to use for detecting captive portals. -->

    <!-- default_captive_portal_http_url is not configured as overlayable so

         OEMs that wish to change captive_portal_http_url configuration must

         do so via configuring runtime resource overlay to

         config_captive_portal_http_url and *NOT* by changing or overlaying

         this resource. It will break if the enforcement of overlayable starts.

         -->

    <string name="default_captive_portal_http_url" translatable="false">http://connectivitycheck.gstatic.com/generate_204</string>



    <!-- HTTPS URL for network validation, to use for confirming internet connectivity. -->

    <!-- default_captive_portal_https_url is not configured as overlayable so

         OEMs that wish to change captive_portal_https_url configuration must

         do so via configuring runtime resource overlay to

         config_captive_portal_https_url and *NOT* by changing or overlaying

         this resource. It will break if the enforcement of overlayable starts.

         -->

    <string name="default_captive_portal_https_url" translatable="false">https://www.google.com/generate_204</string>



    <!-- List of fallback URLs to use for detecting captive portals. -->

    <!-- default_captive_portal_fallback_urls is not configured as overlayable

         so OEMs that wish to change captive_portal_fallback_urls configuration

         must do so via configuring runtime resource overlay to

         config_captive_portal_fallback_urls and *NOT* by changing or overlaying

         this resource. It will break if the enforcement of overlayable starts.

         -->

    <string-array name="default_captive_portal_fallback_urls" translatable="false">

        <item>http://www.google.com/gen_204</item>

        <item>http://play.googleapis.com/generate_204</item>

    </string-array>



    <!-- List of fallback probe specs to use for detecting captive portals.

         This is an alternative to fallback URLs that provides more flexibility on detection rules.

         Empty, so unused by default. -->

    <string-array name="default_captive_portal_fallback_probe_specs" translatable="false">

    </string-array>



    <!-- Configuration hooks for the above settings.

         Empty by default but may be overridden by RROs. -->

    <integer name="config_captive_portal_dns_probe_timeout"></integer>

src/android/net/util/NetworkStackUtils.java

+14 −0
Original line number Diff line number Diff line
@@ -154,17 +154,31 @@ public class NetworkStackUtils { 
     */

    public static final int CAPTIVE_PORTAL_MODE_AVOID = 2;



    /**

     * DNS probe timeout for network validation. Enough for 3 DNS queries 5 seconds apart.

     */

    public static final int DEFAULT_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT = 12500;



    /**

     * List of fallback probe specs to use for detecting captive portals. This is an alternative to

     * fallback URLs that provides more flexibility on detection rules. Empty, so unused by default.

     */

    public static final String[] DEFAULT_CAPTIVE_PORTAL_FALLBACK_PROBE_SPECS =

            new String[] {};



    /**

     * The default list of HTTP URLs to use for detecting captive portals.

     */

    public static final String[] DEFAULT_CAPTIVE_PORTAL_HTTP_URLS =

            new String [] {"http://connectivitycheck.gstatic.com/generate_204"};



    /**

     * The default list of HTTPS URLs for network validation, to use for confirming internet

     * connectivity.

     */

    public static final String[] DEFAULT_CAPTIVE_PORTAL_HTTPS_URLS =

            new String [] {"https://www.google.com/generate_204"};



    /**

     * @deprecated Considering boolean experiment flag is likely to cause misconfiguration

     *             particularly when NetworkStack module rolls back to previous version. It's

src/com/android/server/connectivity/NetworkMonitor.java

+27 −20
Original line number Diff line number Diff line
@@ -67,6 +67,8 @@ import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_OTHER_HTTPS_URLS 
import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_OTHER_HTTP_URLS;

import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_USER_AGENT;

import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_USE_HTTPS;

import static android.net.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT;

import static android.net.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_FALLBACK_PROBE_SPECS;

import static android.net.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_HTTPS_URLS;

import static android.net.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_HTTP_URLS;

import static android.net.util.NetworkStackUtils.DISMISS_PORTAL_IN_VALIDATED_NETWORK;
@@ -1801,8 +1803,9 @@ public class NetworkMonitor extends StateMachine { 
        final String testUrl = getTestUrl(TEST_CAPTIVE_PORTAL_HTTPS_URL);

        if (isValidTestUrl(testUrl)) return testUrl;

        final Context targetContext = getCustomizedContextOrDefault();

        return getSettingFromResource(targetContext, R.string.config_captive_portal_https_url,

                R.string.default_captive_portal_https_url, CAPTIVE_PORTAL_HTTPS_URL);

        return getSettingFromResource(targetContext,

                R.string.config_captive_portal_https_url, CAPTIVE_PORTAL_HTTPS_URL,

                targetContext.getResources().getString(R.string.default_captive_portal_https_url));

    }



    private static boolean isValidTestUrl(@Nullable String url) {
@@ -1819,14 +1822,13 @@ public class NetworkMonitor extends StateMachine { 


    private int getDnsProbeTimeout() {

        return getIntSetting(mContext, R.integer.config_captive_portal_dns_probe_timeout,

                CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT,

                R.integer.default_captive_portal_dns_probe_timeout);

                CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT, DEFAULT_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT);

    }



    /**

     * Gets an integer setting from resources or device config

     *

     * configResource is used if set, followed by device config if set, followed by defaultResource.

     * configResource is used if set, followed by device config if set, followed by defaultValue.

     * If none of these are set then an exception is thrown.

     *

     * TODO: move to a common location such as a ConfigUtils class.
@@ -1834,13 +1836,13 @@ public class NetworkMonitor extends StateMachine { 
     */

    @VisibleForTesting

    int getIntSetting(@NonNull final Context context, @StringRes int configResource,

            @NonNull String symbol, @StringRes int defaultResource) {

            @NonNull String symbol, int defaultValue) {

        final Resources res = context.getResources();

        try {

            return res.getInteger(configResource);

        } catch (Resources.NotFoundException e) {

            return mDependencies.getDeviceConfigPropertyInt(NAMESPACE_CONNECTIVITY,

                    symbol, res.getInteger(defaultResource));

                    symbol, defaultValue);

        }

    }
@@ -1894,8 +1896,9 @@ public class NetworkMonitor extends StateMachine { 
        final String testUrl = getTestUrl(TEST_CAPTIVE_PORTAL_HTTP_URL);

        if (isValidTestUrl(testUrl)) return testUrl;

        final Context targetContext = getCustomizedContextOrDefault();

        return getSettingFromResource(targetContext, R.string.config_captive_portal_http_url,

                R.string.default_captive_portal_http_url, CAPTIVE_PORTAL_HTTP_URL);

        return getSettingFromResource(targetContext,

                R.string.config_captive_portal_http_url, CAPTIVE_PORTAL_HTTP_URL,

                targetContext.getResources().getString(R.string.default_captive_portal_http_url));

    }



    private int getConsecutiveDnsTimeoutThreshold() {
@@ -1937,7 +1940,8 @@ public class NetworkMonitor extends StateMachine { 
                combineCaptivePortalUrls(firstUrl, CAPTIVE_PORTAL_OTHER_FALLBACK_URLS);

            return getProbeUrlArrayConfig(settingProviderUrls,

                    R.array.config_captive_portal_fallback_urls,

                    R.array.default_captive_portal_fallback_urls, this::makeURL);

                    R.array.default_captive_portal_fallback_urls,

                    this::makeURL);

        } catch (Exception e) {

            // Don't let a misconfiguration bootloop the system.

            Log.e(TAG, "Error parsing configured fallback URLs", e);
@@ -1957,7 +1961,7 @@ public class NetworkMonitor extends StateMachine { 


            return getProbeUrlArrayConfig(providerValue,

                    R.array.config_captive_portal_fallback_probe_specs,

                    R.array.default_captive_portal_fallback_probe_specs,

                    DEFAULT_CAPTIVE_PORTAL_FALLBACK_PROBE_SPECS,

                    CaptivePortalProbeSpec::parseSpecOrNull);

        } catch (Exception e) {

            // Don't let a misconfiguration bootloop the system.
@@ -1971,6 +1975,8 @@ public class NetworkMonitor extends StateMachine { 
        try {

            final URL[] settingProviderUrls =

                combineCaptivePortalUrls(firstUrl, CAPTIVE_PORTAL_OTHER_HTTPS_URLS);

            // firstUrl will at least be default configuration, so default value in

            // getProbeUrlArrayConfig is actually never used.

            return getProbeUrlArrayConfig(settingProviderUrls,

                    R.array.config_captive_portal_https_urls,

                    DEFAULT_CAPTIVE_PORTAL_HTTPS_URLS, this::makeURL);
@@ -1987,6 +1993,8 @@ public class NetworkMonitor extends StateMachine { 
        try {

            final URL[] settingProviderUrls =

                    combineCaptivePortalUrls(firstUrl, CAPTIVE_PORTAL_OTHER_HTTP_URLS);

            // firstUrl will at least be default configuration, so default value in

            // getProbeUrlArrayConfig is actually never used.

            return getProbeUrlArrayConfig(settingProviderUrls,

                    R.array.config_captive_portal_http_urls,

                    DEFAULT_CAPTIVE_PORTAL_HTTP_URLS, this::makeURL);
@@ -1998,11 +2006,11 @@ public class NetworkMonitor extends StateMachine { 
        }

    }



    private URL[] combineCaptivePortalUrls(final String firstUrl, final String name) {

    private URL[] combineCaptivePortalUrls(final String firstUrl, final String propertyName) {

        if (TextUtils.isEmpty(firstUrl)) return new URL[0];



        final String otherUrls = mDependencies.getDeviceConfigProperty(

                NAMESPACE_CONNECTIVITY, name, "");

                NAMESPACE_CONNECTIVITY, propertyName, "");

        // otherUrls may be empty, but .split() ignores trailing empty strings

        final String separator = ",";

        final String[] urls = (firstUrl + separator + otherUrls).split(separator);
@@ -2012,27 +2020,26 @@ public class NetworkMonitor extends StateMachine { 
    /**

     * Read a setting from a resource or the settings provider.

     *

     * <p>The configuration resource is prioritized, then the provider value, then the default

     * resource value.

     * <p>The configuration resource is prioritized, then the provider value.

     * @param context The context

     * @param configResource The resource id for the configuration parameter

     * @param defaultResource The resource id for the default value

     * @param symbol The symbol in the settings provider

     * @param defaultValue The default value

     * @return The best available value

     */

    @NonNull

    @Nullable

    private String getSettingFromResource(@NonNull final Context context,

            @StringRes int configResource, @StringRes int defaultResource,

            @NonNull String symbol) {

            @StringRes int configResource, @NonNull String symbol, @NonNull String defaultValue) {

        final Resources res = context.getResources();

        String setting = res.getString(configResource);



        if (!TextUtils.isEmpty(setting)) return setting;



        setting = mDependencies.getSetting(context, symbol, null);



        if (!TextUtils.isEmpty(setting)) return setting;



        return res.getString(defaultResource);

        return defaultValue;

    }



    /**

tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java

+27 −6
Original line number Diff line number Diff line
@@ -42,6 +42,7 @@ import static android.net.util.DataStallUtils.DATA_STALL_EVALUATION_TYPE_TCP; 
import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_FALLBACK_PROBE_SPECS;

import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS;

import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_USE_HTTPS;

import static android.net.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT;

import static android.net.util.NetworkStackUtils.DISMISS_PORTAL_IN_VALIDATED_NETWORK;

import static android.net.util.NetworkStackUtils.DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION;

import static android.net.util.NetworkStackUtils.TEST_CAPTIVE_PORTAL_HTTPS_URL;
@@ -801,6 +802,28 @@ public class NetworkMonitorTest { 
                R.integer.config_max_matches_http_content_length, 0));

    }



    @Test

    public void testGetHttpProbeUrl() {

        final WrappedNetworkMonitor wnm = makeNotMeteredNetworkMonitor();

        // If config_captive_portal_http_url is set and the global setting is set, the config is

        // used.

        doReturn(TEST_HTTP_URL).when(mResources).getString(R.string.config_captive_portal_http_url);

        doReturn(TEST_HTTP_OTHER_URL2).when(mResources).getString(

                R.string.default_captive_portal_http_url);

        when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_HTTP_URL), any()))

                .thenReturn(TEST_HTTP_OTHER_URL1);

        assertEquals(TEST_HTTP_URL, wnm.getCaptivePortalServerHttpUrl());

        // If config_captive_portal_http_url is unset and the global setting is set, the global

        // setting is used.

        doReturn(null).when(mResources).getString(R.string.config_captive_portal_http_url);

        assertEquals(TEST_HTTP_OTHER_URL1, wnm.getCaptivePortalServerHttpUrl());

        // If both config_captive_portal_http_url and global setting are unset,

        // default_captive_portal_http_url is used.

        when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_HTTP_URL), any()))

                .thenReturn(null);

        assertEquals(TEST_HTTP_OTHER_URL2, wnm.getCaptivePortalServerHttpUrl());

    }



    @Test

    public void testGetLocationMcc() throws Exception {

        final WrappedNetworkMonitor wnm = makeNotMeteredNetworkMonitor();
@@ -951,12 +974,10 @@ public class NetworkMonitorTest { 
        }).when(mDependencies).getDeviceConfigPropertyInt(any(),

                eq(NetworkMonitor.CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT),

                anyInt());

        when(mResources.getInteger(eq(R.integer.default_captive_portal_dns_probe_timeout)))

                .thenReturn(42);

        assertEquals(42, wnm.getIntSetting(mContext,

        assertEquals(DEFAULT_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT, wnm.getIntSetting(mContext,

                R.integer.config_captive_portal_dns_probe_timeout,

                NetworkMonitor.CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT,

                R.integer.default_captive_portal_dns_probe_timeout));

                DEFAULT_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT));



        // Set device config. Expect to get device config.

        when(mDependencies.getDeviceConfigPropertyInt(any(),
@@ -965,7 +986,7 @@ public class NetworkMonitorTest { 
        assertEquals(1234, wnm.getIntSetting(mContext,

                R.integer.config_captive_portal_dns_probe_timeout,

                NetworkMonitor.CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT,

                R.integer.default_captive_portal_dns_probe_timeout));

                DEFAULT_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT));



        // Set config resource. Expect to get config resource.

        when(mResources.getInteger(eq(R.integer.config_captive_portal_dns_probe_timeout)))
@@ -973,7 +994,7 @@ public class NetworkMonitorTest { 
        assertEquals(5678, wnm.getIntSetting(mContext,

                R.integer.config_captive_portal_dns_probe_timeout,

                NetworkMonitor.CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT,

                R.integer.default_captive_portal_dns_probe_timeout));

                DEFAULT_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT));

    }



    @Test