TimeZoneInfo.FindSystemTimeZoneById throws TimeZoneNotFoundException

[mjrousos]

.NET Core exposes TimeZoneInfo.FindSystemTimeZoneById(string) (MSDN) which throws a TimeZoneNotFoundException.

But the exception can't be caught because TimeZoneNotFoundException isn't exposed in .NET Core. We should include the exception type in the System.Runtime contract alongside the API that throws it.

Repro:

using System;

public class Program
{
    public static void Main(string[] args)
    {
        Console.WriteLine(TimeZoneInfo.FindSystemTimeZoneById("DoesNotExist").ToString());
    }
}

[joshfree]

good catch @mjrousos

/cc @eerhardt

[eerhardt]

Tests caught this, but a human decided it was OK:

https://github.com/dotnet/corefx/blob/master/src/System.Runtime/tests/System/TimeZoneInfoTests.cs#L100

https://github.com/dotnet/corefx/blob/master/src/System.Runtime/tests/System/TimeZoneInfoTests.cs#L137-L140

[mjrousos]

I'll cast my vote as a human that it's not ok ;-)

[jchannon]

What's the recommended practice to handle the scenario where this exception is caught because of cross platform differences, should it just check for Exception when assigning a TimeZoneInfo variable?

[tarekgh]

@jchannon we have a future issue to support the IANA Ids on Windows and Windows Id on Linux/OSX. till we have this work done, you'll need to handle the cross platform manually. so it depends on your scenario you are using TZ for. if you need to have some level of cross platform TZ support you will need to do manual mapping between Windows Id and IANA Ids, otherwise you'll just need to fallback to some TZ. again this really depends on the scenario. if you can tell more about your scenario, we may think together what is best can be done for now in such scenario.

[jchannon]

Just to confirm, CoreFx will support Linux timezone Ids or you'll make the
Windows Id's work when running on Linux or both?

The example code we have at the moment is this:
try
{
var timeZone = license.DefaultTimeZone;
}
catch (TimeZoneNotFoundException)
{
// we couldn't find a suitable time zone..
license.DefaultTimeZoneId = unix ? "Europe/London" : "GMT
Standard Time";
}

If the exception is removed shall I just catch for Exception? In theory
this should never happen as we wouldn't expect our customers at the moment
to swap OSs.

[tarekgh]

Just to confirm, CoreFx will support Linux timezone Ids or you'll make the
Windows Id's work when running on Linux or both?

The future work should allow both.

If the exception is removed shall I just catch for Exception?

Yes, you need to catch Exception instead of TimeZoneNotFoundException till we expose TimeZoneNotFoundException.

[jchannon]

Thanks. Good news about the Ids

On Tuesday, 3 May 2016, Tarek Mahmoud Sayed notifications@github.com
wrote:

Just to confirm, CoreFx will support Linux timezone Ids or you'll make the
Windows Id's work when running on Linux or both?

The future work should allow both.

If the exception is removed shall I just catch for Exception?

Yes, you need to catch Exception instead of TimeZoneNotFoundException till
we expose TimeZoneNotFoundException.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/dotnet/corefx/issues/7552#issuecomment-216585265

[jchannon]

Final question. How does this effect the Portable Analyzer tool. At the
moment it says it's invalid but looks like you're planning on making it
ship with RTM so how/when does the Analyzer get updated?

On Tuesday, 3 May 2016, Tarek Mahmoud Sayed notifications@github.com
wrote:

Just to confirm, CoreFx will support Linux timezone Ids or you'll make the
Windows Id's work when running on Linux or both?

The future work should allow both.

If the exception is removed shall I just catch for Exception?

Yes, you need to catch Exception instead of TimeZoneNotFoundException till
we expose TimeZoneNotFoundException.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/dotnet/corefx/issues/7552#issuecomment-216585265

[mjrousos]

The portability analyzer tool pulls its data from reference assemblies (via API catalog), so it should update automatically once the changes are made. Until then, it may be worthwhile to add a 'recommended action' that mentions the API is incoming and provides a short-term workaround.

[jchannon]

So at the moment the Analyzer is using rc1 assemblies not rc2?

On Tuesday, 3 May 2016, Mike Rousos notifications@github.com wrote:

The portability analyzer tool pulls its data from reference assemblies
(via API catalog), so it should update automatically once the changes are
made. Until then, it may be worthwhile to add a 'recommended action' that
mentions the API is incoming and provides a short-term workaround.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/dotnet/corefx/issues/7552#issuecomment-216589162

[mjrousos]

@terrajobst @twsouthwick - do you guys know which assemblies are used to populate API catalog? Is it the RC1 stuff from NuGet or RC2/3 binaries from myget?

[twsouthwick]

It pulls it from the current CI builds as far as I know. There is a manual step to get it into the analyzer that we are working on automating.

@conniey

[hann013]

The tests were fixed by dotnet/corefx#11149.

[jchannon]

Hi @tarekgh

Any news on when Windows will be able to return/handle Linux Timezone Ids?

[tarekgh]

Hi @jchannon

not soon, this is planned to be done in the future as we are currently busy working on the netstandard 2.0.

[jchannon]

So post netstandard 2.0?

On 13 October 2016 at 17:10, Tarek Mahmoud Sayed notifications@github.com
wrote:

Hi @jchannon https://github.com/jchannon

not soon, this is planned to be done in the future as we are currently
busy working on the netstandard 2.0.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/corefx/issues/7552#issuecomment-253560315, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGapozKfFuE3_x_KgIbPHSej9SBKweMks5qzlgAgaJpZM4IBYxk
.

[tarekgh]

So post netstandard 2.0?

hopefully. this is really the feature I need to do as soon as we get a chance.

[jchannon]

Yup really looking forward to it

On Thursday, 13 October 2016, Tarek Mahmoud Sayed notifications@github.com
wrote:

So post netstandard 2.0?

hopefully. this is really the feature I need to do as soon as we get a
chance.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/corefx/issues/7552#issuecomment-253561401, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGaplk37VGe38EmdIrGmna3Xt2clmgYks5qzljogaJpZM4IBYxk
.

[Anwar-Faiz]

The TimeZone exception thrown shows that the API was not unit tested with some negative tests. And, as suggested earlier, an automation test caught it but was ignored or taken as a False Positive.

We Surely need a fix for that. There can be a more crafted attack on the function/API and help crashing the systems.

What is the exception:
If you send undesired or unformatted inputs like "Invalid", "IST" etc, you get an error returned.
An unhandled exception of type 'System.TimeZoneNotFoundException' occurred in mscorlib.dll

Attached the screenshot.

www.w3lc.com

[tarekgh]

@Anwar-Faiz

could you clarify the statement you mentioned We Surely need a fix for that. There can be a more crafted attack on the function/API and help crashing the systems.

What is exactly the attack you are talking about? and what is your suggestion?

Usually, the applications which getting data (e.g. zone names) from external sources has to protect itself when using such data. throwing an exception in the mentioned case is legitimate