diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 0733607c26a49..7e52f39440e2a 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -12000,25 +12000,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 872da620e649a..0acf2ae3a8567 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",