Skip to content

gh-146092: Raise MemoryError on allocation failure in _zoneinfo#146165

Open
vstinner wants to merge 2 commits intopython:mainfrom
vstinner:zoneinfo_memoroyerror
Open

gh-146092: Raise MemoryError on allocation failure in _zoneinfo#146165
vstinner wants to merge 2 commits intopython:mainfrom
vstinner:zoneinfo_memoroyerror

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Mar 19, 2026 •
edited by bedevere-app bot
Loading

@vstinner vstinner added skip news needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Mar 19, 2026
@bedevere-app bedevere-app bot mentioned this pull request Mar 19, 2026
Open
StanFromIreland
StanFromIreland reviewed Mar 19, 2026
View reviewed changes
Copy link
Member

@StanFromIreland StanFromIreland left a comment

Choose a reason for hiding this comment

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

Please also fix:

https://github.com/python/cpython/blob/main/Modules/_zoneinfo.c#L1872-L1875

Modules/_zoneinfo.c Outdated

DayRule *rv = PyMem_Calloc(1, sizeof(DayRule));
if (rv == NULL) {
PyErr_NoMemory();
Copy link
Member

@StanFromIreland StanFromIreland Mar 19, 2026

Choose a reason for hiding this comment

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

parse_tz_str will then also raise a PyExc_ValueError, this will be confusing for users.

Copy link
Member Author

@vstinner vstinner Mar 19, 2026

Choose a reason for hiding this comment

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

Oh, I didn't want to raise MemoryError in parse_transition_rule(). I revert this change.

@vstinner
Copy link
Member Author

vstinner commented Mar 19, 2026

Please also fix: https://github.com/python/cpython/blob/main/Modules/_zoneinfo.c#L1872-L1875

parse_transition_rule() is only used in parse_tz_str() and parse_tz_str() raises ValueError on parse_transition_rule() failure. Other parse_transition_rule() error paths don't set an exception. So I didn't add PyErr_NoMemory() in parse_transition_rule().

@StanFromIreland
Copy link
Member

StanFromIreland commented Mar 19, 2026

So I didn't add PyErr_NoMemory() in parse_transition_rule().

Can you please clarify why? The ValueError message may be misleading: "Malformed transition rule in TZ string"
I would update the check to only raise the ValueError if no other exception is already set.

@vstinner
Copy link
Member Author

vstinner commented Mar 19, 2026

I would update the check to only raise the ValueError if no other exception is already set.

I tried that but I ended up with strange exceptions from a subfunction.

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

Reviewers

@StanFromIreland StanFromIreland StanFromIreland left review comments

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

@pganssle pganssle Awaiting requested review from pganssle pganssle is a code owner

Assignees

No one assigned

Labels

awaiting merge needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vstinner @StanFromIreland @kumaraditya303