Skip to content

Commit

Permalink
Android remove backward timezones (#64028)
Browse files Browse the repository at this point in the history 
Fixes #63693

It was discovered that Android produces duplicate TimeZone DisplayNames among all timezone IDs in GetSystemTimeZones. These duplicate DisplayNames occur across TimeZone IDs that are aliases, where all except one are backward timezone IDs.

If a name is changed, put its old spelling in the 'backward' file

From the Android TimeZone data file tzdata, it isn't obvious which TimeZone IDs are backward (I find it strange that they're included in the first place), however we discovered that on some versions of Android, there is an adjacent file tzlookup.xml that can aid us in determining which TimeZone IDs are "current" (not backward).

This PR aims to utilize tzlookup.xml when it exists and post-filter's the Populated TimeZone IDs in the AndroidTzData instance by removing IDs and their associated information (byteoffset and length) from the AndroidTzData instance if it is not found in tzlookup.xml. This is using the assumption that all non-backward TimeZone IDs make it to the tzlookup.xml file.

This PR also adds a new TimeZoneInfo Test to check whether or not there are duplicate DisplayNames in GetSystemTimeZones
  • Loading branch information
@mdh1418
mdh1418 authored Jan 24, 2022
1 parent 8bfb725 commit 74d8d0d
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 9 deletions.
92 changes: 83 additions & 9 deletions src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.Android.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ private sealed class AndroidTzData
private string[] _ids;
private int[] _byteOffsets;
private int[] _lengths;
private bool[] _isBackwards;
private string _tzFileDir;
private string _tzFilePath;

