Merge "Refactoring before supporting multiple NTP servers"

This commit is contained in:
Neil Fuller
2022-07-12 20:30:59 +00:00
committed by Android (Google) Code Review
7 changed files with 367 additions and 108 deletions

View File

@@ -11998,7 +11998,23 @@ public final class Settings {
public static final String NITZ_NETWORK_DISCONNECT_RETENTION =
"nitz_network_disconnect_retention";
/** Preferred NTP server. {@hide} */
/**
* The preferred NTP server. This setting overrides Android's static xml configuration when
* present and valid.
*
* <p>The legacy form is the NTP server name as a string.
* <p>Newer code should use the form: ntp://{server name}[:port] (the standard NTP port,
* 123, is used if not specified).
*
* <p>For example, the following examples are valid:
* <ul>
* <li>time.android.com</li>
* <li>ntp://time.android.com</li>
* <li>ntp://time.android.com:123</li>
* </ul>
*
* @hide
*/
@Readable
public static final String NTP_SERVER = "ntp_server";
/** Timeout in milliseconds to wait for NTP server. {@hide} */

View File

@@ -32,8 +32,11 @@ import android.provider.Settings;
import android.text.TextUtils;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
import java.io.PrintWriter;
import java.net.URI;
import java.net.URISyntaxException;
import java.time.Duration;
import java.time.Instant;
import java.util.Objects;
@@ -49,6 +52,56 @@ import java.util.function.Supplier;
*/
public class NtpTrustedTime implements TrustedTime {
private static final String URI_SCHEME_NTP = "ntp";
/**
* NTP server configuration.
*
* @hide
*/
public static final class NtpConfig {
@NonNull private final URI mServerUri;
@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.
*/
public NtpConfig(@NonNull URI serverUri, @NonNull Duration timeout)
throws IllegalArgumentException {
try {
mServerUri = validateNtpServerUri(Objects.requireNonNull(serverUri));
} catch (URISyntaxException e) {
throw new IllegalArgumentException("Bad URI", e);
}
if (timeout.isNegative() || timeout.isZero()) {
throw new IllegalArgumentException("timeout < 0");
}
mTimeout = timeout;
}
@NonNull
public URI getServerUri() {
return mServerUri;
}
@NonNull
public Duration getTimeout() {
return mTimeout;
}
@Override
public String toString() {
return "NtpConnectionInfo{"
+ "mServerUri=" + mServerUri
+ ", mTimeout=" + mTimeout
+ '}';
}
}
/**
* The result of a successful NTP query.
*
@@ -139,15 +192,7 @@ public class NtpTrustedTime implements TrustedTime {
/** An in-memory config override for use during tests. */
@Nullable
private String mHostnameForTests;
/** An in-memory config override for use during tests. */
@Nullable
private Integer mPortForTests;
/** An in-memory config override for use during tests. */
@Nullable
private Duration mTimeoutForTests;
private NtpConfig mNtpConfigForTests;
// Declared volatile and accessed outside of synchronized blocks to avoid blocking reads during
// forceRefresh().
@@ -170,19 +215,16 @@ public class NtpTrustedTime implements TrustedTime {
* Overrides the NTP server config for tests. Passing {@code null} to a parameter clears the
* test value, i.e. so the normal value will be used next time.
*/
public void setServerConfigForTests(
@Nullable String hostname, @Nullable Integer port, @Nullable Duration timeout) {
public void setServerConfigForTests(@NonNull NtpConfig ntpConfig) {
synchronized (this) {
mHostnameForTests = hostname;
mPortForTests = port;
mTimeoutForTests = timeout;
mNtpConfigForTests = ntpConfig;
}
}
@UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
public boolean forceRefresh() {
synchronized (this) {
NtpConnectionInfo connectionInfo = getNtpConnectionInfo();
NtpConfig connectionInfo = getNtpConfig();
if (connectionInfo == null) {
// missing server config, so no NTP time available
if (LOGD) Log.d(TAG, "forceRefresh: invalid server config");
@@ -213,9 +255,11 @@ public class NtpTrustedTime implements TrustedTime {
if (LOGD) Log.d(TAG, "forceRefresh() from cache miss");
final SntpClient client = new SntpClient();
final String serverName = connectionInfo.getServer();
final int port = connectionInfo.getPort();
final int timeoutMillis = connectionInfo.getTimeoutMillis();
final URI ntpServerUri = connectionInfo.getServerUri();
final String serverName = ntpServerUri.getHost();
final int port = ntpServerUri.getPort() == -1
? SntpClient.STANDARD_NTP_PORT : ntpServerUri.getPort();
final int timeoutMillis = saturatedCast(connectionInfo.getTimeout().toMillis());
if (client.requestTime(serverName, port, timeoutMillis, network)) {
int ntpUncertaintyMillis = saturatedCast(client.getRoundTripTime() / 2);
mTimeResult = new TimeResult(
@@ -327,85 +371,115 @@ public class NtpTrustedTime implements TrustedTime {
}
}
private static class NtpConnectionInfo {
@NonNull private final String mServer;
private final int mPort;
private final int mTimeoutMillis;
NtpConnectionInfo(@NonNull String server, int port, int timeoutMillis) {
mServer = Objects.requireNonNull(server);
mPort = port;
mTimeoutMillis = timeoutMillis;
}
@NonNull
public String getServer() {
return mServer;
}
@NonNull
public int getPort() {
return mPort;
}
int getTimeoutMillis() {
return mTimeoutMillis;
}
@Override
public String toString() {
return "NtpConnectionInfo{"
+ "mServer='" + mServer + '\''
+ ", mPort='" + mPort + '\''
+ ", mTimeoutMillis=" + mTimeoutMillis
+ '}';
}
}
@GuardedBy("this")
private NtpConnectionInfo getNtpConnectionInfo() {
final ContentResolver resolver = mContext.getContentResolver();
private NtpConfig getNtpConfig() {
if (mNtpConfigForTests != null) {
return mNtpConfigForTests;
}
final ContentResolver resolver = mContext.getContentResolver();
final Resources res = mContext.getResources();
final String hostname;
if (mHostnameForTests != null) {
hostname = mHostnameForTests;
// 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);
URI ntpServerUri;
if (settingsServerInfo != null) {
ntpServerUri = settingsServerInfo;
} else {
String serverGlobalSetting =
Settings.Global.getString(resolver, Settings.Global.NTP_SERVER);
if (serverGlobalSetting != null) {
hostname = serverGlobalSetting;
} else {
hostname = res.getString(com.android.internal.R.string.config_ntpServer);
String configValue = res.getString(com.android.internal.R.string.config_ntpServer);
try {
ntpServerUri = parseNtpUriStrict(configValue);
} catch (URISyntaxException e) {
ntpServerUri = null;
}
}
final Integer port;
if (mPortForTests != null) {
port = mPortForTests;
} else {
port = SntpClient.STANDARD_NTP_PORT;
}
final int defaultTimeoutMillis =
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);
}
final int timeoutMillis;
if (mTimeoutForTests != null) {
timeoutMillis = (int) mTimeoutForTests.toMillis();
/**
* Parses and returns an NTP server config URI, or throws an exception if the URI doesn't
* conform to expectations.
*
* <p>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 {
// 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);
return validateNtpServerUri(unvalidatedUri);
}
/**
* 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.
*
* <p>NTP server config URIs are in the form "ntp://{hostname}[:port]". This is not a registered
* IANA URI scheme.
*
* <p>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}".
*/
@VisibleForTesting
public static URI parseNtpServerSetting(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 {
int defaultTimeoutMillis =
res.getInteger(com.android.internal.R.integer.config_ntpTimeout);
timeoutMillis = Settings.Global.getInt(
resolver, Settings.Global.NTP_TIMEOUT, defaultTimeoutMillis);
// This is the legacy settings path. Assumes that the string is just a host name and
// creates a URI in the form ntp://<host name>
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);
return null;
}
}
return TextUtils.isEmpty(hostname) ? null :
new NtpConnectionInfo(hostname, port, timeoutMillis);
}
/**
* Checks that the supplied URI can be used to identify an NTP server.
* 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 {
if (!uri.isAbsolute()) {
throw new URISyntaxException(uri.toString(), "Relative URI not supported");
}
if (!URI_SCHEME_NTP.equals(uri.getScheme())) {
throw new URISyntaxException(uri.toString(), "Unrecognized scheme");
}
String host = uri.getHost();
if (TextUtils.isEmpty(host)) {
throw new URISyntaxException(uri.toString(), "Missing host");
}
return uri;
}
/** Prints debug information. */
public void dump(PrintWriter pw) {
synchronized (this) {
pw.println("getNtpConnectionInfo()=" + getNtpConnectionInfo());
pw.println("getNtpConfig()=" + getNtpConfig());
pw.println("mTimeResult=" + mTimeResult);
if (mTimeResult != null) {
pw.println("mTimeResult.getAgeMillis()="

View File

@@ -2308,8 +2308,10 @@
it should be disabled in that locale's resources. -->
<bool name="config_actionMenuItemAllCaps">true</bool>
<!-- Remote server that can provide NTP responses. -->
<string translatable="false" name="config_ntpServer">time.android.com</string>
<!-- Remote server that can provide NTP responses.
Values must be in the form: "ntp://<host>[:port]"
This is not a registered IANA URI scheme. -->
<string translatable="false" name="config_ntpServer">ntp://time.android.com</string>
<!-- Normal polling frequency in milliseconds -->
<integer name="config_ntpPollingInterval">64800000</integer>
<!-- Try-again polling interval in milliseconds, in case the network request failed -->

View File

@@ -0,0 +1,141 @@
/*
* Copyright (C) 2022 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package android.util;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.SmallTest;
import org.junit.Test;
import org.junit.runner.RunWith;
import java.net.URI;
import java.net.URISyntaxException;
import java.time.Duration;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
@SmallTest
@RunWith(AndroidJUnit4.class)
public class NtpTrustedTimeTest {
// Valid absolute URIs, but not ones that will be accepted as NTP server URIs.
private static final List<String> BAD_ABSOLUTE_NTP_URIS = Arrays.asList(
"ntp://:123/",
"ntp://:123",
"ntp://:/",
"ntp://:",
"ntp://foobar:abc/",
"ntp://foobar:abc",
"ntp://foobar:456:789/",
"ntp://foobar:456:789",
"ntp://foobar:456:abc/",
"ntp://foobar:456:abc"
);
// Valid relative URIs, but not ones that will be accepted as NTP server URIs.
private static final List<String> BAD_RELATIVE_NTP_URIS = Arrays.asList(
"foobar",
"/foobar",
"foobar:456"
);
// Valid NTP server URIs: input value -> expected URI.toString() value.
private static final Map<String, String> 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"
);
private static final URI VALID_SERVER_URI = URI.create("ntp://foobar/");
@Test
public void testParseNtpServerSetting() {
assertEquals(URI.create("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
// string like these, and so they won't work properly.
assertNull(NtpTrustedTime.parseNtpServerSetting("foobar:123"));
assertNull(NtpTrustedTime.parseNtpServerSetting("/foobar"));
// NTP URI cases that must not be accepted.
for (String badNtpUri : BAD_ABSOLUTE_NTP_URIS) {
assertNull("Input: \"" + badNtpUri + "\"",
NtpTrustedTime.parseNtpServerSetting(badNtpUri));
}
// Valid URIs
for (Map.Entry<String, String> goodNtpUri : GOOD_NTP_URIS.entrySet()) {
URI uri = NtpTrustedTime.parseNtpServerSetting(goodNtpUri.getKey());
assertNotNull(goodNtpUri.getKey(), uri);
assertEquals(goodNtpUri.getValue(), uri.toString());
}
}
@Test
public void testParseNtpUriStrict() throws Exception {
// ntp: URI cases that must not be accepted.
for (String badNtpUri : BAD_ABSOLUTE_NTP_URIS) {
assertParseNtpUriStrictThrows(badNtpUri);
}
for (String badNtpUri : BAD_RELATIVE_NTP_URIS) {
assertParseNtpUriStrictThrows(badNtpUri);
}
// Bad scheme.
assertParseNtpUriStrictThrows("notntp://foobar:123");
// Valid NTP URIs
for (Map.Entry<String, String> goodNtpUri : GOOD_NTP_URIS.entrySet()) {
URI uri = NtpTrustedTime.parseNtpUriStrict(goodNtpUri.getKey());
assertNotNull(goodNtpUri.getKey(), uri);
assertEquals(goodNtpUri.getValue(), uri.toString());
}
}
private void assertParseNtpUriStrictThrows(String badNtpUri) throws Exception {
assertThrows("Input: \"" + badNtpUri, URISyntaxException.class,
() -> NtpTrustedTime.parseNtpUriStrict(badNtpUri));
}
@Test(expected = NullPointerException.class)
public void testNtpConfig_nullConstructorServerInfo() {
new NtpTrustedTime.NtpConfig(null, Duration.ofSeconds(5));
}
@Test(expected = NullPointerException.class)
public void testNtpConfig_nullConstructorTimeout() {
new NtpTrustedTime.NtpConfig(VALID_SERVER_URI, null);
}
@Test(expected = IllegalArgumentException.class)
public void testNtpConfig_zeroTimeout() {
new NtpTrustedTime.NtpConfig(VALID_SERVER_URI, Duration.ofMillis(0));
}
@Test(expected = IllegalArgumentException.class)
public void testNtpConfig_negativeTimeout() {
new NtpTrustedTime.NtpConfig(VALID_SERVER_URI, Duration.ofMillis(-1));
}
}

View File

@@ -0,0 +1,2 @@
per-file NtpTrustedTimeTest.java = file:/services/core/java/com/android/server/timezonedetector/OWNERS
per-file TypedValueTest.kt = file:/core/java/android/content/res/OWNERS

View File

@@ -195,15 +195,12 @@ public class NetworkTimeUpdateService extends Binder {
* Overrides the NTP server config for tests. Passing {@code null} to a parameter clears the
* test value, i.e. so the normal value will be used next time.
*/
void setServerConfigForTests(
@Nullable String hostname, @Nullable Integer port, @Nullable Duration timeout) {
void setServerConfigForTests(@Nullable NtpTrustedTime.NtpConfig ntpConfig) {
mContext.enforceCallingPermission(
android.Manifest.permission.SET_TIME, "set NTP server config for tests");
mLocalLog.log("Setting server config for tests: hostname=" + hostname
+ ", port=" + port
+ ", timeout=" + timeout);
mTime.setServerConfigForTests(hostname, port, timeout);
mLocalLog.log("Setting server config for tests: ntpConnectionInfo=" + ntpConfig);
mTime.setServerConfigForTests(ntpConfig);
}
private void onPollNetworkTime(int event) {

View File

@@ -18,8 +18,11 @@ package com.android.server.timedetector;
import android.annotation.NonNull;
import android.os.ShellCommand;
import android.util.NtpTrustedTime;
import java.io.PrintWriter;
import java.net.URI;
import java.net.URISyntaxException;
import java.time.Duration;
import java.util.Objects;
@@ -42,13 +45,18 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand {
private static final String SHELL_COMMAND_FORCE_REFRESH = "force_refresh";
/**
* A shell command that sets the NTP server config for tests. Config is cleared on reboot.
* A shell command that sets the NTP server config for tests. Config is cleared on reboot or
* using {@link #SHELL_COMMAND_RESET_SERVER_CONFIG}.
*/
private static final String SHELL_COMMAND_SET_SERVER_CONFIG = "set_server_config";
private static final String SET_SERVER_CONFIG_HOSTNAME_ARG = "--hostname";
private static final String SET_SERVER_CONFIG_PORT_ARG = "--port";
private static final String SHELL_COMMAND_SET_SERVER_CONFIG = "set_server_config_for_tests";
private static final String SET_SERVER_CONFIG_SERVER_ARG = "--server";
private static final String SET_SERVER_CONFIG_TIMEOUT_ARG = "--timeout_millis";
/**
* A shell command that resets the NTP server config for tests.
*/
private static final String SHELL_COMMAND_RESET_SERVER_CONFIG = "reset_server_config_for_tests";
@NonNull
private final NetworkTimeUpdateService mNetworkTimeUpdateService;
@@ -69,6 +77,8 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand {
return runForceRefresh();
case SHELL_COMMAND_SET_SERVER_CONFIG:
return runSetServerConfig();
case SHELL_COMMAND_RESET_SERVER_CONFIG:
return runResetServerConfig();
default: {
return handleDefaultCommands(cmd);
}
@@ -87,18 +97,17 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand {
}
private int runSetServerConfig() {
String hostname = null;
Integer port = null;
URI serverUri = null;
Duration timeout = null;
String opt;
while ((opt = getNextArg()) != null) {
switch (opt) {
case SET_SERVER_CONFIG_HOSTNAME_ARG: {
hostname = getNextArgRequired();
break;
}
case SET_SERVER_CONFIG_PORT_ARG: {
port = Integer.parseInt(getNextArgRequired());
case SET_SERVER_CONFIG_SERVER_ARG: {
try {
serverUri = NtpTrustedTime.parseNtpUriStrict(getNextArgRequired());
} catch (URISyntaxException e) {
throw new IllegalArgumentException("Bad NTP server value", e);
}
break;
}
case SET_SERVER_CONFIG_TIMEOUT_ARG: {
@@ -110,7 +119,23 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand {
}
}
}
mNetworkTimeUpdateService.setServerConfigForTests(hostname, port, timeout);
if (serverUri == null) {
throw new IllegalArgumentException(
"Missing required option: --" + SET_SERVER_CONFIG_SERVER_ARG);
}
if (timeout == null) {
throw new IllegalArgumentException(
"Missing required option: --" + SET_SERVER_CONFIG_TIMEOUT_ARG);
}
NtpTrustedTime.NtpConfig ntpConfig = new NtpTrustedTime.NtpConfig(serverUri, timeout);
mNetworkTimeUpdateService.setServerConfigForTests(ntpConfig);
return 0;
}
private int runResetServerConfig() {
mNetworkTimeUpdateService.setServerConfigForTests(null);
return 0;
}
@@ -126,11 +151,13 @@ 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 <hostname>] [%s <port>] [%s <millis>]\n",
SET_SERVER_CONFIG_HOSTNAME_ARG, SET_SERVER_CONFIG_PORT_ARG,
SET_SERVER_CONFIG_TIMEOUT_ARG);
pw.printf(" Each key/value is optional and must be specified to override the\n");
pw.printf(" normal value, not specifying a key causes it to reset to the original.\n");
pw.printf(" Options: %s <uri> %s <millis>\n",
SET_SERVER_CONFIG_SERVER_ARG, SET_SERVER_CONFIG_TIMEOUT_ARG);
pw.printf(" The URI 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",
SHELL_COMMAND_SET_SERVER_CONFIG);
pw.println();
}
}