diff options
6 files changed, 249 insertions, 93 deletions
diff --git a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java index ff743c8..2ba1dcc 100755 --- a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java +++ b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java @@ -16,14 +16,20 @@ package android.net.captiveportal; +import static android.net.metrics.ValidationProbeEvent.PROBE_HTTP; +import static android.net.metrics.ValidationProbeEvent.PROBE_HTTPS; + +import androidx.annotation.IntDef; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; /** * Result of calling isCaptivePortal(). * @hide */ -public final class CaptivePortalProbeResult { +public class CaptivePortalProbeResult { public static final int SUCCESS_CODE = 204; public static final int FAILED_CODE = 599; public static final int PORTAL_CODE = 302; @@ -42,18 +48,17 @@ public final class CaptivePortalProbeResult { // be FAILED if one of the probes returns this status. // This logic is only used if the config_force_dns_probe_private_ip_no_internet flag is set. public static final int DNS_PRIVATE_IP_RESPONSE_CODE = -2; - - @NonNull - public static final CaptivePortalProbeResult FAILED = new CaptivePortalProbeResult(FAILED_CODE); - @NonNull - public static final CaptivePortalProbeResult SUCCESS = - new CaptivePortalProbeResult(SUCCESS_CODE); - public static final CaptivePortalProbeResult PARTIAL = - new CaptivePortalProbeResult(PARTIAL_CODE); + // The private IP logic only applies to the HTTP probe. public static final CaptivePortalProbeResult PRIVATE_IP = - new CaptivePortalProbeResult(DNS_PRIVATE_IP_RESPONSE_CODE); + new CaptivePortalProbeResult(DNS_PRIVATE_IP_RESPONSE_CODE, 1 << PROBE_HTTP); + // Partial connectivity should be concluded from both HTTP and HTTPS probes. + @NonNull + public static final CaptivePortalProbeResult PARTIAL = new CaptivePortalProbeResult( + PARTIAL_CODE, 1 << PROBE_HTTP | 1 << PROBE_HTTPS); + // Probe type that is used for unspecified probe result. + public static final int PROBE_UNKNOWN = 0; - private final int mHttpResponseCode; // HTTP response code returned from Internet probe. + final int mHttpResponseCode; // HTTP response code returned from Internet probe. @Nullable public final String redirectUrl; // Redirect destination returned from Internet probe. @Nullable @@ -62,21 +67,39 @@ public final class CaptivePortalProbeResult { @Nullable public final CaptivePortalProbeSpec probeSpec; - public CaptivePortalProbeResult(int httpResponseCode) { - this(httpResponseCode, null, null); + // Indicate what kind of probe this is. This is a bitmask constructed by + // bitwise-or-ing(i.e. {@code |}) the {@code ValidationProbeEvent.PROBE_HTTP} or + // {@code ValidationProbeEvent.PROBE_HTTPS} values. Set it as {@code PROBE_UNKNOWN} if the probe + // type is unspecified. + @ProbeType + public final int probeType; + + @IntDef(value = { + PROBE_UNKNOWN, + 1 << PROBE_HTTP, + 1 << PROBE_HTTPS, + }) + @Retention(RetentionPolicy.SOURCE) + public @interface ProbeType { + } + + public CaptivePortalProbeResult(int httpResponseCode, @ProbeType int probeType) { + this(httpResponseCode, null, null, null, probeType); } public CaptivePortalProbeResult(int httpResponseCode, @Nullable String redirectUrl, - @Nullable String detectUrl) { - this(httpResponseCode, redirectUrl, detectUrl, null); + @Nullable String detectUrl, @ProbeType int probeType) { + this(httpResponseCode, redirectUrl, detectUrl, null, probeType); } public CaptivePortalProbeResult(int httpResponseCode, @Nullable String redirectUrl, - @Nullable String detectUrl, @Nullable CaptivePortalProbeSpec probeSpec) { + @Nullable String detectUrl, @Nullable CaptivePortalProbeSpec probeSpec, + @ProbeType int probeType) { mHttpResponseCode = httpResponseCode; this.redirectUrl = redirectUrl; this.detectUrl = detectUrl; this.probeSpec = probeSpec; + this.probeType = probeType; } public boolean isSuccessful() { @@ -98,4 +121,40 @@ public final class CaptivePortalProbeResult { public boolean isDnsPrivateIpResponse() { return mHttpResponseCode == DNS_PRIVATE_IP_RESPONSE_CODE; } + + /** + * Make a failed {@code this} for either HTTPS or HTTP determined by {@param isHttps}. + * @param probeType probe type of the result. + * @return a failed {@link CaptivePortalProbeResult} + */ + public static CaptivePortalProbeResult failed(@ProbeType int probeType) { + return new CaptivePortalProbeResult(FAILED_CODE, probeType); + } + + /** + * Make a success {@code this} for either HTTPS or HTTP determined by {@param isHttps}. + * @param probeType probe type of the result. + * @return a success {@link CaptivePortalProbeResult} + */ + public static CaptivePortalProbeResult success(@ProbeType int probeType) { + return new CaptivePortalProbeResult(SUCCESS_CODE, probeType); + } + + /** + * As {@code this} is the result of calling isCaptivePortal(), the result may be determined from + * one or more probes depending on the configuration of detection probes. Return if HTTPS probe + * is one of the probes that concludes it. + */ + public boolean isConcludedFromHttps() { + return (probeType & (1 << PROBE_HTTPS)) != 0; + } + + /** + * As {@code this} is the result of calling isCaptivePortal(), the result may be determined from + * one or more probes depending on the configuration of detection probes. Return if HTTP probe + * is one of the probes that concludes it. + */ + public boolean isConcludedFromHttp() { + return (probeType & (1 << PROBE_HTTP)) != 0; + } } diff --git a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeSpec.java b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeSpec.java index bf983a5..f58f50b 100644 --- a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeSpec.java +++ b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeSpec.java @@ -18,6 +18,7 @@ package android.net.captiveportal; import static android.net.captiveportal.CaptivePortalProbeResult.PORTAL_CODE; import static android.net.captiveportal.CaptivePortalProbeResult.SUCCESS_CODE; +import static android.net.metrics.ValidationProbeEvent.PROBE_HTTP; import android.text.TextUtils; import android.util.Log; @@ -40,6 +41,7 @@ public abstract class CaptivePortalProbeSpec { private static final String TAG = CaptivePortalProbeSpec.class.getSimpleName(); private static final String REGEX_SEPARATOR = "@@/@@"; private static final String SPEC_SEPARATOR = "@@,@@"; + private static final String PROTOCOL_HTTP = "http"; private final String mEncodedSpec; private final URL mUrl; @@ -138,7 +140,7 @@ public abstract class CaptivePortalProbeSpec { } /** - * Get the probe result from HTTP status and location header. + * Get the HTTP probe result from HTTP status and location header. */ @NonNull public abstract CaptivePortalProbeResult getResult(int status, @Nullable String locationHeader); @@ -157,6 +159,7 @@ public abstract class CaptivePortalProbeSpec { * Implementation of {@link CaptivePortalProbeSpec} that is based on configurable regular * expressions for the HTTP status code and location header (if any). Matches indicate that * the page is not a portal. + * @throws IllegalArgumentException The protocol of the url is not http. * This probe cannot fail: it always returns SUCCESS_CODE or PORTAL_CODE */ private static class RegexMatchProbeSpec extends CaptivePortalProbeSpec { @@ -165,9 +168,17 @@ public abstract class CaptivePortalProbeSpec { @Nullable final Pattern mLocationHeaderRegex; - RegexMatchProbeSpec( - String spec, URL url, Pattern statusRegex, Pattern locationHeaderRegex) { + RegexMatchProbeSpec(@NonNull String spec, @NonNull URL url, @Nullable Pattern statusRegex, + @Nullable Pattern locationHeaderRegex) throws ParseException { super(spec, url); + final String protocol = url.getProtocol(); + if (!PROTOCOL_HTTP.equals(protocol)) { + // The probe type taken in the result is hard-coded as PROBE_HTTP currently, so the + // probe type taken into the result of {@code getResult} have to change if other + // kind of probes are used. + throw new IllegalArgumentException("Protocol for probe spec should be http but was" + + protocol); + } mStatusRegex = statusRegex; mLocationHeaderRegex = locationHeaderRegex; } @@ -178,7 +189,7 @@ public abstract class CaptivePortalProbeSpec { final boolean locationMatch = safeMatch(locationHeader, mLocationHeaderRegex); final int returnCode = statusMatch && locationMatch ? SUCCESS_CODE : PORTAL_CODE; return new CaptivePortalProbeResult( - returnCode, locationHeader, getUrl().toString(), this); + returnCode, locationHeader, getUrl().toString(), this, PROBE_HTTP); } } diff --git a/src/android/net/captiveportal/CapportApiProbeResult.java b/src/android/net/captiveportal/CapportApiProbeResult.java new file mode 100644 index 0000000..3ef07ce --- /dev/null +++ b/src/android/net/captiveportal/CapportApiProbeResult.java @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package android.net.captiveportal; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + +import com.android.networkstack.apishim.CaptivePortalDataShim; + +/** + * Captive portal probe detection result including capport API detection result. + * @hide + */ +public class CapportApiProbeResult extends CaptivePortalProbeResult { + @NonNull + private final CaptivePortalDataShim mCapportData; + + public CapportApiProbeResult(@NonNull CaptivePortalProbeResult result, + @NonNull CaptivePortalDataShim capportData) { + this(result.mHttpResponseCode, result.redirectUrl, result.detectUrl, capportData, + result.probeType); + } + + public CapportApiProbeResult(int httpResponseCode, @Nullable String redirectUrl, + @Nullable String detectUrl, @Nullable CaptivePortalDataShim capportData, + int probeType) { + super(httpResponseCode, redirectUrl, detectUrl, probeType); + mCapportData = capportData; + } + + public CaptivePortalDataShim getCaptivePortalData() { + return mCapportData; + } +} diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java index eed63e6..40ad87d 100755 --- a/src/com/android/server/connectivity/NetworkMonitor.java +++ b/src/com/android/server/connectivity/NetworkMonitor.java @@ -103,6 +103,7 @@ import android.net.NetworkCapabilities; import android.net.ProxyInfo; import android.net.TrafficStats; import android.net.Uri; +import android.net.captiveportal.CapportApiProbeResult; import android.net.captiveportal.CaptivePortalProbeResult; import android.net.captiveportal.CaptivePortalProbeSpec; import android.net.metrics.IpConnectivityLog; @@ -406,7 +407,8 @@ public class NetworkMonitor extends StateMachine { private final Stopwatch mEvaluationTimer = new Stopwatch(); // This variable is set before transitioning to the mCaptivePortalState. - private CaptivePortalProbeResult mLastPortalProbeResult = CaptivePortalProbeResult.FAILED; + private CaptivePortalProbeResult mLastPortalProbeResult = + CaptivePortalProbeResult.failed(CaptivePortalProbeResult.PROBE_UNKNOWN); // Random generator to select fallback URL index private final Random mRandom; @@ -1840,7 +1842,7 @@ public class NetworkMonitor extends StateMachine { protected CaptivePortalProbeResult isCaptivePortal() { if (!mIsCaptivePortalCheckEnabled) { validationLog("Validation disabled."); - return CaptivePortalProbeResult.SUCCESS; + return CaptivePortalProbeResult.success(CaptivePortalProbeResult.PROBE_UNKNOWN); } URL pacUrl = null; @@ -1868,13 +1870,13 @@ public class NetworkMonitor extends StateMachine { if (proxyInfo != null && !Uri.EMPTY.equals(proxyInfo.getPacFileUrl())) { pacUrl = makeURL(proxyInfo.getPacFileUrl().toString()); if (pacUrl == null) { - return CaptivePortalProbeResult.FAILED; + return CaptivePortalProbeResult.failed(CaptivePortalProbeResult.PROBE_UNKNOWN); } } if ((pacUrl == null) && (httpUrls.length == 0 || httpsUrls.length == 0 || httpUrls[0] == null || httpsUrls[0] == null)) { - return CaptivePortalProbeResult.FAILED; + return CaptivePortalProbeResult.failed(CaptivePortalProbeResult.PROBE_UNKNOWN); } long startTime = SystemClock.elapsedRealtime(); @@ -2073,7 +2075,8 @@ public class NetworkMonitor extends StateMachine { logValidationProbe(probeTimer.stop(), probeType, httpResponseCode); if (probeSpec == null) { - return new CaptivePortalProbeResult(httpResponseCode, redirectUrl, url.toString()); + return new CaptivePortalProbeResult(httpResponseCode, redirectUrl, url.toString(), + 1 << probeType); } else { return probeSpec.getResult(httpResponseCode, redirectUrl); } @@ -2162,33 +2165,29 @@ public class NetworkMonitor extends StateMachine { } } - private abstract static class ProbeThread extends Thread { + private class ProbeThread extends Thread { private final CountDownLatch mLatch; - private final ProxyInfo mProxy; - private final URL mUrl; - protected final Uri mCaptivePortalApiUrl; + private final Probe mProbe; - protected ProbeThread(CountDownLatch latch, ProxyInfo proxy, URL url, + ProbeThread(CountDownLatch latch, ProxyInfo proxy, URL url, int probeType, Uri captivePortalApiUrl) { mLatch = latch; - mProxy = proxy; - mUrl = url; - mCaptivePortalApiUrl = captivePortalApiUrl; + mProbe = (probeType == ValidationProbeEvent.PROBE_HTTPS) + ? new HttpsProbe(proxy, url, captivePortalApiUrl) + : new HttpProbe(proxy, url, captivePortalApiUrl); + mResult = CaptivePortalProbeResult.failed(probeType); } - private volatile CaptivePortalProbeResult mResult = CaptivePortalProbeResult.FAILED; + private volatile CaptivePortalProbeResult mResult; public CaptivePortalProbeResult result() { return mResult; } - protected abstract CaptivePortalProbeResult sendProbe(ProxyInfo proxy, URL url); - public abstract boolean isConclusiveResult(CaptivePortalProbeResult result); - @Override public void run() { - mResult = sendProbe(mProxy, mUrl); - if (isConclusiveResult(mResult)) { + mResult = mProbe.sendProbe(); + if (isConclusiveResult(mResult, mProbe.mCaptivePortalApiUrl)) { // Stop waiting immediately if any probe is conclusive. while (mLatch.getCount() > 0) { mLatch.countDown(); @@ -2199,36 +2198,34 @@ public class NetworkMonitor extends StateMachine { } } - final class HttpsProbeThread extends ProbeThread { - HttpsProbeThread(CountDownLatch latch, ProxyInfo proxy, URL url, Uri captivePortalApiUrl) { - super(latch, proxy, url, captivePortalApiUrl); + private abstract static class Probe { + protected final ProxyInfo mProxy; + protected final URL mUrl; + protected final Uri mCaptivePortalApiUrl; + + protected Probe(ProxyInfo proxy, URL url, Uri captivePortalApiUrl) { + mProxy = proxy; + mUrl = url; + mCaptivePortalApiUrl = captivePortalApiUrl; } + // sendProbe() is synchronous and blocks until it has the result. + protected abstract CaptivePortalProbeResult sendProbe(); + } - @Override - protected CaptivePortalProbeResult sendProbe(ProxyInfo proxy, URL url) { - return sendDnsAndHttpProbes(proxy, url, ValidationProbeEvent.PROBE_HTTPS); + final class HttpsProbe extends Probe { + HttpsProbe(ProxyInfo proxy, URL url, Uri captivePortalApiUrl) { + super(proxy, url, captivePortalApiUrl); } @Override - public boolean isConclusiveResult(CaptivePortalProbeResult result) { - // isPortal() is not expected on the HTTPS probe, but check it nonetheless. - // In case the capport API is available, the API is authoritative on whether there is - // a portal, so the HTTPS probe is not enough to conclude there is connectivity, - // and a determination will be made once the capport API probe returns. Note that the - // API can only force the system to detect a portal even if the HTTPS probe succeeds. - // It cannot force the system to detect no portal if the HTTPS probe fails. - return (result.isPortal() || result.isSuccessful()) && mCaptivePortalApiUrl == null; + protected CaptivePortalProbeResult sendProbe() { + return sendDnsAndHttpProbes(mProxy, mUrl, ValidationProbeEvent.PROBE_HTTPS); } } - final class HttpProbeThread extends ProbeThread { - private volatile CaptivePortalDataShim mCapportData; - HttpProbeThread(CountDownLatch latch, ProxyInfo proxy, URL url, Uri captivePortalApiUrl) { - super(latch, proxy, url, captivePortalApiUrl); - } - - CaptivePortalDataShim getCaptivePortalData() { - return mCapportData; + final class HttpProbe extends Probe { + HttpProbe(ProxyInfo proxy, URL url, Uri captivePortalApiUrl) { + super(proxy, url, captivePortalApiUrl); } private CaptivePortalDataShim tryCapportApiProbe() { @@ -2277,33 +2274,61 @@ public class NetworkMonitor extends StateMachine { } @Override - protected CaptivePortalProbeResult sendProbe(ProxyInfo proxy, URL url) { - mCapportData = tryCapportApiProbe(); - if (mCapportData != null && mCapportData.isCaptive()) { - if (mCapportData.getUserPortalUrl() == null) { + protected CaptivePortalProbeResult sendProbe() { + final CaptivePortalDataShim capportData = tryCapportApiProbe(); + if (capportData != null && capportData.isCaptive()) { + if (capportData.getUserPortalUrl() == null) { validationLog("Missing user-portal-url from capport response"); - return sendDnsAndHttpProbes(proxy, url, ValidationProbeEvent.PROBE_HTTP); + return sendDnsAndHttpProbes(mProxy, mUrl, ValidationProbeEvent.PROBE_HTTP); } - final String loginUrlString = mCapportData.getUserPortalUrl().toString(); + final String loginUrlString = capportData.getUserPortalUrl().toString(); // Starting from R (where CaptivePortalData was introduced), the captive portal app // delegates to NetworkMonitor for verifying when the network validates instead of // probing the detectUrl. So pass the detectUrl to have the portal open on that, // page; CaptivePortalLogin will not use it for probing. - return new CaptivePortalProbeResult( + return new CapportApiProbeResult( CaptivePortalProbeResult.PORTAL_CODE, loginUrlString /* redirectUrl */, - loginUrlString /* detectUrl */); + loginUrlString /* detectUrl */, + capportData, + 1 << ValidationProbeEvent.PROBE_HTTP); } // If the API says it's not captive, still check for HTTP connectivity. This helps // with partial connectivity detection, and a broken API saying that there is no // redirect when there is one. - return sendDnsAndHttpProbes(proxy, url, ValidationProbeEvent.PROBE_HTTP); + final CaptivePortalProbeResult res = + sendDnsAndHttpProbes(mProxy, mUrl, ValidationProbeEvent.PROBE_HTTP); + return capportData == null ? res : new CapportApiProbeResult(res, capportData); } + } - @Override - public boolean isConclusiveResult(CaptivePortalProbeResult result) { - return result.isPortal(); + private static boolean isConclusiveResult(@NonNull CaptivePortalProbeResult result, + @Nullable Uri captivePortalApiUrl) { + // isPortal() is not expected on the HTTPS probe, but treat the network as portal would make + // sense if the probe reports portal. In case the capport API is available, the API is + // authoritative on whether there is a portal, so the HTTPS probe is not enough to conclude + // there is connectivity, and a determination will be made once the capport API probe + // returns. Note that the API can only force the system to detect a portal even if the HTTPS + // probe succeeds. It cannot force the system to detect no portal if the HTTPS probe fails. + return result.isPortal() + || (result.isConcludedFromHttps() && result.isSuccessful() + && captivePortalApiUrl == null); + } + + private void reportProbeResult(@NonNull CaptivePortalProbeResult res) { + if (res instanceof CapportApiProbeResult) { + maybeReportCaptivePortalData(((CapportApiProbeResult) res).getCaptivePortalData()); + } + + // This is not a if-else case since partial connectivity will concluded from both HTTP and + // HTTPS probe. Both HTTP and HTTPS result should be reported. + if (res.isConcludedFromHttps()) { + reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTPS, res); + } + + if (res.isConcludedFromHttp()) { + reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, res); } } @@ -2314,9 +2339,10 @@ public class NetworkMonitor extends StateMachine { final CountDownLatch latch = new CountDownLatch(2); final Uri capportApiUrl = getCaptivePortalApiUrl(mLinkProperties); - final HttpsProbeThread httpsProbe = new HttpsProbeThread(latch, proxy, httpsUrl, - capportApiUrl); - final HttpProbeThread httpProbe = new HttpProbeThread(latch, proxy, httpUrl, capportApiUrl); + final ProbeThread httpsProbe = new ProbeThread(latch, proxy, httpsUrl, + ValidationProbeEvent.PROBE_HTTPS, capportApiUrl); + final ProbeThread httpProbe = new ProbeThread(latch, proxy, httpUrl, + ValidationProbeEvent.PROBE_HTTP, capportApiUrl); try { httpsProbe.start(); @@ -2324,22 +2350,20 @@ public class NetworkMonitor extends StateMachine { latch.await(PROBE_TIMEOUT_MS, TimeUnit.MILLISECONDS); } catch (InterruptedException e) { validationLog("Error: probes wait interrupted!"); - return CaptivePortalProbeResult.FAILED; + return CaptivePortalProbeResult.failed(CaptivePortalProbeResult.PROBE_UNKNOWN); } final CaptivePortalProbeResult httpsResult = httpsProbe.result(); final CaptivePortalProbeResult httpResult = httpProbe.result(); // Look for a conclusive probe result first. - if (httpProbe.isConclusiveResult(httpResult)) { - maybeReportCaptivePortalData(httpProbe.getCaptivePortalData()); - reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, httpResult); + if (isConclusiveResult(httpResult, capportApiUrl)) { + reportProbeResult(httpProbe.result()); return httpResult; } - if (httpsProbe.isConclusiveResult(httpsResult)) { - maybeReportCaptivePortalData(httpProbe.getCaptivePortalData()); - reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTPS, httpsResult); + if (isConclusiveResult(httpsResult, capportApiUrl)) { + reportProbeResult(httpsProbe.result()); return httpsResult; } // Consider a DNS response with a private IP address on the HTTP probe as an indication that @@ -2350,7 +2374,7 @@ public class NetworkMonitor extends StateMachine { // probe should not be delayed by this check. if (mPrivateIpNoInternetEnabled && (httpResult.isDnsPrivateIpResponse())) { validationLog("DNS response to the URL is private IP"); - return CaptivePortalProbeResult.FAILED; + return CaptivePortalProbeResult.failed(1 << ValidationProbeEvent.PROBE_HTTP); } // 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. @@ -2367,8 +2391,7 @@ public class NetworkMonitor extends StateMachine { // Otherwise wait until http and https probes completes and use their results. try { httpProbe.join(); - maybeReportCaptivePortalData(httpProbe.getCaptivePortalData()); - reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, httpProbe.result()); + reportProbeResult(httpProbe.result()); if (httpProbe.result().isPortal()) { return httpProbe.result(); @@ -2386,7 +2409,7 @@ public class NetworkMonitor extends StateMachine { return httpsProbe.result(); } catch (InterruptedException e) { validationLog("Error: http or https probe wait interrupted!"); - return CaptivePortalProbeResult.FAILED; + return CaptivePortalProbeResult.failed(CaptivePortalProbeResult.PROBE_UNKNOWN); } } diff --git a/tests/unit/src/com/android/networkstack/metrics/DataStallStatsUtilsTest.kt b/tests/unit/src/com/android/networkstack/metrics/DataStallStatsUtilsTest.kt index 3820165..4c62071 100644 --- a/tests/unit/src/com/android/networkstack/metrics/DataStallStatsUtilsTest.kt +++ b/tests/unit/src/com/android/networkstack/metrics/DataStallStatsUtilsTest.kt @@ -23,6 +23,7 @@ import com.android.server.connectivity.nano.DataStallEventProto import org.junit.Assert.assertEquals import org.junit.Test import org.junit.runner.RunWith +import android.net.metrics.ValidationProbeEvent @RunWith(AndroidJUnit4::class) @SmallTest @@ -30,14 +31,26 @@ class DataStallStatsUtilsTest { @Test fun testProbeResultToEnum() { assertEquals(DataStallStatsUtils.probeResultToEnum(null), DataStallEventProto.INVALID) - assertEquals(DataStallStatsUtils.probeResultToEnum(CaptivePortalProbeResult.FAILED), + // Metrics cares only http response code. + assertEquals(DataStallStatsUtils.probeResultToEnum( + CaptivePortalProbeResult.failed(ValidationProbeEvent.PROBE_HTTP)), + DataStallEventProto.INVALID) + assertEquals(DataStallStatsUtils.probeResultToEnum( + CaptivePortalProbeResult.failed(ValidationProbeEvent.PROBE_HTTPS)), DataStallEventProto.INVALID) - assertEquals(DataStallStatsUtils.probeResultToEnum(CaptivePortalProbeResult.SUCCESS), + assertEquals(DataStallStatsUtils.probeResultToEnum( + CaptivePortalProbeResult.success(ValidationProbeEvent.PROBE_HTTP)), + DataStallEventProto.VALID) + assertEquals(DataStallStatsUtils.probeResultToEnum( + CaptivePortalProbeResult.success(ValidationProbeEvent.PROBE_HTTP)), DataStallEventProto.VALID) assertEquals(DataStallStatsUtils.probeResultToEnum(CaptivePortalProbeResult.PARTIAL), DataStallEventProto.PARTIAL) - assertEquals(DataStallStatsUtils.probeResultToEnum( - CaptivePortalProbeResult(CaptivePortalProbeResult.PORTAL_CODE)), + assertEquals(DataStallStatsUtils.probeResultToEnum(CaptivePortalProbeResult( + CaptivePortalProbeResult.PORTAL_CODE, ValidationProbeEvent.PROBE_HTTP)), + DataStallEventProto.PORTAL) + assertEquals(DataStallStatsUtils.probeResultToEnum(CaptivePortalProbeResult( + CaptivePortalProbeResult.PORTAL_CODE, ValidationProbeEvent.PROBE_HTTPS)), DataStallEventProto.PORTAL) } } diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java index 6a0aa8a..64c106d 100644 --- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java @@ -27,6 +27,7 @@ import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_PRIVDNS; import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_PARTIAL; import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_VALID; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; +import static android.net.metrics.ValidationProbeEvent.PROBE_HTTP; import static android.net.util.DataStallUtils.CONFIG_DATA_STALL_CONSECUTIVE_DNS_TIMEOUT_THRESHOLD; import static android.net.util.DataStallUtils.CONFIG_DATA_STALL_EVALUATION_TYPE; import static android.net.util.DataStallUtils.CONFIG_DATA_STALL_MIN_EVALUATE_INTERVAL; @@ -1731,13 +1732,15 @@ public class NetworkMonitorTest { resetCallbacks(); - nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, CaptivePortalProbeResult.SUCCESS); + nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, + CaptivePortalProbeResult.success(PROBE_HTTP)); // Verify result should be appended and notifyNetworkTestedWithExtras callback is triggered // once. assertEquals(nm.getEvaluationState().getNetworkTestResult(), VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP); - nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, CaptivePortalProbeResult.FAILED); + nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, + CaptivePortalProbeResult.failed(PROBE_HTTP)); // Verify DNS probe result should not be cleared. assertTrue((nm.getEvaluationState().getNetworkTestResult() & NETWORK_VALIDATION_PROBE_DNS) == NETWORK_VALIDATION_PROBE_DNS); |