Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Breaking change]: FindSystemTimeZoneById Returns Incorrect StandardName for Id #41830

Closed
3 tasks
jchli opened this issue Jul 18, 2024 · 4 comments
Closed
3 tasks
Assignees
Labels
breaking-change Indicates a .NET Core breaking change doc-idea Indicates issues that are suggestions for new topics [org][type][category] Pri1 High priority, do before Pri2 and Pri3 ⌚ Not Triaged Not triaged

Comments

@jchli
Copy link

jchli commented Jul 18, 2024

Description

It looks like the TimeZoneInfo object that is returned from FindSystemTimeZoneById() is returning an incorrect name for a given ID (It gives the correct offset but with "GMT"). Further, it seems to only be a problem for IDs that are shaped like "Eastern Standard Time". In contrast, "America/New_York" works fine.

I suspect that this has something to do with the caching behavior introduced in .NET 8. Take a look at the unit test below, which will fail in different ways depending on the test cases provided

Unit test to reproduce behavior:

public class TimeZoneHelperTests
{
    [Theory]
    [InlineData("Eastern Standard Time", false, "EST")] // fully qualified IDs fail sometimes
    [InlineData("Central Standard Time", false, "CST")]
    [InlineData("Mountain Standard Time", false, "MST")]
    [InlineData("Pacific Standard Time", false, "PST")]
    [InlineData("Eastern Standard Time", true, "EDT")]
    [InlineData("Central Standard Time", true, "CDT")]
    [InlineData("Mountain Standard Time", true, "MDT")]
    [InlineData("Pacific Standard Time", true, "PDT")]
    [InlineData("America/New_York", false, "EST")] //America/X type IDs always succeed
    [InlineData("America/Chicago", false, "CST")]
    [InlineData("America/Denver", false, "MST")]
    [InlineData("America/Los_Angeles", false, "PST")]
    [InlineData("America/New_York", true, "EDT")]
    [InlineData("America/Chicago", true, "CDT")]
    [InlineData("America/Denver", true, "MDT")]
    [InlineData("America/Los_Angeles", true, "PDT")]
    public void GetTzAbbreviation_BuildsCorrectAbbreviation(string timeZoneId, bool isDaylightSavingsTime, string expectedAbbreviation)
    {
        // Arrange
        var tzi = TimeZoneInfo.FindSystemTimeZoneById(timeZoneId);
        
        // Act
        var abbreviation = GetTzAbbreviation(isDaylightSavingsTime, tzi);

        // Assert
        Assert.Equal(expectedAbbreviation, abbreviation);
    }
    
    private static string GetTzAbbreviation(bool isDaylightSavingsTime, TimeZoneInfo timeZoneInfo)
    {
        var timeZoneString = isDaylightSavingsTime ? timeZoneInfo.DaylightName : timeZoneInfo.StandardName;
            
        return string.Concat(Regex
            .Matches(timeZoneString, "[A-Z]")
            .Select(match => match.Value));
    }
}

Version

.NET 8 GA

Previous behavior

The standard name should be analogous the provided time zone.

New behavior

The standard name is set to GMT

Type of breaking change

  • Binary incompatible: Existing binaries might encounter a breaking change in behavior, such as failure to load or execute, and if so, require recompilation.
  • Source incompatible: When recompiled using the new SDK or component or to target the new runtime, existing source code might require source changes to compile successfully.
  • Behavioral change: Existing binaries might behave differently at run time.

Reason for change

N/A

Recommended action

Seems like the America/X IDs are working properly. Use those instead

Feature area

Core .NET libraries

Affected APIs

TimeZoneInfo.FindSystemTimeZoneById(String)

@jchli jchli added breaking-change Indicates a .NET Core breaking change doc-idea Indicates issues that are suggestions for new topics [org][type][category] Pri1 High priority, do before Pri2 and Pri3 labels Jul 18, 2024
@dotnet-bot dotnet-bot added ⌚ Not Triaged Not triaged labels Jul 18, 2024
@gewarren
Copy link
Contributor

gewarren commented Aug 2, 2024

@tarekgh Can you take a look at this issue?

@tarekgh
Copy link
Member

tarekgh commented Aug 3, 2024

@jchli which OS are you running on? Windows, Linux, Mac...

@tarekgh
Copy link
Member

tarekgh commented Aug 6, 2024

I have done some investigations and this is unintentional bug. I careated the PR dotnet/runtime#106038 and will try to check if we can port the fix to 8.0 servicing. If we couldn't, then we can document the behavior as described here.

To give some more information, this issue only happen on non-Windows OS's when using Windows time zone ids e.g. Eastern Standard Time.

@tarekgh
Copy link
Member

tarekgh commented Aug 8, 2024

I have ported the fix for this to 8.0 servicing through the PR dotnet/runtime#106056. I am closing this issue as no more actions are needed here. Please let me know if you have any questions, I can help with answering them. Thanks @jchli for your report here.

@tarekgh tarekgh closed this as completed Aug 8, 2024
@dotnet-bot dotnet-bot added ⌚ Not Triaged Not triaged and removed ⌚ Not Triaged Not triaged labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Indicates a .NET Core breaking change doc-idea Indicates issues that are suggestions for new topics [org][type][category] Pri1 High priority, do before Pri2 and Pri3 ⌚ Not Triaged Not triaged
Projects
None yet
Development

No branches or pull requests

4 participants