bpo-43295: Fix error handling of datetime.strptime format string '%z' #24627

noormichael · 2021-02-23T05:08:07Z

Previously, datetime.strptime would match 'z' with the format string '%z' (for UTC offsets), throwing an IndexError by erroneously trying to parse 'z' as a timestamp. As a special case, '%z' matches the string 'Z' which is equivalent to the offset '+00:00', however this behavior is not defined for lowercase 'z'.

This change ensures a ValueError is thrown when encountering the original example, as follows:

>>> from datetime import datetime
>>> datetime.strptime('z', '%z')
ValueError: time data 'z' does not match format '%z'

https://bugs.python.org/issue43295

Automerge-Triggered-By: GH:pganssle

The text was updated successfully, but these errors were encountered:

the-knights-who-say-ni · 2021-02-23T05:08:10Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@noormichael

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

pganssle

Thanks for the quick fix on this! Just a few minor changes.

pganssle · 2021-02-26T16:34:20Z

Lib/test/test_strptime.py

+    def test_timezone_offset_cannot_parse_z(self):
+        # Check ValueError is raised when matching "z" (ordinary letter)
+        # with "%z" (timezone offset)
+        self.assertRaises(ValueError, _strptime._strptime_time, "z", "%z")


Two things here:

I much prefer the context manager version of assertRaises, since it's much clearer what calls are being tested.

It would be preferable to test this using only the public interface. I realize that there are other uses of _strptime._strptime_time in this file, but going forward I'd like to keep the tests focused on the public interface, so can you try and exercise this using time.strptime and/or datetime.strptime?

pganssle · 2021-02-26T16:34:20Z

Misc/NEWS.d/next/Library/2021-02-22-22-54-40.bpo-43295.h_ffu7.rst

@@ -0,0 +1,10 @@
+Previously, datetime.strptime would match 'z' with the format string '%z'


This is a very good summary of the issue, but it is long for a news stub. They're usually 1 or two lines like this: https://docs.python.org/3/whatsnew/changelog.html

How about:

Previously, :meth:`datetime.datetime.strptime` would fail with `IndexError` when the string `'z'` was found for the `%z` format specifier. This has been fixed to properly raise `ValueError` instead.

(You'll want to check to make sure that the :meth: directive works correctly, I may have made a mistake with that.
A BPO link is automatically generated, and users who want more information can click on it for an MWE and such.

bedevere-bot · 2021-02-26T16:34:23Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

noormichael · 2021-03-03T04:51:02Z

Is there a way to re-run the tests? test_ssl failed on Ubuntu which has passed on previous runs. In any case, I've made the requested changes. Could you please review this again?

pganssle

@noormichael No need to worry about that. I believe this is something to do with github upgrading the default Ubuntu container from 18.04 to 20.04 and some patch in Ubuntu that breaks OpenSSL. See this bug for more details.

I believe no further action on your part is necessary, this looks good and we can probably wait until that bug is resolved to merge so that backports will go more easily. Thanks for your contribution to CPython!

…%z' (GH-43295)

miss-islington · 2021-03-03T16:58:56Z

@noormichael: Status check is done, and it's a success ✅ .

miss-islington · 2021-03-03T16:59:00Z

Thanks @noormichael for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

bedevere-bot · 2021-03-03T16:59:18Z

GH-24728 is a backport of this pull request to the 3.9 branch.

…pythonGH-24627) Previously, `datetime.strptime` would match `'z'` with the format string `'%z'` (for UTC offsets), throwing an `IndexError` by erroneously trying to parse `'z'` as a timestamp. As a special case, `'%z'` matches the string `'Z'` which is equivalent to the offset `'+00:00'`, however this behavior is not defined for lowercase `'z'`. This change ensures a `ValueError` is thrown when encountering the original example, as follows: ``` >>> from datetime import datetime >>> datetime.strptime('z', '%z') ValueError: time data 'z' does not match format '%z' ``` Automerge-Triggered-By: GH:pganssle (cherry picked from commit 04f6fbb) Co-authored-by: Noor Michael <nsmichael31@gmail.com>

