dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.91k stars 4.63k forks source link

TimeZoneInfo.Utc can no longer be used for reliable comparison to detect Utc #90046

Closed osexpert closed 1 year ago

osexpert commented 1 year ago

Description

In the old days, before TimeZoneInfo supported Iana, I could write something like this to reliably detect Utc: var tzIsUtc1 = tz == TimeZoneInfo.Utc;

After Iana came, this is no longer this simple: var tzIsUtc2 = TimeZoneInfo.FindSystemTimeZoneById("UTC") == TimeZoneInfo.Utc; // true var tzIsUtc3 = TimeZoneInfo.FindSystemTimeZoneById("Etc/UTC") == TimeZoneInfo.Utc; // false var tzIsUtc4 = TimeZoneInfo.FindSystemTimeZoneById("Etc/Zulu") == TimeZoneInfo.Utc; // false var tzIsUtc5 = TimeZoneInfo.FindSystemTimeZoneById("Zulu") == TimeZoneInfo.Utc; // false etc.

So my old code no longer works to detect Utc. I notice that they all seem to share the same DisplayName ("(UTC) Coordinated Universal Time"), but it feels a bit iffy to use this to detect Utc.

So I wonder...is this code really broken now? Should the 3 false's above really have been true? If you think it is correct that they return false...then at least it would have been nice with a bool that could have said that the time zone is Utc or not? I feel using the DisplayName for detecting Utc is unreliable and sensitive.

Reproduction Steps

Run the code above?

Expected behavior

That all Utc time zones should have been equal, thou I am not completely sure.

Actual behavior

All the different Utc time zones are not equal to each other, and not equal to TimeZoneInfo.Utc. I guess this make TimeZoneInfo.Utc a "red herring" that you think you can trust\use for Utc detection.

Regression?

Kind of. Before Iana there was only one Utc time zone.

Known Workarounds

Yes, can use DisplayName or compare with a list of all known time zone ids that are Utc to detect Utc. I wonder how you do it internally.

Configuration

Win10, NET7

Other information

Suggested fix: Either make all Utc time zones compare equal to each other, or create a new bool IsUtc in TimeZoneInfo, so we can reliably compare them ourself, without using DisplayName, a list of known Utc tiem zone id's or other "hacks".

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-datetime See info in area-owners.md if you want to be subscribed.

Issue Details
### Description In the old days, before TimeZoneInfo supported Iana, I could write something like this to reliably detect Utc: var tzIsUtc1 = tz == TimeZoneInfo.Utc; After Iana came, this is no longer this simple: var tzIsUtc2 = TimeZoneInfo.FindSystemTimeZoneById("UTC") == TimeZoneInfo.Utc; // true var tzIsUtc3 = TimeZoneInfo.FindSystemTimeZoneById("Etc/UTC") == TimeZoneInfo.Utc; // false var tzIsUtc4 = TimeZoneInfo.FindSystemTimeZoneById("Etc/Zulu") == TimeZoneInfo.Utc; // false var tzIsUtc5 = TimeZoneInfo.FindSystemTimeZoneById("Zulu") == TimeZoneInfo.Utc; // false etc. So my old code no longer works to detect Utc. I notice that they all seem to share the same DisplayName ("(UTC) Coordinated Universal Time"), but it feels a bit iffy to use this to detect Utc. So I wonder...is this code really broken now? Should the 3 false's above really have been true? If you think it is correct that they return false...then at least it would have been nice with a bool that could have said that the time zone is Utc or not? I feel using the DisplayName for detecting Utc is unreliable and sensitive. ### Reproduction Steps Run the code above? ### Expected behavior That all Utc time zones should have been equal, thou I am not completely sure. ### Actual behavior All the different Utc time zones are not equal to each other, and not equal to TimeZoneInfo.Utc. I guess this make TimeZoneInfo.Utc a "red herring" that you think you can trust\use for Utc detection. ### Regression? Kind of. Before Iana there was only one Utc time zone. ### Known Workarounds Yes, can use DisplayName or compare with a list of all known time zone ids that are Utc to detect Utc. I wonder how you do it internally. ### Configuration Win10, NET7 ### Other information Suggested fix: Either make all Utc time zones compare equal to each other, or create a new bool IsUtc in TimeZoneInfo, so we can reliably compare them ourself, without using DisplayName, a list of known Utc tiem zone id's or other "hacks".
Author: osexpert
Assignees: -
Labels: `untriaged`, `area-System.DateTime`
Milestone: -
osexpert commented 1 year ago

From this code on Win10, NET7 I can find a current list of time zones that are Utc.

            List<string> tzu = new();
                foreach (var ii in IanaTimeZone.GetIanaIds())
                {
                    try
                    {

                        var t = TimeZoneInfo.FindSystemTimeZoneById(ii);
                        if (t.DisplayName == "(UTC) Coordinated Universal Time")
                            tzu.Add(t.Id);
                    }
                    catch { }
                }
    [0] "Etc/GMT"   string
    [1] "Etc/GMT+0" string
    [2] "Etc/GMT0"  string
    [3] "Etc/GMT-0" string
    [4] "Etc/Greenwich" string
    [5] "Etc/UCT"   string
    [6] "Etc/Universal" string
    [7] "Etc/UTC"   string
    [8] "Etc/Zulu"  string
    [9] "GMT"   string
    [10]    "GMT+0" string
    [11]    "GMT0"  string
    [12]    "GMT-0" string
    [13]    "Greenwich" string
    [14]    "UCT"   string
    [15]    "Universal" string
    [16]    "UTC"   string
    [17]    "Zulu"  string

