Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Error condition in _datetimemodule.c is unchecked with datetime.fromtimestamp on local times > Y10k #91581

Closed
pganssle opened this issue Apr 15, 2022 · 0 comments · Fixed by #91582
Closed

Error condition in _datetimemodule.c is unchecked with datetime.fromtimestamp on local times > Y10k #91581

pganssle opened this issue Apr 15, 2022 · 0 comments · Fixed by #91582

Comments

@pganssle
Copy link
Member

On twitter, someone asked about a weird difference in the error messages from datetime.fromtimestamp() when called with a timestamp that falls on 10000-01-01 when compared to 10000-01-02. Compare:

$ TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ValueError: year 0 is out of range

$ TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1 + 24 * 3600)"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ValueError: year 10000 is out of range

Turns out that this is because in the localtime-specific implementation of datetime.fromtimestamp in _datetimemodule.c, there's an unchecked error condition when calling utc_to_seconds, which fails with -1.

As a practical matter, this always results in basically the same error anyway, since in all situations where result_seconds gets set to -1 the subsequent calculations fail anyway, and the only difference is that the error message has a slightly different form, but we may as well fix this.
@pganssle pganssle mentioned this issue Apr 15, 2022
Merged
pganssle added a commit to pganssle/cpython that referenced this issue May 12, 2022
@pganssle
Check result of utc_to_seconds and skip fold probe in pure Python
55a77c1 
The `utc_to_seconds` call can fail, here's a minimal reproducer on
Linux:

TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)"

The old behavior still raised an error in a similar way, but only
because subsequent calculations happened to fail as well. Better to fail
fast.

This also refactors the tests to split out the `fromtimestamp` and
`utcfromtimestamp` tests, and to get us closer to the actual desired
limits of the functions. As part of this, we also changed the way we
detect platforms where the same limits don't necessarily apply (e.g.
Windows).

As part of refactoring the tests to hit this condition explicitly (even
though the user-facing behvior doesn't change in any way we plan to
guarantee), I noticed that there was a difference in the places that
`datetime.utcfromtimestamp` fails in the C and pure Python versions, which
was fixed by skipping the "probe for fold" logic for UTC specifically —
since UTC doesn't have any folds or gaps, we were never going to find a
fold value anyway. This should prevent some failures in the pure python
`utcfromtimestamp` method on timestamps close to 0001-01-01.

There are two separate news entries for this because one is a
potentially user-facing change, the other is an internal code
correctness change that, if anything, changes some error messages. The
two happen to be coupled because of the test refactoring, but they are
probably best thought of as independent changes.

Fixes pythonGH-91581
pganssle added a commit that referenced this issue May 12, 2022
@pganssle
Check result of utc_to_seconds and skip fold probe in pure Python (#9…
83c0247 
…1582)

The `utc_to_seconds` call can fail, here's a minimal reproducer on
Linux:

TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)"

The old behavior still raised an error in a similar way, but only
because subsequent calculations happened to fail as well. Better to fail
fast.

This also refactors the tests to split out the `fromtimestamp` and
`utcfromtimestamp` tests, and to get us closer to the actual desired
limits of the functions. As part of this, we also changed the way we
detect platforms where the same limits don't necessarily apply (e.g.
Windows).

As part of refactoring the tests to hit this condition explicitly (even
though the user-facing behvior doesn't change in any way we plan to
guarantee), I noticed that there was a difference in the places that
`datetime.utcfromtimestamp` fails in the C and pure Python versions, which
was fixed by skipping the "probe for fold" logic for UTC specifically —
since UTC doesn't have any folds or gaps, we were never going to find a
fold value anyway. This should prevent some failures in the pure python
`utcfromtimestamp` method on timestamps close to 0001-01-01.

There are two separate news entries for this because one is a
potentially user-facing change, the other is an internal code
correctness change that, if anything, changes some error messages. The
two happen to be coupled because of the test refactoring, but they are
probably best thought of as independent changes.

Fixes GH-91581
@miss-islington miss-islington mentioned this issue May 12, 2022
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 12, 2022
@pganssle @miss-islington
Check result of utc_to_seconds and skip fold probe in pure Python (py…
620ee1c 
…thonGH-91582)

The `utc_to_seconds` call can fail, here's a minimal reproducer on
Linux:

TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)"

The old behavior still raised an error in a similar way, but only
because subsequent calculations happened to fail as well. Better to fail
fast.

This also refactors the tests to split out the `fromtimestamp` and
`utcfromtimestamp` tests, and to get us closer to the actual desired
limits of the functions. As part of this, we also changed the way we
detect platforms where the same limits don't necessarily apply (e.g.
Windows).

