Several TZ related fixes#75
Merged
Merged
Conversation
POSIX[1] does not describe the empty string case: it's either starting with colon (implementation-defined) or at least 3 bytes for 'std' designation. I decided to follow example of glibc. From tzset(3): > If the TZ variable does appear in the environment, but its value > is empty, or its value cannot be interpreted using any of the formats > specified below, then Coordinated Universal Time (UTC) is used. A side note: this is actually a bit broken in glibc: the empty case is checked but sets the designation to "Universal", which is not a valid designation but results in what behaves as UTC. (Except that word "Universal", not "UTC" appears in as designation--just like with any "random" text that is not valid sub-path of zoneinfo.) [1]: https://pubs.opengroup.org/onlinepubs/9699919799.2016edition/basedefs/V1_chap08.html
The `i -= 1;` line would crash if TZ started with a numeric character,
which is not valid POSIX TZ, so it should return error.InvalidPosix.
I've also added check that the designation must be at least 3
characters, as per POSIX[1]:
> std and dst
> Indicate no less than three, nor more than {TZNAME_MAX}, bytes that are
> the designation for the standard (std) or the alternative (dst -such as
> Daylight Savings Time) timezone. Only std is required; if dst is missing,
> then the alternative time does not apply in this locale.
I haven't added the TZNAME_MAX check as it would require larger changes
and would probably aply to the quoted case as well.
[1]: https://pubs.opengroup.org/onlinepubs/9699919799.2016edition/basedefs/V1_chap08.html
The POSIX TZ parser accepted malformed input and could split fields incorrectly when offsets or names were missing. In particular, DST offsets at end of string updated the standard offset, and malformed strings such as UTC,foo or short DST names were accepted. Parse standard and daylight names consistently, require valid time fields, fix DST offset assignment, and add regressions for malformed TZ strings and empty TZ handling.
Owner
|
Thanks! |
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
2 participants
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.
Details are in commit messages; feel free to cherry-pick and/or request changes (or squash as needed).
TZ="0", ie. numeric (should return error)TZ="", ie. returning UTC instead (like glibc)The last one is the most interesting: the empty string case is not clearly specified, so @rockorager , should zeit follow what glibc does? If not, I'd be happy to change it to some other behavior.