Skip to content

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

Merged
vstinner merged 3 commits intopython:mainfrom
vstinner:zoneinfo_is_dir
Jun 30, 2025
Merged

gh-85702: Catch PermissionError in zoneinfo.load_tzdata()#136117
vstinner merged 3 commits intopython:mainfrom
vstinner:zoneinfo_is_dir

Conversation

@vstinner
Copy link
Copy Markdown
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
Closed
@vstinner
Copy link
Copy Markdown
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
Copy Markdown
Member Author

vstinner commented Jun 30, 2025

cc @StanFromIreland

StanFromIreland
StanFromIreland approved these changes Jun 30, 2025
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Member Author

vstinner commented Jun 30, 2025

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
Copy Markdown
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.

Comment thread Misc/NEWS.d/next/Library/2025-06-30-11-12-24.gh-issue-85702.0Lrbwu.rst Outdated
@StanFromIreland
Copy link
Copy Markdown
Member

StanFromIreland commented Jun 30, 2025

@ZeroIntensity

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

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

@ZeroIntensity
Copy link
Copy Markdown
Member

ZeroIntensity commented Jun 30, 2025

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
Copy Markdown
Member Author

vstinner commented Jun 30, 2025

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
Copy Markdown
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
Copy Markdown
Member

StanFromIreland commented Jun 30, 2025

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
Copy Markdown

miss-islington-app Bot commented Jun 30, 2025

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
Copy Markdown

miss-islington-app Bot commented Jun 30, 2025

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
Copy Markdown

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
Copy Markdown

miss-islington-app Bot commented Jun 30, 2025

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
Copy Markdown

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]>
AndPuQing pushed a commit to AndPuQing/cpython that referenced this pull request Jul 11, 2025
@vstinner @ZeroIntensity @AndPuQing
pythongh-85702: Catch PermissionError in zoneinfo.load_tzdata() (pyth…
702a874 
…on#136117)

Co-authored-by: Peter Bierma <[email protected]>
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@vstinner @ZeroIntensity @Pranjal095
pythongh-85702: Catch PermissionError in zoneinfo.load_tzdata() (pyth…
2bfa526 
…on#136117)

Co-authored-by: Peter Bierma <[email protected]>
picnixz pushed a commit to picnixz/cpython that referenced this pull request Jul 13, 2025
@vstinner @ZeroIntensity @picnixz
pythongh-85702: Catch PermissionError in zoneinfo.load_tzdata() (pyth…
36d2036 
…on#136117)

Co-authored-by: Peter Bierma <[email protected]>
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@vstinner @ZeroIntensity @taegyunkim
pythongh-85702: Catch PermissionError in zoneinfo.load_tzdata() (pyth…
0e24c41 
…on#136117)

Co-authored-by: Peter Bierma <[email protected]>
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@vstinner @ZeroIntensity @Agent-Hellboy
pythongh-85702: Catch PermissionError in zoneinfo.load_tzdata() (pyth…
1a38d95 
…on#136117)

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