ICU-13694 Support TZ set to a zoneinfo file path in timezone detection#4063
Open
vlasky wants to merge 1 commit into
Open
ICU-13694 Support TZ set to a zoneinfo file path in timezone detection#4063vlasky wants to merge 1 commit into
vlasky wants to merge 1 commit into
Conversation
On Linux with glibc (and other platforms with tzset()), the TZ environment variable may be set to the full path of a zoneinfo file, with or without a leading colon, e.g. TZ=:/usr/share/zoneinfo/America/New_York. Some Linux distributions configure TZ this way, in which case ICU's host timezone detection previously returned the whole path as the timezone ID and detection failed. Extract the Olson ID from the portion of the path following "/zoneinfo/", mirroring the existing handling of the /etc/localtime symlink target. If TZ points at a path without "/zoneinfo/" (e.g. TZ=:/etc/localtime), fall through to the existing TZDEFAULT handling. The directories above the Olson ID need not look like an Olson ID themselves (they may contain digits, e.g. a tzdata version), so a path is recognized before the isValidOlsonID() check. Add a cintltst test covering TZ handling in uprv_tzname(). Based on PR unicode-org#2213 by Jungshik Shin, rebased onto the current realpath-based TZDEFAULT logic and restructured per review feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #2213 by @jungshik, rebased onto the current realpath-based TZDEFAULT logic in putil.cpp and restructured per the review feedback on that PR.
Problem
On Linux with glibc (and on other platforms with tzset()), the TZ environment variable may be set to the full path of a zoneinfo file, with or without a leading colon, e.g.
TZ=:/usr/share/zoneinfo/America/New_York. Some Linux distributions configure TZ this way. In that case ICU's host timezone detection previously returned the whole path as the timezone ID, so detection failed:TimeZone::detectHostTimeZone()reported the raw path (e.g./usr/share/zoneinfo/Asia/Seoul) as the zone ID.Fix
In
uprv_tzname(), when TZ holds an absolute path, extract the Olson ID from the portion of the path following/zoneinfo/(TZZONEINFOTAIL), mirroring the existing handling of the /etc/localtime symlink target. Notes:isValidOlsonID()check, because the directories above the Olson ID need not look like an Olson ID themselves: they may contain digits, e.g. a tzdata version such as/nix/store/abc-tzdata-2024a/share/zoneinfo/America/New_York./zoneinfo/(e.g.TZ=:/etc/localtime), the code falls through to the existing TZDEFAULT handling, which resolves the /etc/localtime symlink.posix/andright/prefixes after/zoneinfo/are skipped as elsewhere.PST8PDTstyle IDs, POSIX rule strings such asCST6CDT5,J129,J131/19:30, and unset TZ.Tests
Added
putiltst/TestTZNameEnvto cintltst. It is hermetic:uprv_tzname()extracts the ID from the path by string parsing without checking that the file exists, so the test uses fabricated paths and does not depend on the host's zoneinfo layout. Verified that the test fails with exactly the four path cases against unpatched putil.cpp and passes with the fix.Also verified manually on Ubuntu 24.04 (glibc, arm64) and macOS with a matrix of 13 TZ values, cross-checked against glibc's own interpretation of each value, and ran
intltest format/TimeZoneTest format/TimeZoneRuleTest format/TimeZoneFormatTest: all pass.Checklist