ical-org/ical.net

Validation of timezone ID

Closed this issue · 1 comments

@minichma
The timezone ID is an important parameter to the CalDateTime CTORs. That's why it should be validated.
For a trial I modfied the initializer like this:

private void Initialize(DateOnly dateOnly, TimeOnly? timeOnly, string? tzId)
{
    if (tzId != null)
    {
        if (DateUtil.GetZone(tzId, false) is null)
            throw new ArgumentException($"Invalid time zone ID: {tzId}", nameof(tzId));
    }
    // other code
}

and ran the unit tests with 6 failures in DateTimeSerializer.Deserialize(...) as followss:

  • Timezone ID "大阪、札幌、東京" (Osaka, Sapporo, Tokyo) was accepted instead of Asia/Tokyo
  • Timezone ID "Sarajewo, Skopie, Warszawa, Zagrzeb" was accepted instead of Europe/Sarajevo, Europe/Skopje, Europe/Warsaw or Europe/Zagreb
  • Timezone ID "Beijing" was accepted instead of Asia/Shanghai

DateUtil.GetZone(...)also explicitly accepts "US-Eastern" instead of US/Eastern, commented with "US/Eastern is commonly represented as US-Eastern".

Conclusions so far:

  1. CalDateTime should always verify the timezone argument
  2. DateTimeZone GetZone(string tzId, bool useLocalIfNotFound = true) should not take "local timezone" if not found
  3. IANA timezones should be the ones we accept. TBD: also Windows TimeZoneInfo IDs?

What do you think?

That's a tricky one. People might be using the lib just for parsing/writing calendar files. In that case its not required that Ical.Net knows the actual tzId. Actually there can be fully legit cases where users of the lib use their own IDs. As the RFC requires the VTIMEZON to be part of a valid calendar, this is perfectly fine. For that reason I think we should not generally verify the tzid. Only when we use it for generating the recurrences or doing tz-related calculations we need to verify it, but in that case its done anyways I think.

Generally speaking I think we should be lenient with users' inputs as far as possible. The lib should not be responsible for sanitization. If we are to strict, the lib might not stop working together with other libs that might slightly deviate from the RFC.