datetime.fromisoformat() parses offset minutes outside 00-59 range

[ariebovenberg]

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

Linked PRs

• gh-126883: Add check that timezone fields are in range for datetime.fromisoformat #127242

[donBarbos]

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:

1. time.fromisoformat()
2. datetime.fromisoformat()

The fromisoformat() functions returns cls so after fromisoformat() 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 classes time and datetime). And we can't call _check_time_fields for tzinfo.

Inside the __new__ method we have access to the already formed tzinfo object of the timedelta class (it can also be None, but this is not our case). This object is formed when returning from _parse_isoformat_time which we call in the fromisoformat method. Also, tzinfo only stores information about days, seconds and milliseconds, 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 get ValueError(f'Invalid isoformat string: {time_string!r}') every time the fields go beyond their range because all exceptions from _parse_isoformat_time are intercepted and ValueError(f'Invalid isoformat string: {time_string!r}') is returned.

And if no one has revivals, I'd like to implement patch.

[donBarbos]

I have a suggestion to call _check_time_fields(hour, minute, second, microsecond, fold) inside timedelta.__new__ (as it is done in the time, datetime classes).
But this will have consequences. There will be restrictions for timedelta object:

• hour must be in 0..23
• minute must be in 0..59
• second must be in 0..59
• microsecond must be in 0..999999

[ariebovenberg]

@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 like parse_isoformat_time is where the fix should be?

[donBarbos]

@ariebovenberg sorry for my inattention but we need to look at the C implementation to solve this problem. And the one in Lib/_pydatetime.py needs 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.py contains a lot of legacy that is poorly maintained, unlike the main implementation (this is in Modules/_datetimemodule.c)

Regarding where to check time fields, I completely agree with you, as it is more logical.