Repository navigation
datetime.fromisoformat() accepts invalid ISO 8601 timestamps #115783
Description
Activity
I'd like to extend the list of accepted invalid timestamps:
2024-01-17T15:21:00-0800 2024-01-17T152100-08:00 20240117T15:21:00-08:00 2024-01-17T152100-0800 20240117T15:21:00-0800 20240117T152100-08:00All these should be rejected by
fromisoformat().
I can report however, that removing just a single dash or a single colon from the date or time is properly rejected.Reacted by Patrick LehmannIt is worth noting, the test_datetime defined within datetimetester.py test assumes inconsistent typing is acceptable. For example, '20250102T03:04:05,6' is considered an acceptable string to the datetime.isoformat() function.
We have been accepting several "wrong" formats for a while (inconstent expanding, though we check a few cases; missing separator; separator other than ["T"] etc.)... This will require deprecation if we even wish to implement it.
test_fromisoformat_datetime_examplescases that fail with a patch that restricts it to only proper ISO 8601 expandedstrings:It's a long list... (Not including other "wrong" cases)
cpython/Lib/test/datetimetester.py
Line 3394 in e82c2ca
cpython/Lib/test/datetimetester.py
Line 3396 in e82c2ca
cpython/Lib/test/datetimetester.py
Line 3481 in e82c2ca
cpython/Lib/test/datetimetester.py
Line 3484 in e82c2ca
cpython/Lib/test/datetimetester.py
Line 3487 in e82c2ca
cpython/Lib/test/datetimetester.py
Line 3497 in e82c2ca
cpython/Lib/test/datetimetester.py
Line 3501 in e82c2ca
cpython/Lib/test/datetimetester.py
Line 3505 in e82c2ca
cpython/Lib/test/datetimetester.py
Line 3507 in e82c2ca
Seeing as we seem to have been accepting these for so long and we even consider them valid in our tests I think this should be considered a feature request rather than a bug. I guess the decision is up to @pganssle, do you think we should restrict the function? I am happy to implement it.
The current documented scope of
fromisoformatis that it should parse the union of all valid ISO8601 strings and all possible outputs ofdatetime.isoformat, so I would consider anything outside of that as a bug, even if we included it in our test suite because we misunderstood the spec. If we were to change it, I wouldn't recommend doing it in a bugfix release, however.The options we have going forward are to document further extensions we support (i.e. mixed and matched separators), issue a deprecation cycle for invalid formats or just go ahead and break this.
Despite the fact that we have been supporting this for a while, I would be surprised if it is in particularly widespread use unless there is something out there generating these formats by default. They don't seem like the kind of format that you would choose to generate, unless maybe you are storing your strings in a place that can't accept : but can accept - or vice versa?
In any case, deprecation is probably the safest option. The argument for being strict is that you are supposed to be able to use
fromisoformatfor some kind of validation, and I've known people who use it that way. If we start emitting a warning, people can set the warning handler to error if they want that, or they can manipulate their strings. Of course, the down side is that people find deprecation warnings extremely annoying and tons of people don't bother handling them and they get passed on to end users who don't know what they mean, so it's not free to do that.In any case, deprecation is probably the safest option.
I agree.
you are supposed to be able to use fromisoformat for some kind of validation
It should also do what is in the name ;-) isoformat, not anyrandomformat
If we start emitting a warning, people can set the warning handler to error if they want that, or they can manipulate their strings.
We should do this. I can assign myself to this, I also propose extending the docs to be more specific and up to date with 8601:2
people find deprecation warnings extremely annoying and tons of people don't bother handling them and they get passed on to end users
Is this really that common an occurrence? They are warnings and not errors, most people do not check warnings. They will bother when it is no longer deprecated.
What may be interesting to note is that Temporal (the new datetime API for JS) chooses explicitly to allow mixing of extended and basic formats (tc39/proposal-temporal#2601)
@ariebovenberg thanks for sharing.
Here is my comment:
An appendix to a standard (here RFC 3339) is not normative wording. Also, just because some vendors of database systems can't read and understand a standard isn't a justification to undermine ISO 8601.In reference to tc39/proposal-temporal#2601 (comment), ISO has made an explicit change and added clarification to disallow mixed formats.
A similar bug has been found at https://www.utctime.net/ and was reported by email. This issues was gives as a reference.
Bug report
Bug description:
Since Python 3.11,
datetime.fromisoformat(...)accepts invalid timestamps. The same code was tested in many Python versions via GitHub Actions:Wrong format:
2024-01-17T15:21:00-0800Fixed format:
2024-01-17T15:21:00-08:00Python code to check:
ISO 8601 defines 2 formats:
When checking the EBNF rules, all parts of the timestamp must be with or without separators. A mix of formats is not allowed.
I haven't checked for other basic and extended format mixes.
My CI checked many Python versions and platforms. Here a failed CI pipeline, shows a correct implementation. A successful pipeline denotes a buggy Python version.
Other resources:
datetime.datetime.fromisoformat#107791CPython versions tested on:
3.8, 3.9, 3.10, 3.11, 3.12, 3.13
Operating systems tested on:
Linux, macOS, Windows
Linked PRs
datetime.fromisoformat#131522