Skip to content

gh-151770: Fix datetime.fromisoformat() assertion on an out-of-range month with a 24:00 time#151771

Open
tonghuaroot wants to merge 3 commits into
python:mainfrom
tonghuaroot:gh-151770-fromiso-month-24h
Open

gh-151770: Fix datetime.fromisoformat() assertion on an out-of-range month with a 24:00 time#151771
tonghuaroot wants to merge 3 commits into
python:mainfrom
tonghuaroot:gh-151770-fromiso-month-24h

Conversation

@tonghuaroot

@tonghuaroot tonghuaroot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

datetime.fromisoformat() reached days_in_month() with month == 0 when the input combined an out-of-range (below 1) month with a 24:00 time, for example "2009-00-01T24:00:00". On a --with-pydebug build this tripped assert(month >= 1) in days_in_month (Modules/_datetimemodule.c); the pure-Python implementation in Lib/_pydatetime.py raised AssertionError on the same input. On a release build both already raised ValueError.

The 24:00 midnight-rollover guard validated only the upper month bound (month <= 12) before the days_in_month() call, and the parser does not range-check the month before that point, so a month below 1 slipped through. The matching upper-bound input "2009-13-01T24:00:00" was already rejected by the month <= 12 check.

This adds the missing month >= 1 lower bound to the guard in both implementations (mirroring the existing check_date_args validation), so an out-of-range month consistently raises ValueError. A regression test is added next to the existing out-of-range-month case in datetimetester.py.

…range month with a 24:00 time

The 24:00 midnight-rollover path validated only the upper month bound
before calling days_in_month(), so a month below 1 reached
assert(month >= 1) on a debug build (and AssertionError in the pure
Python implementation). Add the missing lower bound to both so an
out-of-range month consistently raises ValueError.

@StanFromIreland StanFromIreland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a few little nits, the core change is correct.

While we're dealing with this assert, can you please update it (on L58) like so:

- assert 1 <= month <= 12, month
+ assert 1 <= month <= 12, f"month must be in 1..12, not {month}"

Comment on lines +1 to +3
Fix :meth:`datetime.datetime.fromisoformat` raising an unexpected error on an
out-of-range month combined with a ``24:00`` time, such as
``"2009-00-01T24:00:00"``. It now consistently raises :exc:`ValueError`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Fix :meth:`datetime.datetime.fromisoformat` raising an unexpected error on an
out-of-range month combined with a ``24:00`` time, such as
``"2009-00-01T24:00:00"``. It now consistently raises :exc:`ValueError`.
Fix :meth:`datetime.datetime.fromisoformat` raising :exc:`AssertionError`
instead of :exc:`ValueError` for an out-of-range month combined with a
``24:00`` time.

Comment thread Lib/test/datetimetester.py Outdated
"2009-04-01T12:30:90", # Second out of range
"2009-04-01T12:90:45", # Minute out of range
"2009-04-01T25:30:45", # Hour out of range
"2009-00-01T24:00:00", # Month out of range (below)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"2009-00-01T24:00:00", # Month out of range (below)
"2009-00-01T24:00:00", # Month below range

@tonghuaroot

Copy link
Copy Markdown
Contributor Author

Thanks @StanFromIreland — applied all three.

Comment thread Modules/_datetimemodule.c Outdated

@StanFromIreland StanFromIreland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants