diff --git a/services/core/java/com/android/server/timezonedetector/location/BinderLocationTimeZoneProvider.java b/services/core/java/com/android/server/timezonedetector/location/BinderLocationTimeZoneProvider.java index c0c9e6d58622c..8e4a18243a008 100644 --- a/services/core/java/com/android/server/timezonedetector/location/BinderLocationTimeZoneProvider.java +++ b/services/core/java/com/android/server/timezonedetector/location/BinderLocationTimeZoneProvider.java @@ -47,7 +47,7 @@ class BinderLocationTimeZoneProvider extends LocationTimeZoneProvider { @NonNull ThreadingDomain threadingDomain, @NonNull String providerName, @NonNull LocationTimeZoneProviderProxy proxy) { - super(threadingDomain, providerName); + super(threadingDomain, providerName, new ZoneInfoDbTimeZoneIdValidator()); mProxy = Objects.requireNonNull(proxy); } diff --git a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java index 0d1692a8781d2..588382158bc9f 100644 --- a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java +++ b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java @@ -520,6 +520,12 @@ public class LocationTimeZoneManagerService extends Binder { } } + static void infoLog(String msg) { + if (Log.isLoggable(TAG, Log.INFO)) { + Slog.i(TAG, msg); + } + } + static void warnLog(String msg) { warnLog(msg, null); } diff --git a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProvider.java b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProvider.java index ef2f357b8c3ef..b97c838017eb5 100644 --- a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProvider.java +++ b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProvider.java @@ -20,6 +20,7 @@ import static android.service.timezone.TimeZoneProviderService.TEST_COMMAND_RESU import static android.service.timezone.TimeZoneProviderService.TEST_COMMAND_RESULT_SUCCESS_KEY; import static com.android.server.timezonedetector.location.LocationTimeZoneManagerService.debugLog; +import static com.android.server.timezonedetector.location.LocationTimeZoneManagerService.infoLog; import static com.android.server.timezonedetector.location.LocationTimeZoneManagerService.warnLog; import static com.android.server.timezonedetector.location.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_DESTROYED; import static com.android.server.timezonedetector.location.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_PERM_FAILED; @@ -84,6 +85,18 @@ abstract class LocationTimeZoneProvider implements Dumpable { void onProviderStateChange(@NonNull ProviderState providerState); } + /** + * Used by {@link LocationTimeZoneProvider} to check if time zone IDs are understood + * by the platform. + */ + interface TimeZoneIdValidator { + + /** + * Returns whether {@code timeZoneId} is supported by the platform or not. + */ + boolean isValid(@NonNull String timeZoneId); + } + /** * Information about the provider's current state. */ @@ -364,13 +377,17 @@ abstract class LocationTimeZoneProvider implements Dumpable { // Non-null and effectively final after initialize() is called. ProviderListener mProviderListener; + @NonNull private TimeZoneIdValidator mTimeZoneIdValidator; + /** Creates the instance. */ LocationTimeZoneProvider(@NonNull ThreadingDomain threadingDomain, - @NonNull String providerName) { + @NonNull String providerName, + @NonNull TimeZoneIdValidator timeZoneIdValidator) { mThreadingDomain = Objects.requireNonNull(threadingDomain); mInitializationTimeoutQueue = threadingDomain.createSingleRunnableQueue(); mSharedLock = threadingDomain.getLockObject(); mProviderName = Objects.requireNonNull(providerName); + mTimeZoneIdValidator = Objects.requireNonNull(timeZoneIdValidator); } /** @@ -610,6 +627,25 @@ abstract class LocationTimeZoneProvider implements Dumpable { mThreadingDomain.assertCurrentThread(); Objects.requireNonNull(timeZoneProviderEvent); + // If the provider has made a suggestion with unknown time zone IDs it cannot be used to set + // the device's time zone. This logic prevents bad time zone IDs entering the time zone + // detection logic from third party code. + // + // An event containing an unknown time zone ID could occur if the provider is using a + // different TZDB version than the device. Provider developers are expected to take steps to + // avoid version skew problem, e.g. by ensuring atomic updates with the platform time zone + // rules, or providing IDs based on the device's TZDB version, so this is not considered a + // common case. + // + // Treating a suggestion containing unknown time zone IDs as "uncertain" in the primary + // enables immediate failover to a secondary provider, one that might provide valid IDs for + // the same location, which should provide better behavior than just ignoring the event. + if (hasInvalidTimeZones(timeZoneProviderEvent)) { + infoLog("event=" + timeZoneProviderEvent + " has unsupported time zones. " + + "Replacing it with uncertain event."); + timeZoneProviderEvent = TimeZoneProviderEvent.createUncertainEvent(); + } + synchronized (mSharedLock) { debugLog("handleTimeZoneProviderEvent: mProviderName=" + mProviderName + ", timeZoneProviderEvent=" + timeZoneProviderEvent); @@ -707,6 +743,20 @@ abstract class LocationTimeZoneProvider implements Dumpable { } } + private boolean hasInvalidTimeZones(@NonNull TimeZoneProviderEvent event) { + if (event.getSuggestion() == null) { + return false; + } + + for (String timeZone : event.getSuggestion().getTimeZoneIds()) { + if (!mTimeZoneIdValidator.isValid(timeZone)) { + return true; + } + } + + return false; + } + @GuardedBy("mSharedLock") private void assertIsStarted() { ProviderState currentState = mCurrentState.get(); diff --git a/services/core/java/com/android/server/timezonedetector/location/ZoneInfoDbTimeZoneIdValidator.java b/services/core/java/com/android/server/timezonedetector/location/ZoneInfoDbTimeZoneIdValidator.java new file mode 100644 index 0000000000000..cab5ad25c54e5 --- /dev/null +++ b/services/core/java/com/android/server/timezonedetector/location/ZoneInfoDbTimeZoneIdValidator.java @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2021 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 com.android.server.timezonedetector.location; + +import android.annotation.NonNull; + +import com.android.i18n.timezone.ZoneInfoDb; + +class ZoneInfoDbTimeZoneIdValidator implements + LocationTimeZoneProvider.TimeZoneIdValidator { + + @Override + public boolean isValid(@NonNull String timeZoneId) { + return ZoneInfoDb.getInstance().hasTimeZone(timeZoneId); + } +} diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/location/ControllerImplTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/location/ControllerImplTest.java index 4284240c72b40..3daa7f0483c67 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/location/ControllerImplTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/location/ControllerImplTest.java @@ -72,6 +72,7 @@ public class ControllerImplTest { private TestCallback mTestCallback; private TestLocationTimeZoneProvider mTestPrimaryLocationTimeZoneProvider; private TestLocationTimeZoneProvider mTestSecondaryLocationTimeZoneProvider; + private FakeTimeZoneIdValidator mTimeZoneAvailabilityChecker; @Before public void setUp() { @@ -80,10 +81,13 @@ public class ControllerImplTest { // will never get a chance to execute. mTestThreadingDomain = new TestThreadingDomain(); mTestCallback = new TestCallback(mTestThreadingDomain); + mTimeZoneAvailabilityChecker = new FakeTimeZoneIdValidator(); mTestPrimaryLocationTimeZoneProvider = - new TestLocationTimeZoneProvider(mTestThreadingDomain, "primary"); + new TestLocationTimeZoneProvider( + mTestThreadingDomain, "primary", mTimeZoneAvailabilityChecker); mTestSecondaryLocationTimeZoneProvider = - new TestLocationTimeZoneProvider(mTestThreadingDomain, "secondary"); + new TestLocationTimeZoneProvider( + mTestThreadingDomain, "secondary", mTimeZoneAvailabilityChecker); } @Test @@ -1177,8 +1181,10 @@ public class ControllerImplTest { /** * Creates the instance. */ - TestLocationTimeZoneProvider(ThreadingDomain threadingDomain, String providerName) { - super(threadingDomain, providerName); + TestLocationTimeZoneProvider(ThreadingDomain threadingDomain, + String providerName, + TimeZoneIdValidator timeZoneIdValidator) { + super(threadingDomain, providerName, timeZoneIdValidator); } public void setFailDuringInitialization(boolean failInitialization) { @@ -1311,4 +1317,14 @@ public class ControllerImplTest { mTestProviderState.commitLatest(); } } + + private static final class FakeTimeZoneIdValidator + implements LocationTimeZoneProvider.TimeZoneIdValidator { + + @Override + public boolean isValid(@NonNull String timeZoneId) { + return true; + } + + } } diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderTest.java index 095c868fc74cd..278fdaff260fe 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderTest.java @@ -32,6 +32,8 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static java.util.Arrays.asList; + import android.annotation.NonNull; import android.annotation.Nullable; import android.os.Bundle; @@ -50,7 +52,9 @@ import org.junit.Test; import java.time.Duration; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; /** @@ -62,20 +66,25 @@ public class LocationTimeZoneProviderTest { private static final long ARBITRARY_ELAPSED_REALTIME_MILLIS = 123456789L; private TestThreadingDomain mTestThreadingDomain; - private TestProviderListener mProviderListener; + private FakeTimeZoneIdValidator mTimeZoneAvailabilityChecker; @Before public void setUp() { mTestThreadingDomain = new TestThreadingDomain(); mProviderListener = new TestProviderListener(); + mTimeZoneAvailabilityChecker = new FakeTimeZoneIdValidator(); } @Test public void lifecycle() { String providerName = "arbitrary"; TestLocationTimeZoneProvider provider = - new TestLocationTimeZoneProvider(mTestThreadingDomain, providerName); + new TestLocationTimeZoneProvider( + mTestThreadingDomain, + providerName, + mTimeZoneAvailabilityChecker); + mTimeZoneAvailabilityChecker.validIds("Europe/London"); // initialize() provider.initialize(mProviderListener); @@ -163,7 +172,10 @@ public class LocationTimeZoneProviderTest { public void defaultHandleTestCommandImpl() { String providerName = "primary"; TestLocationTimeZoneProvider provider = - new TestLocationTimeZoneProvider(mTestThreadingDomain, providerName); + new TestLocationTimeZoneProvider( + mTestThreadingDomain, + providerName, + mTimeZoneAvailabilityChecker); TestCommand testCommand = TestCommand.createForTests("test", new Bundle()); AtomicReference resultReference = new AtomicReference<>(); @@ -180,8 +192,12 @@ public class LocationTimeZoneProviderTest { public void stateRecording() { String providerName = "primary"; TestLocationTimeZoneProvider provider = - new TestLocationTimeZoneProvider(mTestThreadingDomain, providerName); + new TestLocationTimeZoneProvider( + mTestThreadingDomain, + providerName, + mTimeZoneAvailabilityChecker); provider.setStateChangeRecordingEnabled(true); + mTimeZoneAvailabilityChecker.validIds("Europe/London"); // initialize() provider.initialize(mProviderListener); @@ -218,6 +234,34 @@ public class LocationTimeZoneProviderTest { provider.assertLatestRecordedState(PROVIDER_STATE_DESTROYED); } + @Test + public void considerSuggestionWithInvalidTimeZoneIdsAsUncertain() { + String providerName = "primary"; + TestLocationTimeZoneProvider provider = + new TestLocationTimeZoneProvider( + mTestThreadingDomain, + providerName, + mTimeZoneAvailabilityChecker); + provider.setStateChangeRecordingEnabled(true); + provider.initialize(mProviderListener); + + ConfigurationInternal config = USER1_CONFIG_GEO_DETECTION_ENABLED; + Duration arbitraryInitializationTimeout = Duration.ofMinutes(5); + Duration arbitraryInitializationTimeoutFuzz = Duration.ofMinutes(2); + provider.startUpdates(config, arbitraryInitializationTimeout, + arbitraryInitializationTimeoutFuzz); + + List invalidTimeZoneIds = asList("Atlantic/Atlantis"); + TimeZoneProviderSuggestion invalidIdSuggestion = new TimeZoneProviderSuggestion.Builder() + .setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS) + .setTimeZoneIds(invalidTimeZoneIds) + .build(); + TimeZoneProviderEvent event = + TimeZoneProviderEvent.createSuggestionEvent(invalidIdSuggestion); + provider.simulateProviderEventReceived(event); + provider.assertLatestRecordedState(PROVIDER_STATE_STARTED_UNCERTAIN); + } + /** A test stand-in for the real {@link LocationTimeZoneProviderController}'s listener. */ private static class TestProviderListener implements ProviderListener { @@ -251,8 +295,9 @@ public class LocationTimeZoneProviderTest { /** Creates the instance. */ TestLocationTimeZoneProvider(@NonNull ThreadingDomain threadingDomain, - @NonNull String providerName) { - super(threadingDomain, providerName); + @NonNull String providerName, + @NonNull TimeZoneIdValidator timeZoneIdValidator) { + super(threadingDomain, providerName, timeZoneIdValidator); } @Override @@ -308,4 +353,19 @@ public class LocationTimeZoneProviderTest { recordedStates.get(recordedStates.size() - 1).stateEnum); } } + + private static final class FakeTimeZoneIdValidator + implements LocationTimeZoneProvider.TimeZoneIdValidator { + private final Set mValidTimeZoneIds = new HashSet<>(); + + @Override + public boolean isValid(@NonNull String timeZoneId) { + return mValidTimeZoneIds.contains(timeZoneId); + } + + public void validIds(String... timeZoneIdss) { + mValidTimeZoneIds.addAll(asList(timeZoneIdss)); + } + + } } diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/location/ZoneInfoDbTimeZoneIdValidatorTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/location/ZoneInfoDbTimeZoneIdValidatorTest.java new file mode 100644 index 0000000000000..5561b2c6a7aa5 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/location/ZoneInfoDbTimeZoneIdValidatorTest.java @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2021 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 com.android.server.timezonedetector.location; + +import static com.google.common.truth.Truth.assertWithMessage; + +import android.platform.test.annotations.Presubmit; + +import org.junit.Test; + +import java.util.Arrays; +import java.util.List; +import java.util.TimeZone; + +@Presubmit +public class ZoneInfoDbTimeZoneIdValidatorTest { + private final LocationTimeZoneProvider.TimeZoneIdValidator mTzChecker = + new ZoneInfoDbTimeZoneIdValidator(); + + @Test + public void timeZoneIdsFromZoneInfoDbAreValid() { + for (String timeZone : TimeZone.getAvailableIDs()) { + assertWithMessage("Time zone %s should be supported", timeZone) + .that(mTzChecker.isValid(timeZone)).isTrue(); + } + } + + @Test + public void nonExistingZones_areNotSupported() { + List nonExistingTimeZones = Arrays.asList( + "SystemV/HST10", "Atlantic/Atlantis", "EUROPE/LONDON", "Etc/GMT-5:30" + ); + + for (String timeZone : nonExistingTimeZones) { + assertWithMessage(timeZone + " is not a valid time zone") + .that(mTzChecker.isValid(timeZone)) + .isFalse(); + } + } +}