gh-152060: Fix datetime.fromisoformat() raising AssertionError in pure Python#152061
Conversation
…in pure Python _pydatetime._parse_isoformat_date() asserted the date portion length, leaking a bare AssertionError out of datetime.fromisoformat() for some malformed strings (e.g. '2020-2020'), and behaving differently under -O. The C accelerator already raises ValueError. Replace the assert with an explicit ValueError so both implementations agree on all builds.
| # see the comment on Modules/_datetimemodule.c:_find_isoformat_datetime_separator | ||
| assert len(dtstr) in (7, 8, 10) | ||
| if len(dtstr) not in (7, 8, 10): | ||
| raise ValueError(f"Invalid isoformat string: {dtstr!r}") |
There was a problem hiding this comment.
{dtstr!r} won't match the C impl, but it doesn’t matter as fromisoformat re-raises anyway.
| raise ValueError(f"Invalid isoformat string: {dtstr!r}") | |
| raise ValueError(f"Invalid isoformat string") |
| '2009-04-19T12:30:45-00:90:00', # Time zone field out from range | ||
| '2009-04-19T12:30:45-00:00:90', # Time zone field out from range | ||
| '2020-2020', # Ambiguous 9-char date portion | ||
| '2020-1234', # Ambiguous 9-char date portion |
There was a problem hiding this comment.
| '2020-1234', # Ambiguous 9-char date portion |
These cases are identical.
StanFromIreland
added
needs backport to 3.13
|
Thanks @tonghuaroot for the PR, and @StanFromIreland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15. |
|
GH-152081 is a backport of this pull request to the 3.15 branch. |
|
Sorry, @tonghuaroot and @StanFromIreland, I could not cleanly backport this to |
bedevere-app
Bot
removed
the
needs backport to 3.15
|
Sorry, @tonghuaroot and @StanFromIreland, I could not cleanly backport this to |
|
@tonghuaroot are you able to do the backports? |
2 participants
Fixes #152060.
_pydatetime._parse_isoformat_date()asserted that the date portion has length 7, 8 or10. For some malformed inputs (e.g.
'2020-2020','2020-1234') the separator finderyields a 9-character date portion, so the assert fires and a bare
AssertionErrorescapesdatetime.fromisoformat(), which is documented to raise onlyValueError. The Caccelerator already raises
ValueError. Because the check is anassert, the input alsoraises
ValueErrorinstead underpython -O, so the behaviour differed by build flag.This replaces the assert with an explicit
ValueErrorwhose message matches the Cimplementation, so both implementations agree on every build. This is the sibling assert
site to the one fixed in #151771 (gh-151770).
date.fromisoformat()already validates the length before calling_parse_isoformat_date(), so it is unaffected.A regression test is added to
test_fromisoformat_fails_datetime, which runs under boththe C (
_Fast) and pure-Python (_Pure) test classes.