bedevere-bot · 2021-03-03T16:59:30Z

GH-24729 is a backport of this pull request to the 3.8 branch.

…pythonGH-24627) Previously, `datetime.strptime` would match `'z'` with the format string `'%z'` (for UTC offsets), throwing an `IndexError` by erroneously trying to parse `'z'` as a timestamp. As a special case, `'%z'` matches the string `'Z'` which is equivalent to the offset `'+00:00'`, however this behavior is not defined for lowercase `'z'`. This change ensures a `ValueError` is thrown when encountering the original example, as follows: ``` >>> from datetime import datetime >>> datetime.strptime('z', '%z') ValueError: time data 'z' does not match format '%z' ``` Automerge-Triggered-By: GH:pganssle (cherry picked from commit 04f6fbb) Co-authored-by: Noor Michael <nsmichael31@gmail.com>

Immorningstar · 2021-03-03T17:01:21Z

Plz don’t mail me I’m not interested in anything

…

Sent from my iPhone

On 03-Mar-2021, at 10:30 PM, Bedevere (bot) ***@***.***> wrote: GH-24729 is a backport of this pull request to the 3.8 branch. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

…pythonGH-24627) Previously, `datetime.strptime` would match `'z'` with the format string `'%z'` (for UTC offsets), throwing an `IndexError` by erroneously trying to parse `'z'` as a timestamp. As a special case, `'%z'` matches the string `'Z'` which is equivalent to the offset `'+00:00'`, however this behavior is not defined for lowercase `'z'`. This change ensures a `ValueError` is thrown when encountering the original example, as follows: ``` >>> from datetime import datetime >>> datetime.strptime('z', '%z') ValueError: time data 'z' does not match format '%z' ``` Automerge-Triggered-By: GH:pganssle

miss-islington · 2021-04-28T16:23:27Z

Thanks @noormichael for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

bedevere-bot · 2021-04-28T16:23:35Z

GH-25695 is a backport of this pull request to the 3.9 branch.

…pythonGH-24627) Previously, `datetime.strptime` would match `'z'` with the format string `'%z'` (for UTC offsets), throwing an `IndexError` by erroneously trying to parse `'z'` as a timestamp. As a special case, `'%z'` matches the string `'Z'` which is equivalent to the offset `'+00:00'`, however this behavior is not defined for lowercase `'z'`. This change ensures a `ValueError` is thrown when encountering the original example, as follows: ``` >>> from datetime import datetime >>> datetime.strptime('z', '%z') ValueError: time data 'z' does not match format '%z' ``` Automerge-Triggered-By: GH:pganssle (cherry picked from commit 04f6fbb) Co-authored-by: Noor Michael <nsmichael31@gmail.com>

…pythonGH-24627) Previously, `datetime.strptime` would match `'z'` with the format string `'%z'` (for UTC offsets), throwing an `IndexError` by erroneously trying to parse `'z'` as a timestamp. As a special case, `'%z'` matches the string `'Z'` which is equivalent to the offset `'+00:00'`, however this behavior is not defined for lowercase `'z'`. This change ensures a `ValueError` is thrown when encountering the original example, as follows: ``` >>> from datetime import datetime >>> datetime.strptime('z', '%z') ValueError: time data 'z' does not match format '%z' ``` Automerge-Triggered-By: GH:pganssle

