Skip to content

gh-85702: Catch PermissionError in zoneinfo.load_tzdata() #136117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Jun 30, 2025
Merged

gh-85702: Catch PermissionError in zoneinfo.load_tzdata() #136117

merged 3 commits into from
Jun 30, 2025

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jun 30, 2025 •
edited by bedevere-app bot
Loading

@vstinner vstinner added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Jun 30, 2025
@bedevere-app bedevere-app bot mentioned this pull request Jun 30, 2025
Open
@vstinner
Copy link
Member Author

vstinner commented Jun 30, 2025 •
edited
Loading

On Windows, test_zoneinfo.test_bad_keys() fails without this fix if tzdata is installed.

vstinner@WIN C:\victor\python\main>env\scripts\python -m test test_zoneinfo -m test_bad_keys -v
...
ERROR: test_bad_keys (test.test_zoneinfo.test_zoneinfo.CTZDataTests.test_bad_keys)  
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\victor\python\main\Lib\test\test_zoneinfo\test_zoneinfo.py", line 231, in
 test_bad_keys
    self.klass(bad_key)
    ~~~~~~~~~~^^^^^^^^^
  File "C:\victor\python\main\Lib\zoneinfo\_common.py", line 12, in load_tzdata     
    return resources.files(package_name).joinpath(resource_name).open("rb")
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  File "C:\victor\python\main\Lib\pathlib\__init__.py", line 777, in open
    return io.open(self, mode, buffering, encoding, errors, newline)
           ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
PermissionError: [Errno 13] Permission denied: 'C:\\victor\\python\\main\\env\\Lib\\site-packages\\tzdata\\zoneinfo\\Europe'
...

test_zoneinfo.test_bad_keys() pass with this fix (if tzdata is installed).

vstinner@WIN C:\victor\python\main>env\scripts\python -m test test_zoneinfo -m test_bad_keys -v
...
Result: SUCCESS

@vstinner
Copy link
Member Author

cc @StanFromIreland

StanFromIreland
StanFromIreland approved these changes Jun 30, 2025
View reviewed changes
Copy link
Member

@StanFromIreland StanFromIreland left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Test fail is unrelated, and there is no need to add another test since this is covered by existing ones as you noted. Though I would not mind if there was a comment explaining why this is done. e.g.

# Workaround for Windows, see gh-85702

@vstinner
Copy link
Member Author

Though I would not mind if there was a comment explaining why this is done.

I added a comment.

ZeroIntensity
ZeroIntensity reviewed Jun 30, 2025
View reviewed changes
Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be a test for this on Windows.

@StanFromIreland
Copy link
Member

@ZeroIntensity

There already was, see Victor's comment: #136117 (comment)

So I assume we have no CI/Buildbots with tzdata installed.

@ZeroIntensity
Copy link
Member

Yeah, I know, intuition is telling me that we don't want tests that incidentally catch a bug. But I didn't realize that there aren't buildbots with tzdata installed, so I guess it's a moot point.

@vstinner
Copy link
Member Author

I think there should be a test for this on Windows.

There is already test_zoneinfo.test_bad_keys().

ZeroIntensity
ZeroIntensity approved these changes Jun 30, 2025
View reviewed changes
Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as well. Looking at the test, it does seem to be exercising exactly what I'd wanted. We should probably try to get tzdata installed on one of the Windows buildbots.

@StanFromIreland
Copy link
Member

I hope I get a definitive reply to my question on the Discord... I can install it on mine but I will probably have to drop it to unstable.

@vstinner vstinner merged commit ee47670 into python:main Jun 30, 2025
44 checks passed
@vstinner vstinner deleted the zoneinfo_is_dir branch June 30, 2025 14:33
@miss-islington-app
Copy link

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 30, 2025
@vstinner @ZeroIntensity @miss-islington
pythongh-85702: Catch PermissionError in zoneinfo.load_tzdata() (pyth…
5304d9a 
…onGH-136117)

(cherry picked from commit ee47670)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
@miss-islington-app
Copy link

Sorry, @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker ee47670e8b8648b14fd4cb64a9d47d6ed3c5b6b7 3.13

@bedevere-app
Copy link

bedevere-app bot commented Jun 30, 2025

GH-136128 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jun 30, 2025
vstinner added a commit that referenced this pull request Jun 30, 2025
@miss-islington @vstinner @ZeroIntensity
[3.14] gh-85702: Catch PermissionError in zoneinfo.load_tzdata() (GH-…
e86ac72 
…136117) (#136128)

gh-85702: Catch PermissionError in zoneinfo.load_tzdata() (GH-136117)
(cherry picked from commit ee47670)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
@vstinner vstinner removed the needs backport to 3.13 bugs and security fixes label Jun 30, 2025
@vstinner vstinner added the needs backport to 3.13 bugs and security fixes label Jun 30, 2025
@miss-islington-app
Copy link

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 30, 2025
@vstinner @ZeroIntensity @miss-islington
pythongh-85702: Catch PermissionError in zoneinfo.load_tzdata() (pyth…
15686ee 
…onGH-136117)

(cherry picked from commit ee47670)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jun 30, 2025

GH-136136 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jun 30, 2025
vstinner added a commit that referenced this pull request Jun 30, 2025
@miss-islington @vstinner @ZeroIntensity
[3.13] gh-85702: Catch PermissionError in zoneinfo.load_tzdata() (GH-…
e194793 
…136117) (#136136)

gh-85702: Catch PermissionError in zoneinfo.load_tzdata() (GH-136117)
(cherry picked from commit ee47670)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZeroIntensity ZeroIntensity ZeroIntensity approved these changes

@StanFromIreland StanFromIreland StanFromIreland approved these changes

Assignees

@vstinner vstinner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vstinner @StanFromIreland @ZeroIntensity