Edit: but this seems not complete, because TimeZoneInfo,Utc seems to use just "UTC" for everything:

            Assert.Equal("UTC", TimeZoneInfo.Utc.DisplayName);
            Assert.Equal("UTC", TimeZoneInfo.Utc.StandardName);
            Assert.Equal("UTC", TimeZoneInfo.Utc.DaylightName);

This is a bit confusing. Edit: Sorry, this was in net4.8. It seems net4.8 also has similar problem, but in a smaller scale:

            var poison = TimeZoneInfo.FindSystemTimeZoneById("uTc");
            var list = TimeZoneInfo.GetSystemTimeZones().Where(tz => tz.DisplayName == "(UTC) Coordinated Universal Time" || tz.DisplayName == "UTC").ToList();
            var tz1 = list.Single(); // tz1.DisplayName: "(UTC) Coordinated Universal Time"
            var tz2 = TimeZoneInfo.Utc; // tz1.DisplayName: "UTC"
            var eq = tz1 == tz2; // false

So here we get two different Utc time zones that are not equal. Thou they are equal in Id "UTC" but not in DisplayName. I also tried to "poison" the cache with funny casing, but this did not work here (fortunately) and not in net6 either (good).

filipnavara commented 1 year ago

Can you use timeZoneInfo == TimeZoneInfo.Utc || timeZoneInfo.HasSameRules(TimeZoneInfo.Utc)?

tarekgh commented 1 year ago

@osexpert it is the nature of supporting all Ids of UTC that we didn't support before.

The right way to do it is to compare the TimeZoneInfo.StandardName should be equal to Coordinated Universal Time. Please don't use the DisplayName because DisplayName depends on the UI settings which can return different names on different environment. Let me know if this works fine for you.

@filipnavara I wouldn't recommend this way because we can have other time zones without rules and different than UTC.

ghost commented 1 year ago

This issue has been marked needs-author-action and may be missing some important information.

filipnavara commented 1 year ago

I wouldn't recommend this way because we can have other time zones without rules and different than UTC.

HasSameRules is documented to also compare base UTC offset. That said, I guess it depends on what you are trying to achieve semantically. If you want to differentiate "UTC" from a "country time zone that happens to observe UTC".

osexpert commented 1 year ago

Can you use timeZoneInfo == TimeZoneInfo.Utc || timeZoneInfo.HasSameRules(TimeZoneInfo.Utc)?

Hi, possibly it will work:-) But there is not hard to find workarounds, there is possible at least 3 different ones that could work: for net6: tz == TimeZoneInfo.Utc || tz.StandardName == "Coordinated Universal Time" for netstandard20: tz == TimeZoneInfo.Utc || tz.StandardName == "Coordinated Universal Time" || tz.StandardName == "UTC" for net6\netstandard20 (this is the one I am currently using, but I may change my mind:-):

tz == TimeZoneInfo.Utc || tz.Id switch {
                "Etc/GMT" => true,
                "Etc/GMT+0" => true,
                "Etc/GMT0" => true,
                "Etc/GMT-0" => true,
                "Etc/Greenwich" => true,
                "Etc/UCT" => true,
                "Etc/Universal" => true,
                "Etc/UTC" => true,
                "Etc/Zulu" => true,
                "GMT" => true,
                "GMT+0" => true,
                "GMT0" => true,
                "GMT-0" => true,
                "Greenwich" => true,
                "UCT" => true,
                "Universal" => true,
                "UTC" => true,
                "Zulu" => true,
                _ => false
            };

The problem is that I feel all these are hacks and may break at any moment. I wish for a standard solution for this.

osexpert commented 1 year ago

The right way to do it is to compare the TimeZoneInfo.StandardName should be equal to Coordinated Universal Time

Ok, great. Good to know! Do you know if this is documented?

It could be cool to have a bool for this thou (IsUtc) to make it easier to use:-) Thou I guess the next complaint then would be that all time zones that has IsUtc = true does not equal TimeZoneInfo.Utc :-D

osexpert commented 1 year ago

If you want to differentiate "UTC" from a "country time zone that happens to observe UTC".

Yes, I need to differentiate here.

tarekgh commented 1 year ago

Do you know if this is documented?

Not explicitly documented but it is implicitly indicated in the doc https://learn.microsoft.com/en-us/dotnet/api/system.timezoneinfo.utc?view=net-7.0 as it always refers to the zone as Coordinated Universal Time. I don't mind if we update the doc too. Feel free to open issue or PR in https://github.com/dotnet/dotnet-api-docs/ repo to do it.

By the way, this check should be enough tz.StandardName == "Coordinated Universal Time" || tz.StandardName == "UTC". You don't have to compare it to TimeZoneInfo.Utc.

It could be cool to have a bool for this thou (IsUtc) to make it easier to use:-)

We can consider that if we see more demand for it. It will be a little bit tricky though if users assume all zones with 0 offset and no rules should be treated as Utc or not.

I am closing the issue but feel free to send any more questions if you still have any. Thanks for your report.