…GH-24627) (#25695) Previously, `datetime.strptime` would match `'z'` with the format string `'%z'` (for UTC offsets), throwing an `IndexError` by erroneously trying to parse `'z'` as a timestamp. As a special case, `'%z'` matches the string `'Z'` which is equivalent to the offset `'+00:00'`, however this behavior is not defined for lowercase `'z'`. This change ensures a `ValueError` is thrown when encountering the original example, as follows: ``` >>> from datetime import datetime >>> datetime.strptime('z', '%z') ValueError: time data 'z' does not match format '%z' ``` Automerge-Triggered-By: GH:pganssle (cherry picked from commit 04f6fbb) Co-authored-by: Noor Michael <nsmichael31@gmail.com> Co-authored-by: Noor Michael <nsmichael31@gmail.com>

noormichael requested review from abalkin and pganssle as code owners Feb 23, 2021

the-knights-who-say-ni added the CLA not signed label Feb 23, 2021

bedevere-bot added awaiting review awaiting core review and removed awaiting review labels Feb 23, 2021

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Feb 23, 2021

serverwentdown approved these changes Feb 24, 2021

View changes

itchyny approved these changes Feb 26, 2021

View changes

pganssle requested changes Feb 26, 2021

View changes

bedevere-bot added awaiting changes and removed awaiting core review labels Feb 26, 2021

pganssle approved these changes Mar 3, 2021

View changes

bedevere-bot added awaiting merge and removed awaiting changes labels Mar 3, 2021

pganssle added needs backport to 3.8 needs backport to 3.9 labels Mar 3, 2021

noormichael added 6 commits Mar 3, 2021

bpo-43295: Throw ValueError when datetime.strptime matches 'z' with '…

e999f60

…%z' (GH-43295)

bpo-43295: Fix Documentation (GH-43295)

018b7f9

bpo-43295: Format Source Code in Documentation (GH-43295)

ab99696

bpo-43295: Addressed Feedback (GH-43295)

6ef4ebe

bpo-43295: Shorten Documentation (GH-43295)

2c0eee4

bpo-43295: Fix backquotes (GH-43295)

901c1fe

pganssle force-pushed the fix-issue-43295 branch from 5d47504 to 901c1fe Mar 3, 2021

pganssle added the 🤖 automerge label Mar 3, 2021

miss-islington merged commit 04f6fbb into python:master Mar 3, 2021
11 checks passed

bedevere-bot removed the awaiting merge label Mar 3, 2021

bedevere-bot removed the needs backport to 3.9 label Mar 3, 2021

bedevere-bot removed the needs backport to 3.8 label Mar 3, 2021

vstinner added the needs backport to 3.9 label Apr 28, 2021

bedevere-bot removed the needs backport to 3.9 label Apr 28, 2021

python / cpython Public

bpo-43295: Fix error handling of datetime.strptime format string '%z' #24627

bpo-43295: Fix error handling of datetime.strptime format string '%z' #24627

noormichael commented Feb 23, 2021 •

edited by miss-islington

the-knights-who-say-ni commented Feb 23, 2021

pganssle left a comment

pganssle Feb 26, 2021

pganssle Feb 26, 2021

bedevere-bot commented Feb 26, 2021

noormichael commented Mar 3, 2021

pganssle left a comment

miss-islington commented Mar 3, 2021

miss-islington commented Mar 3, 2021

bedevere-bot commented Mar 3, 2021

bedevere-bot commented Mar 3, 2021

Immorningstar commented Mar 3, 2021

miss-islington commented Apr 28, 2021

bedevere-bot commented Apr 28, 2021

		@@ -0,0 +1,10 @@
		Previously, datetime.strptime would match 'z' with the format string '%z'

python / cpython Public

bpo-43295: Fix error handling of datetime.strptime format string '%z' #24627

bpo-43295: Fix error handling of datetime.strptime format string '%z' #24627

Conversation

noormichael commented Feb 23, 2021 • edited by miss-islington

the-knights-who-say-ni commented Feb 23, 2021

CLA Missing

pganssle left a comment

pganssle Feb 26, 2021

pganssle Feb 26, 2021

bedevere-bot commented Feb 26, 2021

noormichael commented Mar 3, 2021

pganssle left a comment

miss-islington commented Mar 3, 2021

miss-islington commented Mar 3, 2021

bedevere-bot commented Mar 3, 2021

bedevere-bot commented Mar 3, 2021

Immorningstar commented Mar 3, 2021

miss-islington commented Apr 28, 2021

bedevere-bot commented Apr 28, 2021

noormichael commented Feb 23, 2021 •

edited by miss-islington