As part of refactoring the tests to hit this condition explicitly (even
though the user-facing behvior doesn't change in any way we plan to
guarantee), I noticed that there was a difference in the places that
`datetime.utcfromtimestamp` fails in the C and pure Python versions, which
was fixed by skipping the "probe for fold" logic for UTC specifically —
since UTC doesn't have any folds or gaps, we were never going to find a
fold value anyway. This should prevent some failures in the pure python
`utcfromtimestamp` method on timestamps close to 0001-01-01.

There are two separate news entries for this because one is a
potentially user-facing change, the other is an internal code
correctness change that, if anything, changes some error messages. The
two happen to be coupled because of the test refactoring, but they are
probably best thought of as independent changes.

Fixes pythonGH-91581
(cherry picked from commit 83c0247)

Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
@miss-islington miss-islington mentioned this issue May 12, 2022
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 12, 2022
@pganssle @miss-islington
Check result of utc_to_seconds and skip fold probe in pure Python (py…
250444e 
…thonGH-91582)

The `utc_to_seconds` call can fail, here's a minimal reproducer on
Linux:

TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)"

The old behavior still raised an error in a similar way, but only
because subsequent calculations happened to fail as well. Better to fail
fast.

This also refactors the tests to split out the `fromtimestamp` and
`utcfromtimestamp` tests, and to get us closer to the actual desired
limits of the functions. As part of this, we also changed the way we
detect platforms where the same limits don't necessarily apply (e.g.
Windows).

As part of refactoring the tests to hit this condition explicitly (even
though the user-facing behvior doesn't change in any way we plan to
guarantee), I noticed that there was a difference in the places that
`datetime.utcfromtimestamp` fails in the C and pure Python versions, which
was fixed by skipping the "probe for fold" logic for UTC specifically —
since UTC doesn't have any folds or gaps, we were never going to find a
fold value anyway. This should prevent some failures in the pure python
`utcfromtimestamp` method on timestamps close to 0001-01-01.

There are two separate news entries for this because one is a
potentially user-facing change, the other is an internal code
correctness change that, if anything, changes some error messages. The
two happen to be coupled because of the test refactoring, but they are
probably best thought of as independent changes.

Fixes pythonGH-91581
(cherry picked from commit 83c0247)

Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
@miss-islington miss-islington mentioned this issue May 12, 2022
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 12, 2022
@pganssle @miss-islington
Check result of utc_to_seconds and skip fold probe in pure Python (py…
4ff3ac0 
…thonGH-91582)

The `utc_to_seconds` call can fail, here's a minimal reproducer on
Linux:

TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)"

The old behavior still raised an error in a similar way, but only
because subsequent calculations happened to fail as well. Better to fail
fast.

This also refactors the tests to split out the `fromtimestamp` and
`utcfromtimestamp` tests, and to get us closer to the actual desired
limits of the functions. As part of this, we also changed the way we
detect platforms where the same limits don't necessarily apply (e.g.
Windows).

As part of refactoring the tests to hit this condition explicitly (even
though the user-facing behvior doesn't change in any way we plan to
guarantee), I noticed that there was a difference in the places that
`datetime.utcfromtimestamp` fails in the C and pure Python versions, which
was fixed by skipping the "probe for fold" logic for UTC specifically —
since UTC doesn't have any folds or gaps, we were never going to find a
fold value anyway. This should prevent some failures in the pure python
`utcfromtimestamp` method on timestamps close to 0001-01-01.

There are two separate news entries for this because one is a
potentially user-facing change, the other is an internal code
correctness change that, if anything, changes some error messages. The
two happen to be coupled because of the test refactoring, but they are
probably best thought of as independent changes.

Fixes pythonGH-91581
(cherry picked from commit 83c0247)

Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
miss-islington added a commit that referenced this issue May 14, 2022
@miss-islington @pganssle
Check result of utc_to_seconds and skip fold probe in pure Python (GH…
dae3e2f 
…-91582)

The `utc_to_seconds` call can fail, here's a minimal reproducer on
Linux:

TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)"

The old behavior still raised an error in a similar way, but only
because subsequent calculations happened to fail as well. Better to fail
fast.

This also refactors the tests to split out the `fromtimestamp` and
`utcfromtimestamp` tests, and to get us closer to the actual desired
limits of the functions. As part of this, we also changed the way we
detect platforms where the same limits don't necessarily apply (e.g.
Windows).

As part of refactoring the tests to hit this condition explicitly (even
though the user-facing behvior doesn't change in any way we plan to
guarantee), I noticed that there was a difference in the places that
`datetime.utcfromtimestamp` fails in the C and pure Python versions, which
was fixed by skipping the "probe for fold" logic for UTC specifically —
since UTC doesn't have any folds or gaps, we were never going to find a
fold value anyway. This should prevent some failures in the pure python
`utcfromtimestamp` method on timestamps close to 0001-01-01.

