Snap for 7799923 from f90e90550b610f6e8fe5532a43618756e8c1d409 to mainline-os-statsd-release Change-Id: I9ffd22483e774ce1729155e2065e9b3552dc8a7f
diff --git a/TEST_MAPPING b/TEST_MAPPING index 2bb0e61..ed6a1a8 100644 --- a/TEST_MAPPING +++ b/TEST_MAPPING
@@ -40,8 +40,7 @@ // We must specify at least one module here or the tests won't run. Use the same set as CTS // so in theory the infra would not need to reinstall/reboot devices to run both. { - // TODO: add back tethering when it is updatable in this branch - "name": "NetworkStackTests[CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex]" + "name": "NetworkStackTests[CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex]" } ], "mainline-postsubmit": [
diff --git a/common/moduleutils/src/android/net/shared/NetworkMonitorUtils.java b/common/moduleutils/src/android/net/shared/NetworkMonitorUtils.java index 0cd9f65..b151cb9 100644 --- a/common/moduleutils/src/android/net/shared/NetworkMonitorUtils.java +++ b/common/moduleutils/src/android/net/shared/NetworkMonitorUtils.java
@@ -83,10 +83,6 @@ return true; } - // TODO: once TRANSPORT_TEST is @SystemApi in S and S SDK is stable (so constant shims can - // be replaced with the SDK constant that will be inlined), replace isTestNetwork with - // hasTransport(TRANSPORT_TEST) - // Test networks that also have one of the major transport types are attempting to replicate // that transport on a test interface (for example, test ethernet networks with // EthernetManager#setIncludeTestInterfaces). Run validation on them for realistic tests.
diff --git a/common/networkstackclient/Android.bp b/common/networkstackclient/Android.bp index 02a39f6..c344d07 100644 --- a/common/networkstackclient/Android.bp +++ b/common/networkstackclient/Android.bp
@@ -121,7 +121,7 @@ enabled: false, }, }, - imports: ["ipmemorystore-aidl-interfaces"], + imports: ["ipmemorystore-aidl-interfaces-V10"], versions: [ "1", "2",
diff --git a/res/values-da/strings.xml b/res/values-da/strings.xml index e54f11c..706f174 100644 --- a/res/values-da/strings.xml +++ b/res/values-da/strings.xml
@@ -19,7 +19,7 @@ <string name="notification_channel_name_connected" msgid="1795068343200033922">"Godkendelse til loginportal"</string> <string name="notification_channel_description_connected" msgid="7239184168268014518">"De notifikationer, der vises, når enheden er blevet godkendt til et netværk via en loginportal"</string> <string name="notification_channel_name_network_venue_info" msgid="6526543187249265733">"Oplysninger om netværksplacering"</string> - <string name="notification_channel_description_network_venue_info" msgid="5131499595382733605">"Notifikationer, der vises for at indikere, at netværket har en side med oplysninger om placeringen"</string> + <string name="notification_channel_description_network_venue_info" msgid="5131499595382733605">"Notifikationer, der vises for at indikere, at netværket har en side med oplysninger om lokationen"</string> <string name="connected" msgid="4563643884927480998">"Der er oprettet forbindelse"</string> <string name="tap_for_info" msgid="6849746325626883711">"Der er oprettet forbindelse/tryk for at se website"</string> <string name="application_label" msgid="1322847171305285454">"Netværksadministrator"</string>
diff --git a/src/android/net/ip/IpClient.java b/src/android/net/ip/IpClient.java index 4de9393..12ab3fd 100644 --- a/src/android/net/ip/IpClient.java +++ b/src/android/net/ip/IpClient.java
@@ -715,7 +715,7 @@ (ifaceUp) -> sendMessage(EVENT_NETLINK_LINKPROPERTIES_CHANGED, ifaceUp ? ARG_LINKPROP_CHANGED_LINKSTATE_UP : ARG_LINKPROP_CHANGED_LINKSTATE_DOWN), - config, mLog) { + config, mLog, mDependencies) { @Override public void onInterfaceAdded(String iface) { super.onInterfaceAdded(iface); @@ -906,6 +906,7 @@ private void stopStateMachineUpdaters() { mObserverRegistry.unregisterObserver(mLinkObserver); + mLinkObserver.clearInterfaceParams(); mLinkObserver.shutdown(); } @@ -1969,7 +1970,6 @@ mHasDisabledIpv6OrAcceptRaOnProvLoss = false; mGratuitousNaTargetAddresses.clear(); - mLinkObserver.clearInterfaceParams(); resetLinkProperties(); if (mStartTimeMillis > 0) { // Completed a life-cycle; send a final empty LinkProperties
diff --git a/src/android/net/ip/IpClientLinkObserver.java b/src/android/net/ip/IpClientLinkObserver.java index 3702674..ff0aafe 100644 --- a/src/android/net/ip/IpClientLinkObserver.java +++ b/src/android/net/ip/IpClientLinkObserver.java
@@ -16,6 +16,7 @@ package android.net.ip; +import static android.net.util.NetworkStackUtils.IPCLIENT_PARSE_NETLINK_EVENTS_VERSION; import static android.system.OsConstants.AF_INET6; import static com.android.net.module.util.NetworkStackConstants.ICMPV6_ROUTER_ADVERTISEMENT; @@ -37,6 +38,7 @@ import com.android.net.module.util.netlink.NetlinkConstants; import com.android.net.module.util.netlink.NetlinkMessage; import com.android.net.module.util.netlink.StructNdOptPref64; +import com.android.net.module.util.netlink.StructNdOptRdnss; import com.android.networkstack.apishim.NetworkInformationShimImpl; import com.android.networkstack.apishim.common.NetworkInformationShim; import com.android.server.NetworkObserver; @@ -107,6 +109,7 @@ } } + private final Context mContext; private final String mInterfaceName; private final Callback mCallback; private final LinkProperties mLinkProperties; @@ -115,13 +118,15 @@ private final AlarmManager mAlarmManager; private final Configuration mConfig; private final Handler mHandler; + private final IpClient.Dependencies mDependencies; private final MyNetlinkMonitor mNetlinkMonitor; private static final boolean DBG = false; public IpClientLinkObserver(Context context, Handler h, String iface, Callback callback, - Configuration config, SharedLog log) { + Configuration config, SharedLog log, IpClient.Dependencies deps) { + mContext = context; mInterfaceName = iface; mTag = "NetlinkTracker/" + mInterfaceName; mCallback = callback; @@ -134,6 +139,7 @@ mAlarmManager = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE); mNetlinkMonitor = new MyNetlinkMonitor(h, log, mTag); mHandler.post(mNetlinkMonitor::start); + mDependencies = deps; } public void shutdown() { @@ -153,6 +159,11 @@ } } + private boolean isNetlinkEventParsingEnabled() { + return mDependencies.isFeatureEnabled(mContext, IPCLIENT_PARSE_NETLINK_EVENTS_VERSION, + false /* default value */); + } + @Override public void onInterfaceRemoved(String iface) { maybeLog("interfaceRemoved", iface); @@ -246,17 +257,21 @@ @Override public void onInterfaceDnsServerInfo(String iface, long lifetime, String[] addresses) { - if (mInterfaceName.equals(iface)) { - maybeLog("interfaceDnsServerInfo", Arrays.toString(addresses)); - final boolean changed = mDnsServerRepository.addServers(lifetime, addresses); - final boolean linkState; - if (changed) { - synchronized (this) { - mDnsServerRepository.setDnsServersOn(mLinkProperties); - linkState = getInterfaceLinkStateLocked(); - } - mCallback.update(linkState); + if (isNetlinkEventParsingEnabled()) return; + if (!mInterfaceName.equals(iface)) return; + maybeLog("interfaceDnsServerInfo", Arrays.toString(addresses)); + updateInterfaceDnsServerInfo(lifetime, addresses); + } + + private void updateInterfaceDnsServerInfo(long lifetime, final String[] addresses) { + final boolean changed = mDnsServerRepository.addServers(lifetime, addresses); + final boolean linkState; + if (changed) { + synchronized (this) { + mDnsServerRepository.setDnsServersOn(mLinkProperties); + linkState = getInterfaceLinkStateLocked(); } + mCallback.update(linkState); } } @@ -408,6 +423,15 @@ updatePref64(opt.prefix, now, expiry); } + private void processRdnssOption(StructNdOptRdnss opt) { + if (!isNetlinkEventParsingEnabled()) return; + final String[] addresses = new String[opt.servers.length]; + for (int i = 0; i < opt.servers.length; i++) { + addresses[i] = opt.servers[i].getHostAddress(); + } + updateInterfaceDnsServerInfo(opt.header.lifetime, addresses); + } + private void processNduseroptMessage(NduseroptMessage msg, final long whenMs) { if (msg.family != AF_INET6 || msg.option == null || msg.ifindex != mIfindex) return; if (msg.icmp_type != (byte) ICMPV6_ROUTER_ADVERTISEMENT) return; @@ -417,8 +441,12 @@ processPref64Option((StructNdOptPref64) msg.option, whenMs); break; + case StructNdOptRdnss.TYPE: + processRdnssOption((StructNdOptRdnss) msg.option); + break; + default: - // TODO: implement RDNSS and DNSSL. + // TODO: implement DNSSL. break; } }
diff --git a/src/android/net/util/NetworkStackUtils.java b/src/android/net/util/NetworkStackUtils.java index e06cdca..6dc2a5b 100755 --- a/src/android/net/util/NetworkStackUtils.java +++ b/src/android/net/util/NetworkStackUtils.java
@@ -250,6 +250,13 @@ "ipclient_garp_na_roaming_version"; /** + * Experiment flag to enable parsing netlink events from kernel directly instead from netd aidl + * interface. + */ + public static final String IPCLIENT_PARSE_NETLINK_EVENTS_VERSION = + "ipclient_parse_netlink_events_version"; + + /** * Experiment flag to disable accept_ra parameter when IPv6 provisioning loss happens due to * the default route has gone. */
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java index 131b3b2..948ce8d 100755 --- a/src/com/android/server/connectivity/NetworkMonitor.java +++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -16,6 +16,7 @@ package com.android.server.connectivity; +import static android.content.Intent.ACTION_CONFIGURATION_CHANGED; import static android.net.CaptivePortal.APP_RETURN_DISMISSED; import static android.net.CaptivePortal.APP_RETURN_UNWANTED; import static android.net.CaptivePortal.APP_RETURN_WANTED_AS_IS; @@ -392,6 +393,11 @@ */ private static final int CMD_BANDWIDTH_CHECK_TIMEOUT = 24; + /** + * Message to self to notify resource configuration is changed. + */ + private static final int EVENT_RESOURCE_CONFIG_CHANGED = 25; + // Start mReevaluateDelayMs at this value and double. @VisibleForTesting static final int INITIAL_REEVALUATE_DELAY_MS = 1000; @@ -416,7 +422,6 @@ private String mPrivateDnsProviderHostname = ""; private final Context mContext; - private final Context mCustomizedContext; private final INetworkMonitorCallbacks mCallback; private final int mCallbackVersion; private final Network mCleartextDnsNetwork; @@ -431,13 +436,21 @@ private final TcpSocketTracker mTcpTracker; // Configuration values for captive portal detection probes. private final String mCaptivePortalUserAgent; - private final URL[] mCaptivePortalFallbackUrls; - @NonNull - private final URL[] mCaptivePortalHttpUrls; - @NonNull - private final URL[] mCaptivePortalHttpsUrls; + // Configuration values in setting providers for captive portal detection probes + private final String mCaptivePortalHttpsUrlFromSetting; + private final String mCaptivePortalHttpUrlFromSetting; @Nullable private final CaptivePortalProbeSpec[] mCaptivePortalFallbackSpecs; + + // The probing URLs may be updated after constructor if system notifies configuration changed. + // Thus, these probing URLs should only be accessed in the StateMachine thread. + @NonNull + private URL[] mCaptivePortalFallbackUrls; + @NonNull + private URL[] mCaptivePortalHttpUrls; + @NonNull + private URL[] mCaptivePortalHttpsUrls; + // Configuration values for network bandwidth check. @Nullable private final String mEvaluatingBandwidthUrl; @@ -511,7 +524,8 @@ private @EvaluationType int mDataStallTypeToCollect; private boolean mAcceptPartialConnectivity = false; private final EvaluationState mEvaluationState = new EvaluationState(); - + @NonNull + private final BroadcastReceiver mConfigurationReceiver; private final boolean mPrivateIpNoInternetEnabled; private final boolean mMetricsEnabled; @@ -575,7 +589,6 @@ mWifiManager = (WifiManager) context.getSystemService(Context.WIFI_SERVICE); mCm = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE); mNotifier = serviceManager.getNotifier(); - mCustomizedContext = getCustomizedContextOrDefault(); // CHECKSTYLE:OFF IndentationCheck addState(mDefaultState); @@ -590,16 +603,18 @@ setInitialState(mDefaultState); // CHECKSTYLE:ON IndentationCheck + mCaptivePortalHttpsUrlFromSetting = + mDependencies.getSetting(context, CAPTIVE_PORTAL_HTTPS_URL, null); + mCaptivePortalHttpUrlFromSetting = + mDependencies.getSetting(context, CAPTIVE_PORTAL_HTTP_URL, null); mIsCaptivePortalCheckEnabled = getIsCaptivePortalCheckEnabled(); mPrivateIpNoInternetEnabled = getIsPrivateIpNoInternetEnabled(); mMetricsEnabled = deps.isFeatureEnabled(context, NAMESPACE_CONNECTIVITY, NetworkStackUtils.VALIDATION_METRICS_VERSION, true /* defaultEnabled */); mUseHttps = getUseHttpsValidation(); mCaptivePortalUserAgent = getCaptivePortalUserAgent(); - mCaptivePortalHttpsUrls = makeCaptivePortalHttpsUrls(); - mCaptivePortalHttpUrls = makeCaptivePortalHttpUrls(); - mCaptivePortalFallbackUrls = makeCaptivePortalFallbackUrls(); - mCaptivePortalFallbackSpecs = makeCaptivePortalFallbackProbeSpecs(); + mCaptivePortalFallbackSpecs = + makeCaptivePortalFallbackProbeSpecs(getCustomizedContextOrDefault()); mRandom = deps.getRandom(); // TODO: Evaluate to move data stall configuration to a specific class. mConsecutiveDnsTimeoutThreshold = getConsecutiveDnsTimeoutThreshold(); @@ -618,7 +633,14 @@ mEvaluatingBandwidthTimeoutMs = getResIntConfig(mContext, R.integer.config_evaluating_bandwidth_timeout_ms, DEFAULT_EVALUATING_BANDWIDTH_TIMEOUT_MS); - + mConfigurationReceiver = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + if (ACTION_CONFIGURATION_CHANGED.equals(intent.getAction())) { + sendMessage(EVENT_RESOURCE_CONFIG_CHANGED); + } + } + }; // Provide empty LinkProperties and NetworkCapabilities to make sure they are never null, // even before notifyNetworkConnected. mLinkProperties = new LinkProperties(); @@ -869,6 +891,18 @@ // does not entail any real state (hence no enter() or exit() routines). private class DefaultState extends State { @Override + public void enter() { + // Register configuration broadcast here instead of constructor to prevent start() was + // not called yet when the broadcast is received and cause crash. + mContext.registerReceiver(mConfigurationReceiver, + new IntentFilter(ACTION_CONFIGURATION_CHANGED)); + checkAndRenewResourceConfig(); + Log.d(TAG, "Starting on network " + mNetwork + + " with capport HTTPS URL " + Arrays.toString(mCaptivePortalHttpsUrls) + + " and HTTP URL " + Arrays.toString(mCaptivePortalHttpUrls)); + } + + @Override public boolean processMessage(Message message) { switch (message.what) { case CMD_NETWORK_CONNECTED: @@ -1014,11 +1048,27 @@ mNetworkCapabilities = (NetworkCapabilities) message.obj; suppressNotificationIfNetworkRestricted(); break; + case EVENT_RESOURCE_CONFIG_CHANGED: + // RRO generation does not happen during package installation and instead after + // the OMS receives the PACKAGE_ADDED event, there is a delay where the old + // idmap is used with the new target package resulting in the incorrect overlay + // is used. Renew the resource if a configuration change is received. + // TODO: Remove it once design to generate the idmaps during package + // installation in overlay manager and package manager is ready. + if (checkAndRenewResourceConfig()) { + sendMessage(CMD_FORCE_REEVALUATION, NO_UID, 1 /* forceAccept */); + } + break; default: break; } return HANDLED; } + + @Override + public void exit() { + mContext.unregisterReceiver(mConfigurationReceiver); + } } // Being in the ValidatedState State indicates a Network is: @@ -1339,7 +1389,9 @@ return HANDLED; case CMD_FORCE_REEVALUATION: // The evaluation process restarts via EvaluatingState#enter. - return shouldAcceptForceRevalidation() ? NOT_HANDLED : HANDLED; + final boolean forceAccept = (message.arg2 != 0); + return forceAccept || shouldAcceptForceRevalidation() + ? NOT_HANDLED : HANDLED; // Disable HTTPS probe and transition to EvaluatingPrivateDnsState because: // 1. Network is connected and finish the network validation. // 2. NetworkMonitor detects network is partial connectivity and user accepts it. @@ -1591,8 +1643,13 @@ final int token = ++mProbeToken; final ValidationProperties deps = new ValidationProperties(mNetworkCapabilities); + final URL fallbackUrl = nextFallbackUrl(); + final URL[] httpsUrls = Arrays.copyOf( + mCaptivePortalHttpsUrls, mCaptivePortalHttpsUrls.length); + final URL[] httpUrls = Arrays.copyOf( + mCaptivePortalHttpUrls, mCaptivePortalHttpUrls.length); mThread = new Thread(() -> sendMessage(obtainMessage(CMD_PROBE_COMPLETE, token, 0, - isCaptivePortal(deps)))); + isCaptivePortal(deps, httpsUrls, httpUrls, fallbackUrl)))); mThread.start(); } @@ -1982,18 +2039,24 @@ } final long now = System.currentTimeMillis(); - if (expTime < now || (expTime - now) > TEST_URL_EXPIRATION_MS) return null; + if (expTime < now || (expTime - now) > TEST_URL_EXPIRATION_MS) { + logw("Skipping test URL with expiration " + expTime + ", now " + now); + return null; + } final String strUrl = mDependencies.getDeviceConfigProperty(NAMESPACE_CONNECTIVITY, key, null /* defaultValue */); - if (!isValidTestUrl(strUrl)) return null; + if (!isValidTestUrl(strUrl)) { + logw("Skipping invalid test URL " + strUrl); + return null; + } return makeURL(strUrl); } - private String getCaptivePortalServerHttpsUrl() { - return getSettingFromResource(mCustomizedContext, - R.string.config_captive_portal_https_url, CAPTIVE_PORTAL_HTTPS_URL, - mCustomizedContext.getResources().getString( + private String getCaptivePortalServerHttpsUrl(@NonNull Context context) { + return getSettingFromResource(context, + R.string.config_captive_portal_https_url, mCaptivePortalHttpsUrlFromSetting, + context.getResources().getString( R.string.default_captive_portal_https_url)); } @@ -2070,10 +2133,10 @@ * it has its own updatable strategies to detect captive portals. The framework only advises * on one URL that can be used, while NetworkMonitor may implement more complex logic. */ - public String getCaptivePortalServerHttpUrl() { - return getSettingFromResource(mCustomizedContext, - R.string.config_captive_portal_http_url, CAPTIVE_PORTAL_HTTP_URL, - mCustomizedContext.getResources().getString( + public String getCaptivePortalServerHttpUrl(@NonNull Context context) { + return getSettingFromResource(context, + R.string.config_captive_portal_http_url, mCaptivePortalHttpUrlFromSetting, + context.getResources().getString( R.string.default_captive_portal_http_url)); } @@ -2109,13 +2172,13 @@ } @VisibleForTesting - URL[] makeCaptivePortalFallbackUrls() { + URL[] makeCaptivePortalFallbackUrls(@NonNull Context context) { try { final String firstUrl = mDependencies.getSetting(mContext, CAPTIVE_PORTAL_FALLBACK_URL, null); final URL[] settingProviderUrls = combineCaptivePortalUrls(firstUrl, CAPTIVE_PORTAL_OTHER_FALLBACK_URLS); - return getProbeUrlArrayConfig(settingProviderUrls, + return getProbeUrlArrayConfig(context, settingProviderUrls, R.array.config_captive_portal_fallback_urls, R.array.default_captive_portal_fallback_urls, this::makeURL); @@ -2126,7 +2189,7 @@ } } - private CaptivePortalProbeSpec[] makeCaptivePortalFallbackProbeSpecs() { + private CaptivePortalProbeSpec[] makeCaptivePortalFallbackProbeSpecs(@NonNull Context context) { try { final String settingsValue = mDependencies.getDeviceConfigProperty( NAMESPACE_CONNECTIVITY, CAPTIVE_PORTAL_FALLBACK_PROBE_SPECS, null); @@ -2136,7 +2199,7 @@ ? emptySpecs : parseCaptivePortalProbeSpecs(settingsValue).toArray(emptySpecs); - return getProbeUrlArrayConfig(providerValue, + return getProbeUrlArrayConfig(context, providerValue, R.array.config_captive_portal_fallback_probe_specs, DEFAULT_CAPTIVE_PORTAL_FALLBACK_PROBE_SPECS, CaptivePortalProbeSpec::parseSpecOrNull); @@ -2147,17 +2210,17 @@ } } - private URL[] makeCaptivePortalHttpsUrls() { + private URL[] makeCaptivePortalHttpsUrls(@NonNull Context context) { final URL testUrl = getTestUrl(TEST_CAPTIVE_PORTAL_HTTPS_URL); if (testUrl != null) return new URL[] { testUrl }; - final String firstUrl = getCaptivePortalServerHttpsUrl(); + final String firstUrl = getCaptivePortalServerHttpsUrl(context); 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, + return getProbeUrlArrayConfig(context, settingProviderUrls, R.array.config_captive_portal_https_urls, DEFAULT_CAPTIVE_PORTAL_HTTPS_URLS, this::makeURL); } catch (Exception e) { @@ -2168,17 +2231,17 @@ } } - private URL[] makeCaptivePortalHttpUrls() { + private URL[] makeCaptivePortalHttpUrls(@NonNull Context context) { final URL testUrl = getTestUrl(TEST_CAPTIVE_PORTAL_HTTP_URL); if (testUrl != null) return new URL[] { testUrl }; - final String firstUrl = getCaptivePortalServerHttpUrl(); + final String firstUrl = getCaptivePortalServerHttpUrl(context); 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, + return getProbeUrlArrayConfig(context, settingProviderUrls, R.array.config_captive_portal_http_urls, DEFAULT_CAPTIVE_PORTAL_HTTP_URLS, this::makeURL); } catch (Exception e) { @@ -2206,21 +2269,20 @@ * <p>The configuration resource is prioritized, then the provider value. * @param context The context * @param configResource The resource id for the configuration parameter - * @param symbol The symbol in the settings provider + * @param settingValue The value in the settings provider * @param defaultValue The default value * @return The best available value */ @Nullable private String getSettingFromResource(@NonNull final Context context, - @StringRes int configResource, @NonNull String symbol, @NonNull String defaultValue) { + @StringRes int configResource, @NonNull String settingValue, + @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; + if (!TextUtils.isEmpty(settingValue)) return settingValue; return defaultValue; } @@ -2230,17 +2292,20 @@ * * <p>The configuration resource is prioritized, then the provider values, then the default * resource values. + * + * @param context The Context * @param providerValue Values obtained from the setting provider. * @param configResId ID of the configuration resource. * @param defaultResId ID of the default resource. * @param resourceConverter Converter from the resource strings to stored setting class. Null * return values are ignored. */ - private <T> T[] getProbeUrlArrayConfig(@NonNull T[] providerValue, @ArrayRes int configResId, - @ArrayRes int defaultResId, @NonNull Function<String, T> resourceConverter) { - final Resources res = mCustomizedContext.getResources(); - return getProbeUrlArrayConfig(providerValue, configResId, res.getStringArray(defaultResId), - resourceConverter); + private <T> T[] getProbeUrlArrayConfig(@NonNull Context context, @NonNull T[] providerValue, + @ArrayRes int configResId, @ArrayRes int defaultResId, + @NonNull Function<String, T> resourceConverter) { + final Resources res = context.getResources(); + return getProbeUrlArrayConfig(context, providerValue, configResId, + res.getStringArray(defaultResId), resourceConverter); } /** @@ -2248,15 +2313,18 @@ * * <p>The configuration resource is prioritized, then the provider values, then the default * resource values. + * + * @param context The Context * @param providerValue Values obtained from the setting provider. * @param configResId ID of the configuration resource. * @param defaultConfig Values of default configuration. * @param resourceConverter Converter from the resource strings to stored setting class. Null * return values are ignored. */ - private <T> T[] getProbeUrlArrayConfig(@NonNull T[] providerValue, @ArrayRes int configResId, - String[] defaultConfig, @NonNull Function<String, T> resourceConverter) { - final Resources res = mCustomizedContext.getResources(); + private <T> T[] getProbeUrlArrayConfig(@NonNull Context context, @NonNull T[] providerValue, + @ArrayRes int configResId, String[] defaultConfig, + @NonNull Function<String, T> resourceConverter) { + final Resources res = context.getResources(); String[] configValue = res.getStringArray(configResId); if (configValue.length == 0) { @@ -2335,15 +2403,14 @@ } } - private CaptivePortalProbeResult isCaptivePortal(ValidationProperties properties) { + private CaptivePortalProbeResult isCaptivePortal(ValidationProperties properties, + URL[] httpsUrls, URL[] httpUrls, URL fallbackUrl) { if (!mIsCaptivePortalCheckEnabled) { validationLog("Validation disabled."); return CaptivePortalProbeResult.success(CaptivePortalProbeResult.PROBE_UNKNOWN); } URL pacUrl = null; - final URL[] httpsUrls = mCaptivePortalHttpsUrls; - final URL[] httpUrls = mCaptivePortalHttpUrls; // 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: @@ -2384,7 +2451,7 @@ } else if (mUseHttps && httpsUrls.length == 1 && httpUrls.length == 1) { // Probe results are reported inside sendHttpAndHttpsParallelWithFallbackProbes. result = sendHttpAndHttpsParallelWithFallbackProbes(properties, proxyInfo, - httpsUrls[0], httpUrls[0]); + httpsUrls[0], httpUrls[0], fallbackUrl); } else if (mUseHttps) { // Support result aggregation from multiple Urls. result = sendMultiParallelHttpAndHttpsProbes(properties, proxyInfo, httpsUrls, @@ -2986,7 +3053,8 @@ } private CaptivePortalProbeResult sendHttpAndHttpsParallelWithFallbackProbes( - ValidationProperties properties, ProxyInfo proxy, URL httpsUrl, URL httpUrl) { + ValidationProperties properties, 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); @@ -3032,10 +3100,10 @@ // If a fallback method exists, use it to retry portal detection. // If we have new-style probe specs, use those. Otherwise, use the fallback URLs. final CaptivePortalProbeSpec probeSpec = nextFallbackSpec(); - final URL fallbackUrl = (probeSpec != null) ? probeSpec.getUrl() : nextFallbackUrl(); + final URL fallback = (probeSpec != null) ? probeSpec.getUrl() : fallbackUrl; CaptivePortalProbeResult fallbackProbeResult = null; - if (fallbackUrl != null) { - fallbackProbeResult = sendHttpProbe(fallbackUrl, PROBE_FALLBACK, probeSpec); + if (fallback != null) { + fallbackProbeResult = sendHttpProbe(fallback, PROBE_FALLBACK, probeSpec); reportHttpProbeResult(NETWORK_VALIDATION_PROBE_FALLBACK, fallbackProbeResult); if (fallbackProbeResult.isPortal()) { return fallbackProbeResult; @@ -3606,4 +3674,37 @@ && captivePortalDataShim.getUserPortalUrlSource() == ConstantsShim.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT; } + + private boolean checkAndRenewResourceConfig() { + boolean reevaluationNeeded = false; + + final Context customizedContext = getCustomizedContextOrDefault(); + final URL[] captivePortalHttpsUrls = makeCaptivePortalHttpsUrls(customizedContext); + if (!Arrays.equals(mCaptivePortalHttpsUrls, captivePortalHttpsUrls)) { + mCaptivePortalHttpsUrls = captivePortalHttpsUrls; + reevaluationNeeded = true; + log("checkAndRenewResourceConfig: update captive portal https urls to " + + Arrays.toString(mCaptivePortalHttpsUrls)); + } + + final URL[] captivePortalHttpUrls = makeCaptivePortalHttpUrls(customizedContext); + if (!Arrays.equals(mCaptivePortalHttpUrls, captivePortalHttpUrls)) { + mCaptivePortalHttpUrls = captivePortalHttpUrls; + reevaluationNeeded = true; + log("checkAndRenewResourceConfig: update captive portal http urls to " + + Arrays.toString(mCaptivePortalHttpUrls)); + } + + final URL[] captivePortalFallbackUrls = makeCaptivePortalFallbackUrls(customizedContext); + if (!Arrays.equals(mCaptivePortalFallbackUrls, captivePortalFallbackUrls)) { + mCaptivePortalFallbackUrls = captivePortalFallbackUrls; + // Reset the index since the array is changed. + mNextFallbackUrlIndex = 0; + reevaluationNeeded = true; + log("checkAndRenewResourceConfig: update captive portal fallback urls to" + + Arrays.toString(mCaptivePortalFallbackUrls)); + } + + return reevaluationNeeded; + } }
diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java index bbbe0ba..6ade54f 100644 --- a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java +++ b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
@@ -146,7 +146,6 @@ import androidx.annotation.NonNull; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; -import androidx.test.runner.AndroidJUnit4; import com.android.internal.util.HexDump; import com.android.internal.util.StateMachine; @@ -181,6 +180,7 @@ import org.junit.Test; import org.junit.rules.TestName; import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.mockito.Mock; @@ -224,7 +224,7 @@ * * Tests in this class can either be run with signature permissions, or with root access. */ -@RunWith(AndroidJUnit4.class) +@RunWith(Parameterized.class) @SmallTest public abstract class IpClientIntegrationTestCommon { private static final int DATA_BUFFER_LEN = 4096; @@ -247,6 +247,17 @@ @Rule public final TestName mTestNameRule = new TestName(); + // Indicate whether the flag of parsing netlink event is enabled or not. If it's disabled, + // integration test still covers the old codepath(i.e. using NetworkObserver), otherwise, + // test goes through the new codepath(i.e. processRtNetlinkxxx). + @Parameterized.Parameter(0) + public boolean mIsNetlinkEventParseEnabled; + + @Parameterized.Parameters + public static Iterable<? extends Object> data() { + return Arrays.asList(Boolean.valueOf("false"), Boolean.valueOf("true")); + } + /** * Indicates that a test requires signature permissions to run. * @@ -553,8 +564,14 @@ @Before public void setUp() throws Exception { - final Method testMethod = IpClientIntegrationTestCommon.class.getMethod( - mTestNameRule.getMethodName()); + // Suffix "[0]" or "[1]" is added to the end of test method name after running with + // Parameterized.class, that's intended behavior, to iterate each test method with the + // parameterize value. However, Class#getMethod() throws NoSuchMethodException when + // searching the target test method name due to this change. Just keep the original test + // method name to fix NoSuchMethodException, and find the correct annotation associated + // to test method. + final String testMethodName = mTestNameRule.getMethodName().split("\\[")[0]; + final Method testMethod = IpClientIntegrationTestCommon.class.getMethod(testMethodName); mIsSignatureRequiredTest = testMethod.getAnnotation(SignatureRequiredTest.class) != null; assumeFalse(testSkipped()); @@ -567,6 +584,12 @@ } mIIpClient = makeIIpClient(mIfaceName, mCb); + + // Depend on the parameterized value to enable/disable netlink message refactor flag. + // Make sure both of the old codepath(rely on the INetdUnsolicitedEventListener aidl) + // and new codepath(parse netlink event from kernel) will be executed. + setFeatureEnabled(NetworkStackUtils.IPCLIENT_PARSE_NETLINK_EVENTS_VERSION, + mIsNetlinkEventParseEnabled /* default value */); } protected void setUpMocks() throws Exception {
diff --git a/tests/unit/Android.bp b/tests/unit/Android.bp index a74d018..fa054fe 100644 --- a/tests/unit/Android.bp +++ b/tests/unit/Android.bp
@@ -56,10 +56,7 @@ min_sdk_version: "29", srcs: [], // TODO: tests that only apply to the current, non-stable API can be added here test_suites: ["general-tests"], - test_mainline_modules: [ - "CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex", - "CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex" - ], + test_mainline_modules: ["CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex"], defaults: ["NetworkStackTestsDefaults"], static_libs: ["NetworkStackApiCurrentLib"], compile_multilib: "both", // Workaround for b/147785146 for mainline-presubmit @@ -86,10 +83,7 @@ min_sdk_version: "29", target_sdk_version: "30", test_suites: ["general-tests", "mts"], - test_mainline_modules: [ - "CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex", - "CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex" - ], + test_mainline_modules: ["CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex"], defaults: ["NetworkStackTestsDefaults"], static_libs: ["NetworkStackApiStableLib"], compile_multilib: "both",
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java index 9d392eb..ff43325 100644 --- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -16,6 +16,7 @@ package com.android.server.connectivity; +import static android.content.Intent.ACTION_CONFIGURATION_CHANGED; import static android.net.CaptivePortal.APP_RETURN_DISMISSED; import static android.net.CaptivePortal.APP_RETURN_WANTED_AS_IS; import static android.net.DnsResolver.TYPE_A; @@ -637,6 +638,7 @@ @Override protected void onQuitting() { + super.onQuitting(); assertTrue(mCreatedNetworkMonitors.remove(this)); mQuitCv.open(); } @@ -863,7 +865,6 @@ @Test public void testGetHttpProbeUrl() { - final WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor(); // 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); @@ -871,16 +872,21 @@ 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()); + final WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor(); + assertEquals(TEST_HTTP_URL, wnm.getCaptivePortalServerHttpUrl(mContext)); // 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()); + assertEquals(TEST_HTTP_OTHER_URL1, wnm.getCaptivePortalServerHttpUrl(mContext)); // If both config_captive_portal_http_url and global setting are unset, - // default_captive_portal_http_url is used. + // default_captive_portal_http_url is used. But the global setting will only be read in the + // constructor. when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_HTTP_URL), any())) .thenReturn(null); - assertEquals(TEST_HTTP_OTHER_URL2, wnm.getCaptivePortalServerHttpUrl()); + assertEquals(TEST_HTTP_OTHER_URL1, wnm.getCaptivePortalServerHttpUrl(mContext)); + // default_captive_portal_http_url is used when the configuration is applied in new NM. + final WrappedNetworkMonitor wnm2 = makeCellNotMeteredNetworkMonitor(); + assertEquals(TEST_HTTP_OTHER_URL2, wnm2.getCaptivePortalServerHttpUrl(mContext)); } @Test @@ -937,6 +943,57 @@ } } + @Test + public void testConfigurationChange_BeforeNMConnected() throws Exception { + final WrappedNetworkMonitor nm = new WrappedNetworkMonitor(); + final ArgumentCaptor<BroadcastReceiver> receiverCaptor = + ArgumentCaptor.forClass(BroadcastReceiver.class); + + // Verify configuration change receiver is registered after start(). + verify(mContext, never()).registerReceiver(receiverCaptor.capture(), + argThat(receiver -> ACTION_CONFIGURATION_CHANGED.equals(receiver.getAction(0)))); + nm.start(); + mCreatedNetworkMonitors.add(nm); + HandlerUtils.waitForIdle(nm.getHandler(), HANDLER_TIMEOUT_MS); + verify(mContext, times(1)).registerReceiver(receiverCaptor.capture(), + argThat(receiver -> ACTION_CONFIGURATION_CHANGED.equals(receiver.getAction(0)))); + // Update a new URL and send a configuration change + doReturn(TEST_HTTPS_OTHER_URL1).when(mResources).getString( + R.string.config_captive_portal_https_url); + receiverCaptor.getValue().onReceive(mContext, new Intent(ACTION_CONFIGURATION_CHANGED)); + HandlerUtils.waitForIdle(nm.getHandler(), HANDLER_TIMEOUT_MS); + // Should stay in default state before receiving CMD_NETWORK_CONNECTED + verify(mOtherHttpsConnection1, never()).getResponseCode(); + } + + @Test + public void testIsCaptivePortal_ConfigurationChange_RenewUrls() throws Exception { + setStatus(mHttpsConnection, 204); + final NetworkMonitor nm = runValidatedNetworkTest(); + final ArgumentCaptor<BroadcastReceiver> receiverCaptor = + ArgumentCaptor.forClass(BroadcastReceiver.class); + verify(mContext, times(1)).registerReceiver(receiverCaptor.capture(), + argThat(receiver -> ACTION_CONFIGURATION_CHANGED.equals(receiver.getAction(0)))); + + resetCallbacks(); + // New URLs with partial connectivity + doReturn(TEST_HTTPS_OTHER_URL1).when(mResources).getString( + R.string.config_captive_portal_https_url); + doReturn(TEST_HTTP_OTHER_URL1).when(mResources).getString( + R.string.config_captive_portal_http_url); + setStatus(mOtherHttpsConnection1, 500); + setStatus(mOtherHttpConnection1, 204); + + // Receive configuration. Expect a reevaluation triggered. + receiverCaptor.getValue().onReceive(mContext, new Intent(ACTION_CONFIGURATION_CHANGED)); + + HandlerUtils.waitForIdle(nm.getHandler(), HANDLER_TIMEOUT_MS); + verifyNetworkTested(NETWORK_VALIDATION_RESULT_PARTIAL, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP); + verify(mOtherHttpsConnection1, times(1)).getResponseCode(); + verify(mOtherHttpConnection1, times(1)).getResponseCode(); + } + private CellInfoGsm makeTestCellInfoGsm(String mcc) throws Exception { final CellInfoGsm info = new CellInfoGsm(); final CellIdentityGsm ci = makeCellIdentityGsm(0, 0, 0, 0, mcc, "01", "", ""); @@ -967,7 +1024,7 @@ public void testMakeFallbackUrls() throws Exception { final WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor(); // Value exist in setting provider. - URL[] urls = wnm.makeCaptivePortalFallbackUrls(); + URL[] urls = wnm.makeCaptivePortalFallbackUrls(mContext); assertEquals(urls[0].toString(), TEST_FALLBACK_URL); // Clear setting provider value. Verify it to get configuration from resource instead. @@ -975,13 +1032,13 @@ // Verify that getting resource with exception. when(mResources.getStringArray(R.array.config_captive_portal_fallback_urls)) .thenThrow(Resources.NotFoundException.class); - urls = wnm.makeCaptivePortalFallbackUrls(); + urls = wnm.makeCaptivePortalFallbackUrls(mContext); assertEquals(urls.length, 0); // Verify resource return 2 different URLs. doReturn(new String[] {"http://testUrl1.com", "http://testUrl2.com"}).when(mResources) .getStringArray(R.array.config_captive_portal_fallback_urls); - urls = wnm.makeCaptivePortalFallbackUrls(); + urls = wnm.makeCaptivePortalFallbackUrls(mContext); assertEquals(urls.length, 2); assertEquals("http://testUrl1.com", urls[0].toString()); assertEquals("http://testUrl2.com", urls[1].toString()); @@ -992,7 +1049,7 @@ setupNoSimCardNeighborMcc(); doReturn(new String[] {"http://testUrl3.com"}).when(mMccResource) .getStringArray(R.array.config_captive_portal_fallback_urls); - urls = wnm.makeCaptivePortalFallbackUrls(); + urls = wnm.makeCaptivePortalFallbackUrls(mContext); assertEquals(urls.length, 2); assertEquals("http://testUrl1.com", urls[0].toString()); assertEquals("http://testUrl2.com", urls[1].toString()); @@ -1005,7 +1062,7 @@ doReturn(new String[] {"http://testUrl.com"}).when(mMccResource) .getStringArray(R.array.config_captive_portal_fallback_urls); final WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor(); - final URL[] urls = wnm.makeCaptivePortalFallbackUrls(); + final URL[] urls = wnm.makeCaptivePortalFallbackUrls(mMccContext); assertEquals(urls.length, 1); assertEquals("http://testUrl.com", urls[0].toString()); } @@ -1894,7 +1951,7 @@ verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)) .showProvisioningNotification(any(), any()); - assertEquals(1, mRegisteredReceivers.size()); + assertCaptivePortalAppReceiverRegistered(true /* isPortal */); // Check that startCaptivePortalApp sends the expected intent. nm.launchCaptivePortalApp(); @@ -1932,7 +1989,7 @@ .notifyNetworkTestedWithExtras(matchNetworkTestResultParcelable( NETWORK_VALIDATION_RESULT_VALID, NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP)); - assertEquals(0, mRegisteredReceivers.size()); + assertCaptivePortalAppReceiverRegistered(false /* isPortal */); } @Test @@ -2518,7 +2575,7 @@ verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)) .showProvisioningNotification(any(), any()); - assertEquals(1, mRegisteredReceivers.size()); + assertCaptivePortalAppReceiverRegistered(true /* isPortal */); // Check that startCaptivePortalApp sends the expected intent. nm.launchCaptivePortalApp(); @@ -2740,7 +2797,7 @@ monitor.notifyNetworkConnected(linkProperties, networkCapabilities); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)) .showProvisioningNotification(any(), any()); - assertEquals(1, mRegisteredReceivers.size()); + assertCaptivePortalAppReceiverRegistered(true /* isPortal */); verifyNetworkTested(VALIDATION_RESULT_PORTAL, 0 /* probesSucceeded */, TEST_LOGIN_URL); // Force reevaluation and confirm that the network is still captive @@ -2903,21 +2960,21 @@ private NetworkMonitor runPortalNetworkTest() throws RemoteException { final NetworkMonitor nm = runNetworkTest(VALIDATION_RESULT_PORTAL, 0 /* probesSucceeded */, TEST_LOGIN_URL); - assertEquals(1, mRegisteredReceivers.size()); + assertCaptivePortalAppReceiverRegistered(true /* isPortal */); return nm; } private NetworkMonitor runNoValidationNetworkTest() throws RemoteException { final NetworkMonitor nm = runNetworkTest(NETWORK_VALIDATION_RESULT_VALID, 0 /* probesSucceeded */, null /* redirectUrl */); - assertEquals(0, mRegisteredReceivers.size()); + assertCaptivePortalAppReceiverRegistered(false /* isPortal */); return nm; } private NetworkMonitor runFailedNetworkTest() throws RemoteException { final NetworkMonitor nm = runNetworkTest( VALIDATION_RESULT_INVALID, 0 /* probesSucceeded */, null /* redirectUrl */); - assertEquals(0, mRegisteredReceivers.size()); + assertCaptivePortalAppReceiverRegistered(false /* isPortal */); return nm; } @@ -2925,7 +2982,7 @@ throws RemoteException { final NetworkMonitor nm = runNetworkTest(NETWORK_VALIDATION_RESULT_PARTIAL, probesSucceeded, null /* redirectUrl */); - assertEquals(0, mRegisteredReceivers.size()); + assertCaptivePortalAppReceiverRegistered(false /* isPortal */); return nm; } @@ -3057,5 +3114,13 @@ private DataStallReportParcelable matchTcpDataStallParcelable() { return argThat(p -> (p.detectionMethod & ConstantsShim.DETECTION_METHOD_TCP_METRICS) != 0); } + + private void assertCaptivePortalAppReceiverRegistered(boolean isPortal) { + // There will be configuration change receiver registered after NetworkMonitor being + // started. If captive portal app receiver is registered, then the size of the registered + // receivers will be 2. Otherwise, mRegisteredReceivers should only contain 1 configuration + // change receiver. + assertEquals(isPortal ? 2 : 1, mRegisteredReceivers.size()); + } }