Loading src/com/android/server/connectivity/NetworkMonitor.java +16 −7 Original line number Original line Diff line number Diff line Loading @@ -1034,7 +1034,7 @@ public class NetworkMonitor extends StateMachine { mPrivateDnsConfig = null; mPrivateDnsConfig = null; validationLog("Strict mode hostname resolution failed: " + uhe.getMessage()); validationLog("Strict mode hostname resolution failed: " + uhe.getMessage()); } } mEvaluationState.reportProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, (mPrivateDnsConfig != null) /* succeeded */); (mPrivateDnsConfig != null) /* succeeded */); } } Loading Loading @@ -1086,7 +1086,7 @@ public class NetworkMonitor extends StateMachine { String.format("%dms - Error: %s", time, uhe.getMessage())); String.format("%dms - Error: %s", time, uhe.getMessage())); } } logValidationProbe(time, PROBE_PRIVDNS, success ? DNS_SUCCESS : DNS_FAILURE); logValidationProbe(time, PROBE_PRIVDNS, success ? DNS_SUCCESS : DNS_FAILURE); mEvaluationState.reportProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, success); mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, success); return success; return success; } } } } Loading Loading @@ -2067,11 +2067,21 @@ public class NetworkMonitor extends StateMachine { } } // Class to keep state of evaluation results and probe results. // Class to keep state of evaluation results and probe results. // The main purpose is to ensure NetworkMonitor can notify ConnectivityService of probe results // // The main purpose was to ensure NetworkMonitor can notify ConnectivityService of probe results // as soon as they happen, without triggering any other changes. This requires keeping state on // as soon as they happen, without triggering any other changes. This requires keeping state on // the most recent evaluation result. Calling reportProbeResult will ensure that the results // the most recent evaluation result. Calling noteProbeResult will ensure that the results // reported to ConnectivityService contain the previous evaluation result, and thus won't // reported to ConnectivityService contain the previous evaluation result, and thus won't // trigger a validation or partial connectivity state change. // trigger a validation or partial connectivity state change. // // Note that this class is not currently being used for this purpose. The reason is that some // of the system behaviour triggered by reporting network validation - notably, NetworkAgent // behaviour - depends not only on the value passed by notifyNetworkTested, but also on the // fact that notifyNetworkTested was called. For example, telephony triggers network recovery // any time it is told that validation failed, i.e., if the result does not contain // NETWORK_VALIDATION_RESULT_VALID. But with this scheme, the first two or three validation // reports are all failures, because they are "HTTP succeeded but validation not yet passed", // "HTTP and HTTPS succeeded but validation not yet passed", etc. @VisibleForTesting @VisibleForTesting protected class EvaluationState { protected class EvaluationState { // The latest validation result for this network. This is a bitmask of // The latest validation result for this network. This is a bitmask of Loading @@ -2088,13 +2098,12 @@ public class NetworkMonitor extends StateMachine { } } // Probe result for http probe should be updated from reportHttpProbeResult(). // Probe result for http probe should be updated from reportHttpProbeResult(). protected void reportProbeResult(int probeResult, boolean succeeded) { protected void noteProbeResult(int probeResult, boolean succeeded) { if (succeeded) { if (succeeded) { mProbeResults |= probeResult; mProbeResults |= probeResult; } else { } else { mProbeResults &= ~probeResult; mProbeResults &= ~probeResult; } } notifyNetworkTested(getNetworkTestResult(), mRedirectUrl); } } protected void reportEvaluationResult(int result, @Nullable String redirectUrl) { protected void reportEvaluationResult(int result, @Nullable String redirectUrl) { Loading Loading @@ -2138,6 +2147,6 @@ public class NetworkMonitor extends StateMachine { if (succeeded) { if (succeeded) { probeResult |= NETWORK_VALIDATION_PROBE_DNS; probeResult |= NETWORK_VALIDATION_PROBE_DNS; } } mEvaluationState.reportProbeResult(probeResult, succeeded); mEvaluationState.noteProbeResult(probeResult, succeeded); } } } } tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +11 −41 Original line number Original line Diff line number Diff line Loading @@ -691,8 +691,7 @@ public class NetworkMonitorTest { @Test @Test public void testNoInternetCapabilityValidated() throws Exception { public void testNoInternetCapabilityValidated() throws Exception { runNetworkTest(NO_INTERNET_CAPABILITIES, NETWORK_VALIDATION_RESULT_VALID, runNetworkTest(NO_INTERNET_CAPABILITIES, NETWORK_VALIDATION_RESULT_VALID); getGeneralVerification()); verify(mCleartextDnsNetwork, never()).openConnection(any()); verify(mCleartextDnsNetwork, never()).openConnection(any()); } } Loading Loading @@ -780,8 +779,9 @@ public class NetworkMonitorTest { wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.bad", new InetAddress[0])); wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.bad", new InetAddress[0])); // Strict mode hostname resolve fail. Expect only notification for evaluation fail. No probe // Strict mode hostname resolve fail. Expect only notification for evaluation fail. No probe // notification. // notification. verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)) verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)) .notifyNetworkTested(eq(VALIDATION_RESULT_VALID), eq(null)); .notifyNetworkTested(eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS), eq(null)); // Change configuration back to working again, but make private DNS not work. // Change configuration back to working again, but make private DNS not work. // Expect validation to fail. // Expect validation to fail. Loading Loading @@ -937,21 +937,9 @@ public class NetworkMonitorTest { nm.forceReevaluation(Process.myUid()); nm.forceReevaluation(Process.myUid()); final ArgumentCaptor<Integer> intCaptor = ArgumentCaptor.forClass(Integer.class); final ArgumentCaptor<Integer> intCaptor = ArgumentCaptor.forClass(Integer.class); // Expect to send HTTP, HTTPs, FALLBACK and evaluation results. // Expect to send HTTP, HTTPs, FALLBACK and evaluation results. verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(4)) verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)) .notifyNetworkTested(intCaptor.capture(), any()); .notifyNetworkTested(eq(NETWORK_VALIDATION_PROBE_DNS | List<Integer> intArgs = intCaptor.getAllValues(); NETWORK_VALIDATION_PROBE_FALLBACK | NETWORK_VALIDATION_RESULT_PARTIAL), any()); // None of these exact values can be known in advance except for intArgs.get(0) because the // HTTP and HTTPS probes race and the order in which they complete is non-deterministic. // Thus, check only exact value for intArgs.get(0) and only check the validation result for // the rest ones. assertEquals(Integer.valueOf(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_FALLBACK | NETWORK_VALIDATION_RESULT_VALID), intArgs.get(0)); assertTrue((intArgs.get(1) & NETWORK_VALIDATION_RESULT_VALID) != 0); assertTrue((intArgs.get(2) & NETWORK_VALIDATION_RESULT_VALID) != 0); assertTrue((intArgs.get(3) & NETWORK_VALIDATION_RESULT_PARTIAL) != 0); assertTrue((intArgs.get(3) & NETWORK_VALIDATION_RESULT_VALID) == 0); } } @Test @Test Loading @@ -973,8 +961,6 @@ public class NetworkMonitorTest { // Verify result should be appended and notifyNetworkTested callback is triggered once. // Verify result should be appended and notifyNetworkTested callback is triggered once. assertEquals(nm.getEvaluationState().getNetworkTestResult(), assertEquals(nm.getEvaluationState().getNetworkTestResult(), VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP); VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyNetworkTested( eq(VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP), any()); nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, CaptivePortalProbeResult.FAILED); nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, CaptivePortalProbeResult.FAILED); // Verify DNS probe result should not be cleared. // Verify DNS probe result should not be cleared. Loading Loading @@ -1064,14 +1050,7 @@ public class NetworkMonitorTest { } } private void runPortalNetworkTest(int result) { private void runPortalNetworkTest(int result) { // The network test event will be triggered twice with the same result. Expect to capture runNetworkTest(result); // the second one with direct url. runPortalNetworkTest(result, (VerificationWithTimeout) timeout(HANDLER_TIMEOUT_MS).times(2)); } private void runPortalNetworkTest(int result, VerificationWithTimeout mode) { runNetworkTest(result, mode); assertEquals(1, mRegisteredReceivers.size()); assertEquals(1, mRegisteredReceivers.size()); assertNotNull(mNetworkTestedRedirectUrlCaptor.getValue()); assertNotNull(mNetworkTestedRedirectUrlCaptor.getValue()); } } Loading Loading @@ -1108,19 +1087,14 @@ public class NetworkMonitorTest { } } private NetworkMonitor runNetworkTest(int testResult) { private NetworkMonitor runNetworkTest(int testResult) { return runNetworkTest(METERED_CAPABILITIES, testResult, getGeneralVerification()); return runNetworkTest(METERED_CAPABILITIES, testResult); } } private NetworkMonitor runNetworkTest(int testResult, VerificationWithTimeout mode) { private NetworkMonitor runNetworkTest(NetworkCapabilities nc, int testResult) { return runNetworkTest(METERED_CAPABILITIES, testResult, mode); } private NetworkMonitor runNetworkTest(NetworkCapabilities nc, int testResult, VerificationWithTimeout mode) { final NetworkMonitor monitor = makeMonitor(nc); final NetworkMonitor monitor = makeMonitor(nc); monitor.notifyNetworkConnected(TEST_LINK_PROPERTIES, nc); monitor.notifyNetworkConnected(TEST_LINK_PROPERTIES, nc); try { try { verify(mCallbacks, mode) verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)) .notifyNetworkTested(eq(testResult), mNetworkTestedRedirectUrlCaptor.capture()); .notifyNetworkTested(eq(testResult), mNetworkTestedRedirectUrlCaptor.capture()); } catch (RemoteException e) { } catch (RemoteException e) { fail("Unexpected exception: " + e); fail("Unexpected exception: " + e); Loading Loading @@ -1153,9 +1127,5 @@ public class NetworkMonitorTest { } } } } private VerificationWithTimeout getGeneralVerification() { return (VerificationWithTimeout) timeout(HANDLER_TIMEOUT_MS).atLeastOnce(); } } } Loading
src/com/android/server/connectivity/NetworkMonitor.java +16 −7 Original line number Original line Diff line number Diff line Loading @@ -1034,7 +1034,7 @@ public class NetworkMonitor extends StateMachine { mPrivateDnsConfig = null; mPrivateDnsConfig = null; validationLog("Strict mode hostname resolution failed: " + uhe.getMessage()); validationLog("Strict mode hostname resolution failed: " + uhe.getMessage()); } } mEvaluationState.reportProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, (mPrivateDnsConfig != null) /* succeeded */); (mPrivateDnsConfig != null) /* succeeded */); } } Loading Loading @@ -1086,7 +1086,7 @@ public class NetworkMonitor extends StateMachine { String.format("%dms - Error: %s", time, uhe.getMessage())); String.format("%dms - Error: %s", time, uhe.getMessage())); } } logValidationProbe(time, PROBE_PRIVDNS, success ? DNS_SUCCESS : DNS_FAILURE); logValidationProbe(time, PROBE_PRIVDNS, success ? DNS_SUCCESS : DNS_FAILURE); mEvaluationState.reportProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, success); mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, success); return success; return success; } } } } Loading Loading @@ -2067,11 +2067,21 @@ public class NetworkMonitor extends StateMachine { } } // Class to keep state of evaluation results and probe results. // Class to keep state of evaluation results and probe results. // The main purpose is to ensure NetworkMonitor can notify ConnectivityService of probe results // // The main purpose was to ensure NetworkMonitor can notify ConnectivityService of probe results // as soon as they happen, without triggering any other changes. This requires keeping state on // as soon as they happen, without triggering any other changes. This requires keeping state on // the most recent evaluation result. Calling reportProbeResult will ensure that the results // the most recent evaluation result. Calling noteProbeResult will ensure that the results // reported to ConnectivityService contain the previous evaluation result, and thus won't // reported to ConnectivityService contain the previous evaluation result, and thus won't // trigger a validation or partial connectivity state change. // trigger a validation or partial connectivity state change. // // Note that this class is not currently being used for this purpose. The reason is that some // of the system behaviour triggered by reporting network validation - notably, NetworkAgent // behaviour - depends not only on the value passed by notifyNetworkTested, but also on the // fact that notifyNetworkTested was called. For example, telephony triggers network recovery // any time it is told that validation failed, i.e., if the result does not contain // NETWORK_VALIDATION_RESULT_VALID. But with this scheme, the first two or three validation // reports are all failures, because they are "HTTP succeeded but validation not yet passed", // "HTTP and HTTPS succeeded but validation not yet passed", etc. @VisibleForTesting @VisibleForTesting protected class EvaluationState { protected class EvaluationState { // The latest validation result for this network. This is a bitmask of // The latest validation result for this network. This is a bitmask of Loading @@ -2088,13 +2098,12 @@ public class NetworkMonitor extends StateMachine { } } // Probe result for http probe should be updated from reportHttpProbeResult(). // Probe result for http probe should be updated from reportHttpProbeResult(). protected void reportProbeResult(int probeResult, boolean succeeded) { protected void noteProbeResult(int probeResult, boolean succeeded) { if (succeeded) { if (succeeded) { mProbeResults |= probeResult; mProbeResults |= probeResult; } else { } else { mProbeResults &= ~probeResult; mProbeResults &= ~probeResult; } } notifyNetworkTested(getNetworkTestResult(), mRedirectUrl); } } protected void reportEvaluationResult(int result, @Nullable String redirectUrl) { protected void reportEvaluationResult(int result, @Nullable String redirectUrl) { Loading Loading @@ -2138,6 +2147,6 @@ public class NetworkMonitor extends StateMachine { if (succeeded) { if (succeeded) { probeResult |= NETWORK_VALIDATION_PROBE_DNS; probeResult |= NETWORK_VALIDATION_PROBE_DNS; } } mEvaluationState.reportProbeResult(probeResult, succeeded); mEvaluationState.noteProbeResult(probeResult, succeeded); } } } }
tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +11 −41 Original line number Original line Diff line number Diff line Loading @@ -691,8 +691,7 @@ public class NetworkMonitorTest { @Test @Test public void testNoInternetCapabilityValidated() throws Exception { public void testNoInternetCapabilityValidated() throws Exception { runNetworkTest(NO_INTERNET_CAPABILITIES, NETWORK_VALIDATION_RESULT_VALID, runNetworkTest(NO_INTERNET_CAPABILITIES, NETWORK_VALIDATION_RESULT_VALID); getGeneralVerification()); verify(mCleartextDnsNetwork, never()).openConnection(any()); verify(mCleartextDnsNetwork, never()).openConnection(any()); } } Loading Loading @@ -780,8 +779,9 @@ public class NetworkMonitorTest { wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.bad", new InetAddress[0])); wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.bad", new InetAddress[0])); // Strict mode hostname resolve fail. Expect only notification for evaluation fail. No probe // Strict mode hostname resolve fail. Expect only notification for evaluation fail. No probe // notification. // notification. verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)) verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)) .notifyNetworkTested(eq(VALIDATION_RESULT_VALID), eq(null)); .notifyNetworkTested(eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS), eq(null)); // Change configuration back to working again, but make private DNS not work. // Change configuration back to working again, but make private DNS not work. // Expect validation to fail. // Expect validation to fail. Loading Loading @@ -937,21 +937,9 @@ public class NetworkMonitorTest { nm.forceReevaluation(Process.myUid()); nm.forceReevaluation(Process.myUid()); final ArgumentCaptor<Integer> intCaptor = ArgumentCaptor.forClass(Integer.class); final ArgumentCaptor<Integer> intCaptor = ArgumentCaptor.forClass(Integer.class); // Expect to send HTTP, HTTPs, FALLBACK and evaluation results. // Expect to send HTTP, HTTPs, FALLBACK and evaluation results. verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(4)) verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)) .notifyNetworkTested(intCaptor.capture(), any()); .notifyNetworkTested(eq(NETWORK_VALIDATION_PROBE_DNS | List<Integer> intArgs = intCaptor.getAllValues(); NETWORK_VALIDATION_PROBE_FALLBACK | NETWORK_VALIDATION_RESULT_PARTIAL), any()); // None of these exact values can be known in advance except for intArgs.get(0) because the // HTTP and HTTPS probes race and the order in which they complete is non-deterministic. // Thus, check only exact value for intArgs.get(0) and only check the validation result for // the rest ones. assertEquals(Integer.valueOf(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_FALLBACK | NETWORK_VALIDATION_RESULT_VALID), intArgs.get(0)); assertTrue((intArgs.get(1) & NETWORK_VALIDATION_RESULT_VALID) != 0); assertTrue((intArgs.get(2) & NETWORK_VALIDATION_RESULT_VALID) != 0); assertTrue((intArgs.get(3) & NETWORK_VALIDATION_RESULT_PARTIAL) != 0); assertTrue((intArgs.get(3) & NETWORK_VALIDATION_RESULT_VALID) == 0); } } @Test @Test Loading @@ -973,8 +961,6 @@ public class NetworkMonitorTest { // Verify result should be appended and notifyNetworkTested callback is triggered once. // Verify result should be appended and notifyNetworkTested callback is triggered once. assertEquals(nm.getEvaluationState().getNetworkTestResult(), assertEquals(nm.getEvaluationState().getNetworkTestResult(), VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP); VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyNetworkTested( eq(VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP), any()); nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, CaptivePortalProbeResult.FAILED); nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, CaptivePortalProbeResult.FAILED); // Verify DNS probe result should not be cleared. // Verify DNS probe result should not be cleared. Loading Loading @@ -1064,14 +1050,7 @@ public class NetworkMonitorTest { } } private void runPortalNetworkTest(int result) { private void runPortalNetworkTest(int result) { // The network test event will be triggered twice with the same result. Expect to capture runNetworkTest(result); // the second one with direct url. runPortalNetworkTest(result, (VerificationWithTimeout) timeout(HANDLER_TIMEOUT_MS).times(2)); } private void runPortalNetworkTest(int result, VerificationWithTimeout mode) { runNetworkTest(result, mode); assertEquals(1, mRegisteredReceivers.size()); assertEquals(1, mRegisteredReceivers.size()); assertNotNull(mNetworkTestedRedirectUrlCaptor.getValue()); assertNotNull(mNetworkTestedRedirectUrlCaptor.getValue()); } } Loading Loading @@ -1108,19 +1087,14 @@ public class NetworkMonitorTest { } } private NetworkMonitor runNetworkTest(int testResult) { private NetworkMonitor runNetworkTest(int testResult) { return runNetworkTest(METERED_CAPABILITIES, testResult, getGeneralVerification()); return runNetworkTest(METERED_CAPABILITIES, testResult); } } private NetworkMonitor runNetworkTest(int testResult, VerificationWithTimeout mode) { private NetworkMonitor runNetworkTest(NetworkCapabilities nc, int testResult) { return runNetworkTest(METERED_CAPABILITIES, testResult, mode); } private NetworkMonitor runNetworkTest(NetworkCapabilities nc, int testResult, VerificationWithTimeout mode) { final NetworkMonitor monitor = makeMonitor(nc); final NetworkMonitor monitor = makeMonitor(nc); monitor.notifyNetworkConnected(TEST_LINK_PROPERTIES, nc); monitor.notifyNetworkConnected(TEST_LINK_PROPERTIES, nc); try { try { verify(mCallbacks, mode) verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)) .notifyNetworkTested(eq(testResult), mNetworkTestedRedirectUrlCaptor.capture()); .notifyNetworkTested(eq(testResult), mNetworkTestedRedirectUrlCaptor.capture()); } catch (RemoteException e) { } catch (RemoteException e) { fail("Unexpected exception: " + e); fail("Unexpected exception: " + e); Loading Loading @@ -1153,9 +1127,5 @@ public class NetworkMonitorTest { } } } } private VerificationWithTimeout getGeneralVerification() { return (VerificationWithTimeout) timeout(HANDLER_TIMEOUT_MS).atLeastOnce(); } } }