Expand Down Expand Up @@ -230,7 +231,7 @@ public AndroidTzData()
foreach (var tzFileDir in tzFileDirList)
{
string tzFilePath = Path.Combine(tzFileDir, TimeZoneFileName);
if (LoadData(tzFilePath))
if (LoadData(tzFileDir, tzFilePath))
{
_tzFileDir = tzFileDir;
_tzFilePath = tzFilePath;
Expand All @@ -241,10 +242,62 @@ public AndroidTzData()
throw new TimeZoneNotFoundException(SR.TimeZoneNotFound_ValidTimeZoneFileMissing);
}

// On some versions of Android, the tzdata file may still contain backward timezone ids.
// We attempt to use tzlookup.xml, which is available on some versions of Android to help
// validate non-backward timezone ids
// tzlookup.xml is an autogenerated file that contains timezone ids in this form:
//
// <timezones ianaversion="2019b">
// <countryzones>
// <country code="au" default="Australia/Sydney" everutc="n">
// <id alts="Australia/ACT,Australia/Canberra,Australia/NSW">Australia/Sydney</id>
// ...
// ...
// <id>Australia/Eucla</id>
// </country>
// <country ...>
// ...
// ...
// ...
// </country>
// </countryzones>
// </timezones>
//
// Once the timezone cache is populated with the IDs, we reference tzlookup id tags
// to determine if an id is backwards and label it as such if they are.
private void FilterBackwardIDs(string tzFileDir, out HashSet<string> tzLookupIDs)
{
tzLookupIDs = new HashSet<string>();
try
{
using (StreamReader sr = File.OpenText(Path.Combine(tzFileDir, "tzlookup.xml")))
{
string? tzLookupLine;
while (sr.Peek() >= 0)
{
if (!(tzLookupLine = sr.ReadLine())!.AsSpan().TrimStart().StartsWith("<id", StringComparison.Ordinal))
continue;

int idStart = tzLookupLine!.IndexOf('>') + 1;
int idLength = tzLookupLine.LastIndexOf("</", StringComparison.Ordinal) - idStart;
if (idStart <= 0 || idLength < 0)
{
// Either the start tag <id ... > or the end tag </id> are not found
continue;
}
string id = tzLookupLine.Substring(idStart, idLength);
tzLookupIDs.Add(id);
}
}
}
catch {}
}

[MemberNotNullWhen(true, nameof(_ids))]
[MemberNotNullWhen(true, nameof(_byteOffsets))]
[MemberNotNullWhen(true, nameof(_lengths))]
private bool LoadData(string path)
[MemberNotNullWhen(true, nameof(_isBackwards))]
private bool LoadData(string tzFileDir, string path)
{
if (!File.Exists(path))
{
Expand All @@ -254,7 +307,7 @@ private bool LoadData(string path)
{
using (FileStream fs = File.OpenRead(path))
{
LoadTzFile(fs);
LoadTzFile(tzFileDir, fs);
}
return true;
}
Expand All @@ -266,15 +319,16 @@ private bool LoadData(string path)
[MemberNotNull(nameof(_ids))]
[MemberNotNull(nameof(_byteOffsets))]
[MemberNotNull(nameof(_lengths))]
private void LoadTzFile(Stream fs)
[MemberNotNull(nameof(_isBackwards))]
private void LoadTzFile(string tzFileDir, Stream fs)
{
const int HeaderSize = 24;
Span<byte> buffer = stackalloc byte[HeaderSize];

ReadTzDataIntoBuffer(fs, 0, buffer);

LoadHeader(buffer, out int indexOffset, out int dataOffset);
ReadIndex(fs, indexOffset, dataOffset);
ReadIndex(tzFileDir, fs, indexOffset, dataOffset);
}

private void LoadHeader(Span<byte> buffer, out int indexOffset, out int dataOffset)
Expand Down Expand Up @@ -303,23 +357,25 @@ private void LoadHeader(Span<byte> buffer, out int indexOffset, out int dataOffs
[MemberNotNull(nameof(_ids))]
[MemberNotNull(nameof(_byteOffsets))]
[MemberNotNull(nameof(_lengths))]
private void ReadIndex(Stream fs, int indexOffset, int dataOffset)
[MemberNotNull(nameof(_isBackwards))]
private void ReadIndex(string tzFileDir, Stream fs, int indexOffset, int dataOffset)
{
int indexSize = dataOffset - indexOffset;
const int entrySize = 52; // Data entry size
int entryCount = indexSize / entrySize;

_byteOffsets = new int[entryCount];
_ids = new string[entryCount];
_lengths = new int[entryCount];

_isBackwards = new bool[entryCount];
FilterBackwardIDs(tzFileDir, out HashSet<string> tzLookupIDs);
for (int i = 0; i < entryCount; ++i)
{
LoadEntryAt(fs, indexOffset + (entrySize*i), out string id, out int byteOffset, out int length);

_byteOffsets[i] = byteOffset + dataOffset;
_ids[i] = id;
_lengths[i] = length;
_isBackwards[i] = !tzLookupIDs.Contains(id);

if (length < 24) // Header Size
{
Expand Down Expand Up @@ -372,7 +428,25 @@ private void LoadEntryAt(Stream fs, long position, out string id, out int byteOf

public string[] GetTimeZoneIds()
{
return _ids;
int numTimeZoneIDs = 0;
for (int i = 0; i < _ids.Length; i++)
{
if (!_isBackwards[i])
{
numTimeZoneIDs++;
}
}
string[] nonBackwardsTZIDs = new string[numTimeZoneIDs];
var index = 0;
for (int i = 0; i < _ids.Length; i++)
{
if (!_isBackwards[i])
{
nonBackwardsTZIDs[index] = _ids[i];
index++;
}
}
return nonBackwardsTZIDs;
}

public string GetTimeZoneDirectory()
Expand Down
14 changes: 14 additions & 0 deletions src/libraries/System.Runtime/tests/System/TimeZoneInfoTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2922,6 +2922,20 @@ public static void AdjustmentRuleBaseUtcOffsetDeltaTest()
Assert.Equal(new TimeSpan(2, 0, 0), customTimeZone.GetUtcOffset(new DateTime(2021, 3, 10, 2, 0, 0)));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/64111", TestPlatforms.Linux)]
public static void NoBackwardTimeZones()
{
ReadOnlyCollection<TimeZoneInfo> tzCollection = TimeZoneInfo.GetSystemTimeZones();
HashSet<String> tzDisplayNames = new HashSet<String>();

foreach (TimeZoneInfo timezone in tzCollection)
{
tzDisplayNames.Add(timezone.DisplayName);
}
Assert.Equal(tzCollection.Count, tzDisplayNames.Count);
}

private static bool IsEnglishUILanguage => CultureInfo.CurrentUICulture.Name.Length == 0 || CultureInfo.CurrentUICulture.TwoLetterISOLanguageName == "en";

private static bool IsEnglishUILanguageAndRemoteExecutorSupported => IsEnglishUILanguage && RemoteExecutor.IsSupported;
Expand Down

0 comments on commit 74d8d0d

Please sign in to comment.