gh-152060: Fix datetime.fromisoformat() raising AssertionError in pure Python#152061
Open
tonghuaroot wants to merge 1 commit into
Open
gh-152060: Fix datetime.fromisoformat() raising AssertionError in pure Python#152061tonghuaroot wants to merge 1 commit into
tonghuaroot wants to merge 1 commit into
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}") |
Member
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.
Suggested change
| 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 |
Member
There was a problem hiding this comment.
Suggested change
| '2020-1234', # Ambiguous 9-char date portion |
These cases are identical.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.