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

Commit 23d44496 authored by Lorenzo Colitti's avatar Lorenzo Colitti Committed by android-build-merger
Browse files

Don't report probe status to ConnectivityService. 

am: aa16f2d4

Change-Id: I228ec8c935bf80d80c240963f9aa47f8f33a4fc0
parents 77a40ed8 aa16f2d4

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

+16 −7
Original line number Diff line number Diff line
@@ -1034,7 +1034,7 @@ public class NetworkMonitor extends StateMachine { 
                mPrivateDnsConfig = null;

                validationLog("Strict mode hostname resolution failed: " + uhe.getMessage());

            }

            mEvaluationState.reportProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS,

            mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS,

                    (mPrivateDnsConfig != null) /* succeeded */);

        }
@@ -1086,7 +1086,7 @@ public class NetworkMonitor extends StateMachine { 
                        String.format("%dms - Error: %s", time, uhe.getMessage()));

            }

            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;

        }

    }
@@ -2067,11 +2067,21 @@ public class NetworkMonitor extends StateMachine { 
    }



    // 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

    // 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

    // 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

    protected class EvaluationState {

        // The latest validation result for this network. This is a bitmask of
@@ -2088,13 +2098,12 @@ public class NetworkMonitor extends StateMachine { 
        }



        // 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) {

                mProbeResults |= probeResult;

            } else {

                mProbeResults &= ~probeResult;

            }

            notifyNetworkTested(getNetworkTestResult(), mRedirectUrl);

        }



        protected void reportEvaluationResult(int result, @Nullable String redirectUrl) {
@@ -2138,6 +2147,6 @@ public class NetworkMonitor extends StateMachine { 
        if (succeeded) {

            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 Diff line number Diff line
@@ -684,8 +684,7 @@ public class NetworkMonitorTest { 


    @Test

    public void testNoInternetCapabilityValidated() throws Exception {

        runNetworkTest(NO_INTERNET_CAPABILITIES, NETWORK_VALIDATION_RESULT_VALID,

                getGeneralVerification());

        runNetworkTest(NO_INTERNET_CAPABILITIES, NETWORK_VALIDATION_RESULT_VALID);

        verify(mCleartextDnsNetwork, never()).openConnection(any());

    }
@@ -773,8 +772,9 @@ public class NetworkMonitorTest { 
        wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.bad", new InetAddress[0]));

        // Strict mode hostname resolve fail. Expect only notification for evaluation fail. No probe

        // notification.

        verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1))

                .notifyNetworkTested(eq(VALIDATION_RESULT_VALID), eq(null));

        verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS))

                .notifyNetworkTested(eq(NETWORK_VALIDATION_PROBE_DNS

                        | NETWORK_VALIDATION_PROBE_HTTPS), eq(null));



        // Change configuration back to working again, but make private DNS not work.

        // Expect validation to fail.
@@ -930,21 +930,9 @@ public class NetworkMonitorTest { 
        nm.forceReevaluation(Process.myUid());

        final ArgumentCaptor<Integer> intCaptor = ArgumentCaptor.forClass(Integer.class);

        // Expect to send HTTP, HTTPs, FALLBACK and evaluation results.

        verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(4))

            .notifyNetworkTested(intCaptor.capture(), any());

        List<Integer> intArgs = intCaptor.getAllValues();



        // 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);

        verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS))

            .notifyNetworkTested(eq(NETWORK_VALIDATION_PROBE_DNS |

                    NETWORK_VALIDATION_PROBE_FALLBACK | NETWORK_VALIDATION_RESULT_PARTIAL), any());

    }



    @Test
@@ -966,8 +954,6 @@ public class NetworkMonitorTest { 
        // Verify result should be appended and notifyNetworkTested callback is triggered once.

        assertEquals(nm.getEvaluationState().getNetworkTestResult(),

                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);

        // Verify DNS probe result should not be cleared.
@@ -1057,14 +1043,7 @@ public class NetworkMonitorTest { 
    }



    private void runPortalNetworkTest(int result) {

        // The network test event will be triggered twice with the same result. Expect to capture

        // 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);

        runNetworkTest(result);

        assertEquals(1, mRegisteredReceivers.size());

        assertNotNull(mNetworkTestedRedirectUrlCaptor.getValue());

    }
@@ -1101,19 +1080,14 @@ public class NetworkMonitorTest { 
    }



    private NetworkMonitor runNetworkTest(int testResult) {

        return runNetworkTest(METERED_CAPABILITIES, testResult, getGeneralVerification());

        return runNetworkTest(METERED_CAPABILITIES, testResult);

    }



    private NetworkMonitor runNetworkTest(int testResult, VerificationWithTimeout mode) {

        return runNetworkTest(METERED_CAPABILITIES, testResult, mode);

    }



    private NetworkMonitor runNetworkTest(NetworkCapabilities nc, int testResult,

            VerificationWithTimeout mode) {

    private NetworkMonitor runNetworkTest(NetworkCapabilities nc, int testResult) {

        final NetworkMonitor monitor = makeMonitor(nc);

        monitor.notifyNetworkConnected(TEST_LINK_PROPERTIES, nc);

        try {

            verify(mCallbacks, mode)

            verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS))

                    .notifyNetworkTested(eq(testResult), mNetworkTestedRedirectUrlCaptor.capture());

        } catch (RemoteException e) {

            fail("Unexpected exception: " + e);
@@ -1146,9 +1120,5 @@ public class NetworkMonitorTest { 
        }

    }



    private VerificationWithTimeout getGeneralVerification() {

        return (VerificationWithTimeout) timeout(HANDLER_TIMEOUT_MS).atLeastOnce();

    }



}