From 0f2b48d1d694bd69cee61da134a938faabfa5c3b Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Wed, 13 Jul 2022 15:56:20 +0100 Subject: [PATCH] Add support for multiple NTP servers Add support for multiple NTP servers in NtpTrustedTime. The algorithm to select the server is documented in comments in that class. Tests have been updated accordingly. This commit changes config.xml, removing (string) config_ntpServer and replacing it with (string-array) config_ntpServers. It also modifies command line and settings behavior. Bug: 223365217 Test: atest frameworks/base/core/tests/coretests/src/android/util/NtpTrustedTimeTest.java Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/timedetector/ Test: atest cts/tests/tests/os/src/android/os/cts/SystemClockSntpTest.java Test: adb shell settings put global ntp_server "ntp://time.google.com\|ntp://time.android.com" Test: Inspection: adb shell dumpsys network_time_update_service Test: Inspection: adb shell dumpsys time_detector Change-Id: I5e19a4d82c9771448940b9f1db1689f21508667b --- core/java/android/provider/Settings.java | 27 +- core/java/android/util/NtpTrustedTime.java | 201 +++++-- core/res/res/values/config.xml | 28 +- core/res/res/values/symbols.xml | 2 +- .../src/android/util/NtpTrustedTimeTest.java | 538 +++++++++++++----- .../NetworkTimeUpdateServiceShellCommand.java | 17 +- 6 files changed, 609 insertions(+), 204 deletions(-) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index a295164f711e5..8745f7a100dfd 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -11999,25 +11999,40 @@ public final class Settings { "nitz_network_disconnect_retention"; /** - * The preferred NTP server. This setting overrides Android's static xml configuration when - * present and valid. + * SNTP client config: The preferred NTP server. This setting overrides the static + * config.xml configuration when present and valid. * *

The legacy form is the NTP server name as a string. *

Newer code should use the form: ntp://{server name}[:port] (the standard NTP port, * 123, is used if not specified). * + *

The settings value can consist of a pipe ("|") delimited list of server names or + * ntp:// URIs. When present, all server name / ntp:// URIs must be valid or the entire + * setting value will be ignored and Android's xml config will be used. + * *

For example, the following examples are valid: *

* * @hide */ @Readable public static final String NTP_SERVER = "ntp_server"; - /** Timeout in milliseconds to wait for NTP server. {@hide} */ + + /** + * SNTP client config: Timeout to wait for an NTP server response. This setting overrides + * the static config.xml configuration when present and valid. + * + *

The value is the timeout in milliseconds. It must be > 0. + * + * @hide + */ @Readable public static final String NTP_TIMEOUT = "ntp_timeout"; diff --git a/core/java/android/util/NtpTrustedTime.java b/core/java/android/util/NtpTrustedTime.java index 12e33f56b3c63..f1fd324b42296 100644 --- a/core/java/android/util/NtpTrustedTime.java +++ b/core/java/android/util/NtpTrustedTime.java @@ -40,6 +40,9 @@ import java.net.URI; import java.net.URISyntaxException; import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Objects; import java.util.function.Supplier; @@ -54,6 +57,9 @@ import java.util.function.Supplier; public abstract class NtpTrustedTime implements TrustedTime { private static final String URI_SCHEME_NTP = "ntp"; + @VisibleForTesting + public static final String NTP_SETTING_SERVER_NAME_DELIMITER = "|"; + private static final String NTP_SETTING_SERVER_NAME_DELIMITER_REGEXP = "\\|"; /** * NTP server configuration. @@ -62,31 +68,47 @@ public abstract class NtpTrustedTime implements TrustedTime { */ public static final class NtpConfig { - @NonNull private final URI mServerUri; + @NonNull private final List mServerUris; @NonNull private final Duration mTimeout; /** - * Creates an instance. If the arguments are invalid then an {@link - * IllegalArgumentException} will be thrown. See {@link #parseNtpUriStrict(String)} and - * {@link #parseNtpServerSetting(String)} to create valid URIs. + * Creates an instance with the supplied properties. There must be at least one NTP server + * URI and the timeout must be non-zero / non-negative. + * + *

If the arguments are invalid then an {@link IllegalArgumentException} will be thrown. + * See {@link #parseNtpUriStrict(String)} and {@link #parseNtpServerSetting(String)} to + * create valid URIs. */ - public NtpConfig(@NonNull URI serverUri, @NonNull Duration timeout) + public NtpConfig(@NonNull List serverUris, @NonNull Duration timeout) throws IllegalArgumentException { - try { - mServerUri = validateNtpServerUri(Objects.requireNonNull(serverUri)); - } catch (URISyntaxException e) { - throw new IllegalArgumentException("Bad URI", e); + + Objects.requireNonNull(serverUris); + if (serverUris.isEmpty()) { + throw new IllegalArgumentException("Server URIs is empty"); } + List validatedServerUris = new ArrayList<>(); + for (URI serverUri : serverUris) { + try { + URI validatedServerUri = validateNtpServerUri( + Objects.requireNonNull(serverUri)); + validatedServerUris.add(validatedServerUri); + } catch (URISyntaxException e) { + throw new IllegalArgumentException("Bad server URI", e); + } + } + mServerUris = Collections.unmodifiableList(validatedServerUris); + if (timeout.isNegative() || timeout.isZero()) { throw new IllegalArgumentException("timeout < 0"); } mTimeout = timeout; } + /** Returns a non-empty, immutable list of NTP server URIs. */ @NonNull - public URI getServerUri() { - return mServerUri; + public List getServerUris() { + return mServerUris; } @NonNull @@ -97,7 +119,7 @@ public abstract class NtpTrustedTime implements TrustedTime { @Override public String toString() { return "NtpConnectionInfo{" - + "mServerUri=" + mServerUri + + "mServerUris=" + mServerUris + ", mTimeout=" + mTimeout + '}'; } @@ -199,6 +221,10 @@ public abstract class NtpTrustedTime implements TrustedTime { @Nullable private NtpConfig mNtpConfigForTests; + @GuardedBy("this") + @Nullable + private URI mLastSuccessfulNtpServerUri; + // Declared volatile and accessed outside synchronized blocks to avoid blocking reads during // forceRefresh(). private volatile TimeResult mTimeResult; @@ -245,13 +271,59 @@ public abstract class NtpTrustedTime implements TrustedTime { Log.d(TAG, "forceRefresh: NTP request network=" + network + " ntpConfig=" + ntpConfig); } - TimeResult timeResult = - queryNtpServer(network, ntpConfig.getServerUri(), ntpConfig.getTimeout()); - if (timeResult != null) { - // Keep any previous time result. - mTimeResult = timeResult; + + List unorderedServerUris = ntpConfig.getServerUris(); + + // Android supports multiple NTP server URIs for situations where servers might be + // unreachable for some devices due to network topology, e.g. we understand that devices + // travelling to China often have difficulty accessing "time.android.com". Android + // partners may want to configure alternative URIs for devices sold globally, or those + // that are likely to travel to part of the world without access to the full internet. + // + // The server URI list is expected to contain one element in the general case, with two + // or three as the anticipated maximum. The list is never empty. Server URIs are + // considered to be in a rough priority order of servers to try initially (no + // randomization), but besides that there is assumed to be no preference. + // + // The server selection algorithm below tries to stick with a successfully accessed NTP + // server's URI where possible: + // + // The algorithm based on the assumption that a cluster of NTP servers sharing the same + // host name, particularly commercially run ones, are likely to agree more closely on + // the time than servers from different URIs, so it's best to be sticky. Switching + // between URIs could result in flip-flopping between reference clocks or involve + // talking to server clusters with different approaches to leap second handling. + // + // Stickiness may also be useful if some server URIs early in the list are permanently + // black-holing requests, or if the responses are not routed back. In those cases it's + // best not to try those URIs more than we have to, as might happen if the algorithm + // always started at the beginning of the list. + // + // Generally, we have to assume that any of the configured servers are going to be "good + // enough" as an external reference clock when reachable, so the stickiness is a very + // lightly applied bias. There's no tracking of failure rates or back-off on a per-URI + // basis; higher level code is expected to handle rate limiting of NTP requests in the + // event of failure to contact any server. + + List orderedServerUris = new ArrayList<>(); + for (URI serverUri : unorderedServerUris) { + if (serverUri.equals(mLastSuccessfulNtpServerUri)) { + orderedServerUris.add(0, serverUri); + } else { + orderedServerUris.add(serverUri); + } } - return timeResult != null; + + for (URI serverUri : orderedServerUris) { + TimeResult timeResult = queryNtpServer(network, serverUri, ntpConfig.getTimeout()); + // Only overwrite previous state if the request was successful. + if (timeResult != null) { + mLastSuccessfulNtpServerUri = serverUri; + mTimeResult = timeResult; + return true; + } + } + return false; } } @@ -388,7 +460,9 @@ public abstract class NtpTrustedTime implements TrustedTime { *

NTP server config URIs are in the form "ntp://{hostname}[:port]". This is not a registered * IANA URI scheme. */ - public static URI parseNtpUriStrict(String ntpServerUriString) throws URISyntaxException { + @NonNull + public static URI parseNtpUriStrict(@NonNull String ntpServerUriString) + throws URISyntaxException { // java.net.URI is used in preference to android.net.Uri, since android.net.Uri is very // forgiving of obvious errors. URI catches issues sooner. URI unvalidatedUri = new URI(ntpServerUriString); @@ -396,41 +470,58 @@ public abstract class NtpTrustedTime implements TrustedTime { } /** - * Parses a setting string and returns a URI that will be accepted by {@link NtpConfig}, or - * {@code null} if the string does not produce a URI considered valid. + * Parses a setting string and returns a list of URIs that will be accepted by {@link + * NtpConfig}, or {@code null} if the string is invalid. + * + *

The setting string is expected to be one or more server values separated by a pipe ("|") + * character. * *

NTP server config URIs are in the form "ntp://{hostname}[:port]". This is not a registered * IANA URI scheme. * *

Unlike {@link #parseNtpUriStrict(String)} this method will not throw an exception. It - * checks for a leading "ntp:" and will call through to {@link #parseNtpUriStrict(String)} to - * attempt to parse it, returning {@code null} if it fails. To support legacy settings values, - * it will also accept a string that only consists of a server name, which will be coerced into - * a URI in the form "ntp://{server name}". + * checks each value for a leading "ntp:" and will call through to {@link + * #parseNtpUriStrict(String)} to attempt to parse it, returning {@code null} if it fails. + * To support legacy settings values, it will also accept string values that only consists of a + * server name, which will be coerced into a URI in the form "ntp://{server name}". */ @VisibleForTesting - public static URI parseNtpServerSetting(String ntpServerSetting) { + @Nullable + public static List parseNtpServerSetting(@Nullable String ntpServerSetting) { if (TextUtils.isEmpty(ntpServerSetting)) { return null; - } else if (ntpServerSetting.startsWith(URI_SCHEME_NTP + ":")) { - try { - return parseNtpUriStrict(ntpServerSetting); - } catch (URISyntaxException e) { - Log.w(TAG, "Rejected NTP uri setting=" + ntpServerSetting, e); - return null; - } } else { - // This is the legacy settings path. Assumes that the string is just a host name and - // creates a URI in the form ntp:// - try { - URI uri = new URI(URI_SCHEME_NTP, /*host=*/ntpServerSetting, - /*path=*/null, /*fragment=*/null); - // Paranoia: validate just in case the host name somehow results in a bad URI. - return validateNtpServerUri(uri); - } catch (URISyntaxException e) { - Log.w(TAG, "Rejected NTP legacy setting=" + ntpServerSetting, e); + String[] values = ntpServerSetting.split(NTP_SETTING_SERVER_NAME_DELIMITER_REGEXP); + if (values.length == 0) { return null; } + + List uris = new ArrayList<>(); + for (String value : values) { + if (value.startsWith(URI_SCHEME_NTP + ":")) { + try { + uris.add(parseNtpUriStrict(value)); + } catch (URISyntaxException e) { + Log.w(TAG, "Rejected NTP uri setting=" + ntpServerSetting, e); + return null; + } + } else { + // This is the legacy settings path. Assumes that the string is just a host name + // and creates a URI in the form ntp:// + try { + URI uri = new URI(URI_SCHEME_NTP, /*host=*/value, + /*path=*/null, /*fragment=*/null); + // Paranoia: validate just in case the host name somehow results in a bad + // URI. + URI validatedUri = validateNtpServerUri(uri); + uris.add(validatedUri); + } catch (URISyntaxException e) { + Log.w(TAG, "Rejected NTP legacy setting=" + ntpServerSetting, e); + return null; + } + } + } + return uris; } } @@ -439,7 +530,8 @@ public abstract class NtpTrustedTime implements TrustedTime { * This method currently ignores Uri components that are not used, only checking the parts that * must be present. Returns the supplied {@code uri} if validation is successful. */ - private static URI validateNtpServerUri(URI uri) throws URISyntaxException { + @NonNull + private static URI validateNtpServerUri(@NonNull URI uri) throws URISyntaxException { if (!uri.isAbsolute()) { throw new URISyntaxException(uri.toString(), "Relative URI not supported"); } @@ -457,6 +549,8 @@ public abstract class NtpTrustedTime implements TrustedTime { public void dump(PrintWriter pw) { synchronized (this) { pw.println("getNtpConfig()=" + getNtpConfig()); + pw.println("mNtpConfigForTests=" + mNtpConfigForTests); + pw.println("mLastSuccessfulNtpServerUri=" + mLastSuccessfulNtpServerUri); pw.println("mTimeResult=" + mTimeResult); if (mTimeResult != null) { pw.println("mTimeResult.getAgeMillis()=" @@ -508,17 +602,22 @@ public abstract class NtpTrustedTime implements TrustedTime { // The Settings value has priority over static config. Check settings first. final String serverGlobalSetting = Settings.Global.getString(resolver, Settings.Global.NTP_SERVER); - final URI settingsServerInfo = parseNtpServerSetting(serverGlobalSetting); + final List settingsServerUris = parseNtpServerSetting(serverGlobalSetting); - URI ntpServerUri; - if (settingsServerInfo != null) { - ntpServerUri = settingsServerInfo; + List ntpServerUris; + if (settingsServerUris != null) { + ntpServerUris = settingsServerUris; } else { - String configValue = res.getString(com.android.internal.R.string.config_ntpServer); + String[] configValues = + res.getStringArray(com.android.internal.R.array.config_ntpServers); try { - ntpServerUri = parseNtpUriStrict(configValue); + List configServerUris = new ArrayList<>(); + for (String configValue : configValues) { + configServerUris.add(parseNtpUriStrict(configValue)); + } + ntpServerUris = configServerUris; } catch (URISyntaxException e) { - ntpServerUri = null; + ntpServerUris = null; } } @@ -526,7 +625,7 @@ public abstract class NtpTrustedTime implements TrustedTime { res.getInteger(com.android.internal.R.integer.config_ntpTimeout); final Duration timeout = Duration.ofMillis(Settings.Global.getInt( resolver, Settings.Global.NTP_TIMEOUT, defaultTimeoutMillis)); - return ntpServerUri == null ? null : new NtpConfig(ntpServerUri, timeout); + return ntpServerUris == null ? null : new NtpConfig(ntpServerUris, timeout); } @Override diff --git a/core/res/res/values/config.xml b/core/res/res/values/config.xml index 622414e3e381d..3daf0331c813d 100644 --- a/core/res/res/values/config.xml +++ b/core/res/res/values/config.xml @@ -2308,20 +2308,26 @@ it should be disabled in that locale's resources. --> true - - ntp://time.android.com - - 64800000 - - 60000 - - 3 - + + ntp://time.android.com + + 5000 + + 64800000 + + 60000 + + 3 + 2048 diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index b7c3839318f8e..e55496d61762b 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -697,7 +697,7 @@ - + diff --git a/core/tests/coretests/src/android/util/NtpTrustedTimeTest.java b/core/tests/coretests/src/android/util/NtpTrustedTimeTest.java index 67a4f44a9863f..40bffb898e876 100644 --- a/core/tests/coretests/src/android/util/NtpTrustedTimeTest.java +++ b/core/tests/coretests/src/android/util/NtpTrustedTimeTest.java @@ -21,14 +21,15 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static java.lang.String.join; +import static java.util.Arrays.asList; + import android.net.Network; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -36,6 +37,8 @@ import androidx.test.filters.SmallTest; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.InOrder; +import org.mockito.Mockito; import java.net.InetSocketAddress; import java.net.URI; @@ -43,14 +46,17 @@ import java.net.URISyntaxException; import java.time.Duration; import java.util.Arrays; import java.util.List; -import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; @SmallTest @RunWith(AndroidJUnit4.class) public class NtpTrustedTimeTest { + private static final Duration VALID_TIMEOUT = Duration.ofSeconds(5); + // Valid absolute URIs, but not ones that will be accepted as NTP server URIs. - private static final List BAD_ABSOLUTE_NTP_URIS = Arrays.asList( + private static final List BAD_ABSOLUTE_NTP_URIS = asList( "ntp://:123/", "ntp://:123", "ntp://:/", @@ -64,25 +70,26 @@ public class NtpTrustedTimeTest { ); // Valid relative URIs, but not ones that will be accepted as NTP server URIs. - private static final List BAD_RELATIVE_NTP_URIS = Arrays.asList( + private static final List BAD_RELATIVE_NTP_URIS = asList( "foobar", "/foobar", "foobar:456" ); - // Valid NTP server URIs: input value -> expected URI.toString() value. - private static final Map GOOD_NTP_URIS = Map.of( - "ntp://foobar", "ntp://foobar", - "ntp://foobar/", "ntp://foobar/", - "ntp://foobar:456/", "ntp://foobar:456/", - "ntp://foobar:456", "ntp://foobar:456" + // Valid NTP server URIs: A pair of {input value} -> {expected URI.toString()} value. + private static final List> GOOD_NTP_URIS = asList( + identityPair("ntp://foobar"), + identityPair("ntp://foobar/"), + identityPair("ntp://foobar:456/"), + identityPair("ntp://foobar:456") ); - private static final URI VALID_SERVER_URI = URI.create("ntp://foobar/"); + private static final List VALID_SERVER_URIS = createUris("ntp://foobar/"); @Test public void testParseNtpServerSetting() { - assertEquals(URI.create("ntp://foobar"), NtpTrustedTime.parseNtpServerSetting("foobar")); + // Valid legacy settings single value. + assertEquals(createUris("ntp://foobar"), NtpTrustedTime.parseNtpServerSetting("foobar")); // Legacy settings values which could easily be confused with relative URIs. Parsing of this // legacy form doesn't have to be robust / treated as errors: Android has never supported @@ -96,12 +103,51 @@ public class NtpTrustedTimeTest { NtpTrustedTime.parseNtpServerSetting(badNtpUri)); } - // Valid URIs - for (Map.Entry goodNtpUri : GOOD_NTP_URIS.entrySet()) { - URI uri = NtpTrustedTime.parseNtpServerSetting(goodNtpUri.getKey()); - assertNotNull(goodNtpUri.getKey(), uri); - assertEquals(goodNtpUri.getValue(), uri.toString()); + // Valid single NTP URIs. + for (Pair goodNtpUri : GOOD_NTP_URIS) { + List uris = NtpTrustedTime.parseNtpServerSetting(goodNtpUri.first); + assertNotNull(uris); + assertEquals(1, uris.size()); + URI actualUri = uris.get(0); + assertNotNull(goodNtpUri.first, actualUri); + assertEquals(goodNtpUri.second, actualUri.toString()); } + + // Valid multi-server name value. Not historically supported, but it's easier to support + // this than rule it out. + final String multiSettingDelimiter = NtpTrustedTime.NTP_SETTING_SERVER_NAME_DELIMITER; + String validMultiServerNameSetting = join(multiSettingDelimiter, "foobar", "barbaz"); + List expectedMultiServerNameUris = createUris("ntp://foobar", "ntp://barbaz"); + assertParseNtpServerSettingResult(expectedMultiServerNameUris, validMultiServerNameSetting); + + // Any invalid values should result in a null return value. + String invalidServerName = "foobar:123"; + assertNull(NtpTrustedTime.parseNtpServerSetting( + join(multiSettingDelimiter, validMultiServerNameSetting, invalidServerName))); + assertNull(NtpTrustedTime.parseNtpServerSetting( + join(multiSettingDelimiter, invalidServerName, validMultiServerNameSetting))); + + // Valid multi-NTP URL string. + Pair goodNtpUri1 = GOOD_NTP_URIS.get(0); + Pair goodNtpUri2 = GOOD_NTP_URIS.get(1); + String validMultiNtpUriSetting = + join(multiSettingDelimiter, goodNtpUri1.first, goodNtpUri2.first); + List expectedMultiNtpUris = createUris(goodNtpUri1.second, goodNtpUri2.second); + assertParseNtpServerSettingResult(expectedMultiNtpUris, validMultiNtpUriSetting); + + // Valid string containing both old and new settings forms. + String validCombinedMultiNtpSetting = + join(multiSettingDelimiter, validMultiNtpUriSetting, validMultiServerNameSetting); + List expectedCombinedNtpUris = + Stream.concat(expectedMultiNtpUris.stream(), expectedMultiServerNameUris.stream()) + .collect(Collectors.toList()); + assertParseNtpServerSettingResult(expectedCombinedNtpUris, validCombinedMultiNtpSetting); + } + + private static void assertParseNtpServerSettingResult( + List expectedUris, String settingsString) { + assertEquals("Input: " + settingsString, expectedUris, + NtpTrustedTime.parseNtpServerSetting(settingsString)); } @Test @@ -119,10 +165,10 @@ public class NtpTrustedTimeTest { assertParseNtpUriStrictThrows("notntp://foobar:123"); // Valid NTP URIs - for (Map.Entry goodNtpUri : GOOD_NTP_URIS.entrySet()) { - URI uri = NtpTrustedTime.parseNtpUriStrict(goodNtpUri.getKey()); - assertNotNull(goodNtpUri.getKey(), uri); - assertEquals(goodNtpUri.getValue(), uri.toString()); + for (Pair goodNtpUri : GOOD_NTP_URIS) { + URI uri = NtpTrustedTime.parseNtpUriStrict(goodNtpUri.first); + assertNotNull(goodNtpUri.first, uri); + assertEquals(goodNtpUri.second, uri.toString()); } } @@ -138,17 +184,17 @@ public class NtpTrustedTimeTest { @Test(expected = NullPointerException.class) public void testNtpConfig_nullConstructorTimeout() { - new NtpTrustedTime.NtpConfig(VALID_SERVER_URI, null); + new NtpTrustedTime.NtpConfig(VALID_SERVER_URIS, null); } @Test(expected = IllegalArgumentException.class) public void testNtpConfig_zeroTimeout() { - new NtpTrustedTime.NtpConfig(VALID_SERVER_URI, Duration.ofMillis(0)); + new NtpTrustedTime.NtpConfig(VALID_SERVER_URIS, Duration.ofMillis(0)); } @Test(expected = IllegalArgumentException.class) public void testNtpConfig_negativeTimeout() { - new NtpTrustedTime.NtpConfig(VALID_SERVER_URI, Duration.ofMillis(-1)); + new NtpTrustedTime.NtpConfig(VALID_SERVER_URIS, Duration.ofMillis(-1)); } @Test @@ -158,150 +204,386 @@ public class NtpTrustedTimeTest { assertFalse(ntpTrustedTime.forceRefresh()); - assertFalse(ntpTrustedTime.hasCache()); - assertEquals(0, ntpTrustedTime.getCachedNtpTime()); - assertEquals(0, ntpTrustedTime.getCachedNtpTimeReference()); - assertEquals(Long.MAX_VALUE, ntpTrustedTime.getCacheAge()); - assertNull(ntpTrustedTime.getCachedTimeResult()); + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verifyNoMoreInteractions(); - verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); - verify(ntpTrustedTime, never()).getNetwork(); - verify(ntpTrustedTime, never()).queryNtpServer(any(), any(), any()); + assertNoCachedTimeValueResult(ntpTrustedTime); } @Test public void testForceRefresh_noConnectivity() { NtpTrustedTime ntpTrustedTime = spy(NtpTrustedTime.class); - URI serverUri = URI.create("ntp://ntpserver.name"); - Duration timeout = Duration.ofSeconds(5); + List serverUris = createUris("ntp://ntpserver.name"); when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( - new NtpTrustedTime.NtpConfig(serverUri, timeout)); + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); when(ntpTrustedTime.getNetwork()).thenReturn(null); assertFalse(ntpTrustedTime.forceRefresh()); - assertFalse(ntpTrustedTime.hasCache()); - assertEquals(0, ntpTrustedTime.getCachedNtpTime()); - assertEquals(0, ntpTrustedTime.getCachedNtpTimeReference()); - assertEquals(Long.MAX_VALUE, ntpTrustedTime.getCacheAge()); - assertNull(ntpTrustedTime.getCachedTimeResult()); + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verifyNoMoreInteractions(); - verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); - verify(ntpTrustedTime, times(1)).getNetwork(); - verify(ntpTrustedTime, never()).queryNtpServer(any(), any(), any()); + assertNoCachedTimeValueResult(ntpTrustedTime); } @Test - public void testForceRefresh_queryFailed() { + public void testForceRefresh_singleServer_queryFailed() { NtpTrustedTime ntpTrustedTime = spy(NtpTrustedTime.class); - URI serverUri = URI.create("ntp://ntpserver.name"); - Duration timeout = Duration.ofSeconds(5); + List serverUris = createUris("ntp://ntpserver.name"); when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( - new NtpTrustedTime.NtpConfig(serverUri, timeout)); + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); Network network = mock(Network.class); when(ntpTrustedTime.getNetwork()).thenReturn(network); - when(ntpTrustedTime.queryNtpServer(network, serverUri, timeout)).thenReturn(null); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT)) + .thenReturn(null); assertFalse(ntpTrustedTime.forceRefresh()); - assertFalse(ntpTrustedTime.hasCache()); - assertEquals(0, ntpTrustedTime.getCachedNtpTime()); - assertEquals(0, ntpTrustedTime.getCachedNtpTimeReference()); - assertEquals(Long.MAX_VALUE, ntpTrustedTime.getCacheAge()); - assertNull(ntpTrustedTime.getCachedTimeResult()); + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT); + inOrder.verifyNoMoreInteractions(); - verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); - verify(ntpTrustedTime, times(1)).getNetwork(); - verify(ntpTrustedTime, times(1)).queryNtpServer(network, serverUri, timeout); + assertNoCachedTimeValueResult(ntpTrustedTime); } @Test - public void testForceRefresh_querySucceeded() { + public void testForceRefresh_singleServer_querySucceeded() { NtpTrustedTime ntpTrustedTime = spy(NtpTrustedTime.class); - URI serverUri = URI.create("ntp://ntpserver.name"); - Duration timeout = Duration.ofSeconds(5); + List serverUris = createUris("ntp://ntpserver.name"); when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( - new NtpTrustedTime.NtpConfig(serverUri, timeout)); + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); Network network = mock(Network.class); when(ntpTrustedTime.getNetwork()).thenReturn(network); NtpTrustedTime.TimeResult successResult = new NtpTrustedTime.TimeResult(123L, 456L, 789, InetSocketAddress.createUnresolved("placeholder", 123)); - when(ntpTrustedTime.queryNtpServer(network, serverUri, timeout)).thenReturn(successResult); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT)) + .thenReturn(successResult); assertTrue(ntpTrustedTime.forceRefresh()); + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT); + inOrder.verifyNoMoreInteractions(); + + assertCachedTimeValueResult(ntpTrustedTime, successResult); + } + + @Test + public void testForceRefresh_multiServer_firstQueryFailed() { + NtpTrustedTime ntpTrustedTime = spy(NtpTrustedTime.class); + List serverUris = createUris("ntp://ntpserver1.name", "ntp://ntpserver2.name"); + when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); + + Network network = mock(Network.class); + when(ntpTrustedTime.getNetwork()).thenReturn(network); + + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT)) + .thenReturn(null); + NtpTrustedTime.TimeResult successResult = new NtpTrustedTime.TimeResult(123L, 456L, 789, + InetSocketAddress.createUnresolved("placeholder", 123)); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT)) + .thenReturn(successResult); + + assertTrue(ntpTrustedTime.forceRefresh()); + + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT); + inOrder.verifyNoMoreInteractions(); + + assertCachedTimeValueResult(ntpTrustedTime, successResult); + } + + @Test + public void testForceRefresh_multiServer_firstQuerySucceeded() { + NtpTrustedTime ntpTrustedTime = spy(NtpTrustedTime.class); + List serverUris = createUris("ntp://ntpserver1.name", "ntp://ntpserver2.name"); + when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); + + Network network = mock(Network.class); + when(ntpTrustedTime.getNetwork()).thenReturn(network); + + NtpTrustedTime.TimeResult successResult = new NtpTrustedTime.TimeResult(123L, 456L, 789, + InetSocketAddress.createUnresolved("placeholder", 123)); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT)) + .thenReturn(successResult); + + assertTrue(ntpTrustedTime.forceRefresh()); + + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT); + inOrder.verifyNoMoreInteractions(); + + assertCachedTimeValueResult(ntpTrustedTime, successResult); + } + + @Test + public void testForceRefresh_multiServer_keepsOldValueOnFailure() { + NtpTrustedTime ntpTrustedTime = spy(NtpTrustedTime.class); + List serverUris = createUris("ntp://ntpserver1.name", "ntp://ntpserver2.name"); + Network network = mock(Network.class); + + NtpTrustedTime.TimeResult successResult = new NtpTrustedTime.TimeResult(123L, 456L, 789, + InetSocketAddress.createUnresolved("placeholder", 123)); + + // First forceRefresh() succeeds. + { + when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); + when(ntpTrustedTime.getNetwork()).thenReturn(network); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT)) + .thenReturn(successResult); + + assertTrue(ntpTrustedTime.forceRefresh()); + + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT); + inOrder.verifyNoMoreInteractions(); + + assertCachedTimeValueResult(ntpTrustedTime, successResult); + + reset(ntpTrustedTime); + } + + // Next forceRefresh() fails, keeping the result of the first forceRefresh(). + { + when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); + when(ntpTrustedTime.getNetwork()).thenReturn(network); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(0), + VALID_TIMEOUT)).thenReturn(null); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(1), + VALID_TIMEOUT)).thenReturn(null); + + assertFalse(ntpTrustedTime.forceRefresh()); + + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT); + inOrder.verifyNoMoreInteractions(); + + assertCachedTimeValueResult(ntpTrustedTime, successResult); + + reset(ntpTrustedTime); + } + } + + /** + * A complex test demonstrating the properties of the multi-server algorithm in different + * scenarios / states. + */ + @Test + public void testForceRefresh_multiServer_complex() { + NtpTrustedTime ntpTrustedTime = spy(NtpTrustedTime.class); + List serverUris = createUris( + "ntp://ntpserver1.name", "ntp://ntpserver2.name", "ntp://ntpserver3.name"); + Network network = mock(Network.class); + + NtpTrustedTime.TimeResult successResult1 = new NtpTrustedTime.TimeResult(111L, 111L, 111, + InetSocketAddress.createUnresolved("placeholder", 111)); + NtpTrustedTime.TimeResult successResult2 = new NtpTrustedTime.TimeResult(222L, 222L, 222, + InetSocketAddress.createUnresolved("placeholder", 222)); + NtpTrustedTime.TimeResult successResult3 = new NtpTrustedTime.TimeResult(333L, 333L, 333, + InetSocketAddress.createUnresolved("placeholder", 333)); + + // The first forceRefresh() should the URIs in the original order. Here, we fail the first + // and succeed with the second. + { + when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); + when(ntpTrustedTime.getNetwork()).thenReturn(network); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT)) + .thenReturn(null); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT)) + .thenReturn(successResult1); + + assertTrue(ntpTrustedTime.forceRefresh()); + + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT); + inOrder.verifyNoMoreInteractions(); + + assertCachedTimeValueResult(ntpTrustedTime, successResult1); + + reset(ntpTrustedTime); + } + + // forceRefresh() should try starting with the last successful server, which will succeed. + { + when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); + when(ntpTrustedTime.getNetwork()).thenReturn(network); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT)) + .thenReturn(successResult2); + + assertTrue(ntpTrustedTime.forceRefresh()); + + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT); + inOrder.verifyNoMoreInteractions(); + + assertCachedTimeValueResult(ntpTrustedTime, successResult2); + + reset(ntpTrustedTime); + } + + // forceRefresh() should try starting with the last successful server, but try the others in + // order. It will succeed with the final server. + { + when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); + when(ntpTrustedTime.getNetwork()).thenReturn(network); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT)) + .thenReturn(null); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT)) + .thenReturn(null); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(2), VALID_TIMEOUT)) + .thenReturn(successResult3); + + assertTrue(ntpTrustedTime.forceRefresh()); + + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(2), VALID_TIMEOUT); + inOrder.verifyNoMoreInteractions(); + + assertCachedTimeValueResult(ntpTrustedTime, successResult3); + + reset(ntpTrustedTime); + } + + // forceRefresh() should try starting with the last successful server, but try the others in + // order. It will fail with all. + { + when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); + when(ntpTrustedTime.getNetwork()).thenReturn(network); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(2), VALID_TIMEOUT)) + .thenReturn(null); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT)) + .thenReturn(null); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT)) + .thenReturn(null); + + assertFalse(ntpTrustedTime.forceRefresh()); + + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(2), VALID_TIMEOUT); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT); + inOrder.verifyNoMoreInteractions(); + + assertCachedTimeValueResult(ntpTrustedTime, successResult3); + + reset(ntpTrustedTime); + } + + // forceRefresh() should try starting with the last successful server, but try the others in + // order. It will succeed on the last. + { + when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( + new NtpTrustedTime.NtpConfig(serverUris, VALID_TIMEOUT)); + when(ntpTrustedTime.getNetwork()).thenReturn(network); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(2), VALID_TIMEOUT)) + .thenReturn(null); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT)) + .thenReturn(null); + when(ntpTrustedTime.queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT)) + .thenReturn(successResult1); + + assertTrue(ntpTrustedTime.forceRefresh()); + + InOrder inOrder = Mockito.inOrder(ntpTrustedTime); + inOrder.verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); + inOrder.verify(ntpTrustedTime, times(1)).getNetwork(); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(2), VALID_TIMEOUT); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(0), VALID_TIMEOUT); + inOrder.verify(ntpTrustedTime, times(1)) + .queryNtpServer(network, serverUris.get(1), VALID_TIMEOUT); + inOrder.verifyNoMoreInteractions(); + + assertCachedTimeValueResult(ntpTrustedTime, successResult1); + + reset(ntpTrustedTime); + } + } + + private static Pair identityPair(String both) { + return Pair.create(both, both); + } + + private static List createUris(String... uriStrings) { + return Arrays.stream(uriStrings).map(URI::create).collect(Collectors.toList()); + } + + @SuppressWarnings("deprecation") + private static void assertNoCachedTimeValueResult(NtpTrustedTime ntpTrustedTime) { + assertFalse(ntpTrustedTime.hasCache()); + assertEquals(0, ntpTrustedTime.getCachedNtpTime()); + assertEquals(0, ntpTrustedTime.getCachedNtpTimeReference()); + assertEquals(Long.MAX_VALUE, ntpTrustedTime.getCacheAge()); + assertNull(ntpTrustedTime.getCachedTimeResult()); + } + + @SuppressWarnings("deprecation") + private static void assertCachedTimeValueResult(NtpTrustedTime ntpTrustedTime, + NtpTrustedTime.TimeResult expected) { assertTrue(ntpTrustedTime.hasCache()); - assertEquals(successResult.getTimeMillis(), ntpTrustedTime.getCachedNtpTime()); - assertEquals(successResult.getElapsedRealtimeMillis(), + assertEquals(expected.getTimeMillis(), ntpTrustedTime.getCachedNtpTime()); + assertEquals(expected.getElapsedRealtimeMillis(), ntpTrustedTime.getCachedNtpTimeReference()); assertTrue(ntpTrustedTime.getCacheAge() != Long.MAX_VALUE); - assertEquals(successResult, ntpTrustedTime.getCachedTimeResult()); - - verify(ntpTrustedTime, times(1)).getNtpConfigInternal(); - verify(ntpTrustedTime, times(1)).getNetwork(); - verify(ntpTrustedTime, times(1)).queryNtpServer(network, serverUri, timeout); - } - - @Test - public void testForceRefresh_keepsOldValueOnFailure() { - NtpTrustedTime ntpTrustedTime = spy(NtpTrustedTime.class); - URI serverUri = URI.create("ntp://ntpserver.name"); - Duration timeout = Duration.ofSeconds(5); - when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( - new NtpTrustedTime.NtpConfig(serverUri, timeout)); - - Network network = mock(Network.class); - when(ntpTrustedTime.getNetwork()).thenReturn(network); - - NtpTrustedTime.TimeResult successResult = new NtpTrustedTime.TimeResult(123L, 456L, 789, - InetSocketAddress.createUnresolved("placeholder", 123)); - when(ntpTrustedTime.queryNtpServer(network, serverUri, timeout)).thenReturn(successResult); - - assertTrue(ntpTrustedTime.forceRefresh()); - - assertTrue(ntpTrustedTime.hasCache()); - assertEquals(successResult, ntpTrustedTime.getCachedTimeResult()); - - when(ntpTrustedTime.queryNtpServer(network, serverUri, timeout)).thenReturn(null); - - assertFalse(ntpTrustedTime.forceRefresh()); - - assertTrue(ntpTrustedTime.hasCache()); - assertEquals(successResult, ntpTrustedTime.getCachedTimeResult()); - } - - @Test - public void testForceRefresh_keepsNewValueOnSuccess() { - NtpTrustedTime ntpTrustedTime = spy(NtpTrustedTime.class); - URI serverUri = URI.create("ntp://ntpserver.name"); - Duration timeout = Duration.ofSeconds(5); - when(ntpTrustedTime.getNtpConfigInternal()).thenReturn( - new NtpTrustedTime.NtpConfig(serverUri, timeout)); - - Network network = mock(Network.class); - when(ntpTrustedTime.getNetwork()).thenReturn(network); - - NtpTrustedTime.TimeResult successResult1 = new NtpTrustedTime.TimeResult(123L, 456L, 789, - InetSocketAddress.createUnresolved("placeholder", 123)); - when(ntpTrustedTime.queryNtpServer(network, serverUri, timeout)).thenReturn(successResult1); - - assertTrue(ntpTrustedTime.forceRefresh()); - - assertTrue(ntpTrustedTime.hasCache()); - assertEquals(successResult1, ntpTrustedTime.getCachedTimeResult()); - - NtpTrustedTime.TimeResult successResult2 = new NtpTrustedTime.TimeResult(123L, 456L, 789, - InetSocketAddress.createUnresolved("placeholder", 123)); - when(ntpTrustedTime.queryNtpServer(network, serverUri, timeout)).thenReturn(successResult2); - - assertTrue(ntpTrustedTime.forceRefresh()); - - assertTrue(ntpTrustedTime.hasCache()); - assertEquals(successResult2, ntpTrustedTime.getCachedTimeResult()); + assertEquals(expected, ntpTrustedTime.getCachedTimeResult()); } } diff --git a/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java b/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java index c8636b9943af1..410b50f8a88b2 100644 --- a/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java +++ b/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java @@ -24,6 +24,8 @@ import java.io.PrintWriter; import java.net.URI; import java.net.URISyntaxException; import java.time.Duration; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; /** Implements the shell command interface for {@link NetworkTimeUpdateService}. */ @@ -97,14 +99,14 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand { } private int runSetServerConfig() { - URI serverUri = null; + List serverUris = new ArrayList<>(); Duration timeout = null; String opt; while ((opt = getNextArg()) != null) { switch (opt) { case SET_SERVER_CONFIG_SERVER_ARG: { try { - serverUri = NtpTrustedTime.parseNtpUriStrict(getNextArgRequired()); + serverUris.add(NtpTrustedTime.parseNtpUriStrict(getNextArgRequired())); } catch (URISyntaxException e) { throw new IllegalArgumentException("Bad NTP server value", e); } @@ -120,7 +122,7 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand { } } - if (serverUri == null) { + if (serverUris.isEmpty()) { throw new IllegalArgumentException( "Missing required option: --" + SET_SERVER_CONFIG_SERVER_ARG); } @@ -129,7 +131,7 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand { "Missing required option: --" + SET_SERVER_CONFIG_TIMEOUT_ARG); } - NtpTrustedTime.NtpConfig ntpConfig = new NtpTrustedTime.NtpConfig(serverUri, timeout); + NtpTrustedTime.NtpConfig ntpConfig = new NtpTrustedTime.NtpConfig(serverUris, timeout); mNetworkTimeUpdateService.setServerConfigForTests(ntpConfig); return 0; } @@ -151,9 +153,10 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand { pw.printf(" Refreshes the latest time. Prints whether it was successful.\n"); pw.printf(" %s\n", SHELL_COMMAND_SET_SERVER_CONFIG); pw.printf(" Sets the NTP server config for tests. The config is not persisted.\n"); - pw.printf(" Options: %s %s \n", - SET_SERVER_CONFIG_SERVER_ARG, SET_SERVER_CONFIG_TIMEOUT_ARG); - pw.printf(" The URI must be in the form \"ntp://hostname\" or" + pw.printf(" Options: %s [%s ]+ %s \n", + SET_SERVER_CONFIG_SERVER_ARG, SET_SERVER_CONFIG_SERVER_ARG, + SET_SERVER_CONFIG_TIMEOUT_ARG); + pw.printf(" NTP server URIs must be in the form \"ntp://hostname\" or" + " \"ntp://hostname:port\""); pw.printf(" %s\n", SHELL_COMMAND_RESET_SERVER_CONFIG); pw.printf(" Resets/clears the NTP server config set via %s.\n",