There are two separate news entries for this because one is a
potentially user-facing change, the other is an internal code
correctness change that, if anything, changes some error messages. The
two happen to be coupled because of the test refactoring, but they are
probably best thought of as independent changes.

Fixes GH-91581
(cherry picked from commit 83c0247)

Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
miss-islington added a commit that referenced this issue May 14, 2022
@miss-islington @pganssle
Check result of utc_to_seconds and skip fold probe in pure Python (GH…
36d42e7 
…-91582)

The `utc_to_seconds` call can fail, here's a minimal reproducer on
Linux:

TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)"

The old behavior still raised an error in a similar way, but only
because subsequent calculations happened to fail as well. Better to fail
fast.

This also refactors the tests to split out the `fromtimestamp` and
`utcfromtimestamp` tests, and to get us closer to the actual desired
limits of the functions. As part of this, we also changed the way we
detect platforms where the same limits don't necessarily apply (e.g.
Windows).

As part of refactoring the tests to hit this condition explicitly (even
though the user-facing behvior doesn't change in any way we plan to
guarantee), I noticed that there was a difference in the places that
`datetime.utcfromtimestamp` fails in the C and pure Python versions, which
was fixed by skipping the "probe for fold" logic for UTC specifically —
since UTC doesn't have any folds or gaps, we were never going to find a
fold value anyway. This should prevent some failures in the pure python
`utcfromtimestamp` method on timestamps close to 0001-01-01.

There are two separate news entries for this because one is a
potentially user-facing change, the other is an internal code
correctness change that, if anything, changes some error messages. The
two happen to be coupled because of the test refactoring, but they are
probably best thought of as independent changes.

Fixes GH-91581
(cherry picked from commit 83c0247)

Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
ambv pushed a commit that referenced this issue May 16, 2022
@miss-islington @pganssle
Check result of utc_to_seconds and skip fold probe in pure Python (GH…
1699a5e 
…-91582) (GH-92748)

The `utc_to_seconds` call can fail, here's a minimal reproducer on
Linux:

TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)"

The old behavior still raised an error in a similar way, but only
because subsequent calculations happened to fail as well. Better to fail
fast.

This also refactors the tests to split out the `fromtimestamp` and
`utcfromtimestamp` tests, and to get us closer to the actual desired
limits of the functions. As part of this, we also changed the way we
detect platforms where the same limits don't necessarily apply (e.g.
Windows).

As part of refactoring the tests to hit this condition explicitly (even
though the user-facing behvior doesn't change in any way we plan to
guarantee), I noticed that there was a difference in the places that
`datetime.utcfromtimestamp` fails in the C and pure Python versions, which
was fixed by skipping the "probe for fold" logic for UTC specifically —
since UTC doesn't have any folds or gaps, we were never going to find a
fold value anyway. This should prevent some failures in the pure python
`utcfromtimestamp` method on timestamps close to 0001-01-01.

There are two separate news entries for this because one is a
potentially user-facing change, the other is an internal code
correctness change that, if anything, changes some error messages. The
two happen to be coupled because of the test refactoring, but they are
probably best thought of as independent changes.

Fixes GH-91581
(cherry picked from commit 83c0247)

Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
hello-adam pushed a commit to hello-adam/cpython that referenced this issue Jun 2, 2022
@miss-islington @pganssle @hello-adam
Check result of utc_to_seconds and skip fold probe in pure Python (py…
7609955 
…thonGH-91582) (pythonGH-92748)

The `utc_to_seconds` call can fail, here's a minimal reproducer on
Linux:

TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)"

The old behavior still raised an error in a similar way, but only
because subsequent calculations happened to fail as well. Better to fail
fast.

This also refactors the tests to split out the `fromtimestamp` and
`utcfromtimestamp` tests, and to get us closer to the actual desired
limits of the functions. As part of this, we also changed the way we
detect platforms where the same limits don't necessarily apply (e.g.
Windows).

As part of refactoring the tests to hit this condition explicitly (even
though the user-facing behvior doesn't change in any way we plan to
guarantee), I noticed that there was a difference in the places that
`datetime.utcfromtimestamp` fails in the C and pure Python versions, which
was fixed by skipping the "probe for fold" logic for UTC specifically —
since UTC doesn't have any folds or gaps, we were never going to find a
fold value anyway. This should prevent some failures in the pure python
`utcfromtimestamp` method on timestamps close to 0001-01-01.

There are two separate news entries for this because one is a
potentially user-facing change, the other is an internal code
correctness change that, if anything, changes some error messages. The
two happen to be coupled because of the test refactoring, but they are
probably best thought of as independent changes.

Fixes pythonGH-91581
(cherry picked from commit 83c0247)

Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

gh-91581: Check result of utc_to_seconds pganssle/cpython
1 participant
@pganssle