Repository navigation
datetime.fromisoformat() parses offset minutes outside 00-59 range #126883
Description
Bug report
Bug description:
>>> datetime.fromisoformat('2020-01-01T00:00+00:90')
datetime.datetime(2020, 1, 1, 0, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=5400)))expected result: ValueError (90 minutes is out of range 00-59)
I wasn't able to find a definitive paragraph of the ISO8601 to quote (it's not an open standard), but it appears datetime libraries in other languages (Temporal, NodaTime) do enforce this. Also, RFC3339 does explicitly forbid offset minutes outside the 00-59 range.
What are your thoughts?
CPython versions tested on:
3.13
Operating systems tested on:
macOS
Activity
donbarbos commentedon Nov 20, 2024
I understand that the solution to this problem still requires discussion, as it may break backward compatibility.
But I decided to save some information about this module here for now.
When we get this problem:
time.fromisoformat()datetime.fromisoformat()The
fromisoformat()functions returnsclsso afterfromisoformat()we call__new__We have function to check input time data:
_check_time_fields(hour, minute, second, microsecond, fold), but this function is called when creating a new instance in__new__methods (in classestimeanddatetime). And we can't call_check_time_fieldsfor tzinfo.Inside the
__new__method we have access to the already formedtzinfoobject of thetimedeltaclass (it can also beNone, but this is not our case). This object is formed when returning from_parse_isoformat_timewhich we call in thefromisoformatmethod. Also,tzinfoonly stores information aboutdays,secondsandmilliseconds, the rest of the fields have not been saved since the object was created.So we can start checking time fields inside
_parse_isoformat_time(at this moment we know tz info fields) but then we will getValueError(f'Invalid isoformat string: {time_string!r}')every time the fields go beyond their range because all exceptions from_parse_isoformat_timeare intercepted andValueError(f'Invalid isoformat string: {time_string!r}')is returned.And if no one has revivals, I'd like to implement patch.
donbarbos commentedon Nov 23, 2024
I have a suggestion to call
_check_time_fields(hour, minute, second, microsecond, fold)insidetimedelta.__new__(as it is done in thetime,datetimeclasses).But this will have consequences. There will be restrictions for
timedeltaobject:ariebovenberg commentedon Nov 24, 2024
@donbarbos we'd have to think of something else.
timedelta(minutes=90)is perfectly valid and should remain so. The restrictions are limited to the parsing of UTC offsets. Sounds likeparse_isoformat_timeis where the fix should be?donbarbos commentedon Nov 24, 2024
@ariebovenberg sorry for my inattention but we need to look at the C implementation to solve this problem. And the one in
Lib/_pydatetime.pyneeds to be tidied up so there are no discrepancies. I'll soon open an issue to draw attention to the Python implementation, as I've noticed that the Python and C implementations of the library have differences and_pydatetime.pycontains a lot of legacy that is poorly maintained, unlike the main implementation (this is inModules/_datetimemodule.c)Regarding where to check time fields, I completely agree with you, as it is more logical.
gh-126883: Add check that timezone fields are in range for `datetime.…
donbarbos commentedon May 19, 2025
i think we can close this issue
cc @pganssle
pythongh-126883: Add check that timezone fields are in range for `dat…
pythongh-126883: Add check that timezone fields are in range for `dat…