Skip to content

Don't look up rrulestr TZID values that name files outside TZPATH - #1633

Open
varun3257 wants to merge 1 commit into
dateutil:masterfrom
varun3257:rrulestr-tzid-not-a-path
Open

varun3257 wants to merge 1 commit into
dateutil:masterfrom
varun3257:rrulestr-tzid-not-a-path

Conversation

@varun3257

Copy link
Copy Markdown

Summary of changes

rrulestr resolves the TZID parameter of DTSTART, RDATE and EXDATE with tz.gettz when no tzids argument is given. gettz also accepts file names: an absolute path is opened directly, and a relative key such as ../../etc/localtime was deprecated in #1130 because it loads a file from anywhere on the file system, but absolute paths were left as they are. A TZID in a rule string is an identifier like America/New_York, and rule strings often come from calendar data the application did not write, so the input decides which file the library opens: DTSTART;TZID=/etc/localtime:20200101T000000 parses with the host's local zone attached, TZID=/etc/passwd raises ValueError: Invalid TZif file: magic not found while TZID=/etc/does-not-exist parses as a naive datetime, which tells the caller whether a file exists, and TZID=../../etc/localtime raises DeprecatedTzKeyWarning out of the parser. The default lookup now goes through a small helper that treats an absolute path, or a key that leaves TZPATH, as a zone that is not found, so the datetime is naive, as it is for any other unknown TZID. Callers that pass their own tzids mapping or callable are not affected, and neither is gettz itself. The new tests fail on master and pass with this change; the full suite and darker are green locally, apart from test_dst_matches_tzdata_zi[tzpath], which already fails against this machine's system tzdata on master. The changelog fragment is an orphan fragment (+rrulestr-tzid-path.bugfix.rst) because the PR number was not known when the branch was made; I can rename it to the PR number if you prefer.

Pull Request Checklist

  • Changes have tests
  • Authors have been added to AUTHORS.md
  • News fragment added in changelog.d. See CONTRIBUTING.md for details

`rrulestr` resolves the `TZID` parameter of `DTSTART`, `RDATE` and
`EXDATE` with `tz.gettz` when no `tzids` argument is given. `gettz` also
accepts file names: an absolute path is opened directly, and a relative
key like `"../../etc/localtime"` was deprecated for this reason, but
absolute paths were left as they are. A `TZID` in a rule string is an
identifier like `"America/New_York"`, and rule strings often come from
calendar data the application did not write, so
`DTSTART;TZID=/etc/localtime:...` parsed with the host's local zone
attached, `TZID=/etc/passwd` raised `ValueError` while a path that does
not exist parsed as a naive datetime, and `TZID=../../etc/localtime`
raised `DeprecatedTzKeyWarning` out of the parser.

The default lookup now treats an absolute path, or a key that leaves
`TZPATH`, as a zone that is not found, so the result is naive, as it is
for any other unknown `TZID`. Callers that pass their own `tzids` are
not affected, and neither is `gettz` itself.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant