classification
Title: email.utils.formatdate uses unreliable time.timezone constant
Type: behavior Stage: resolved
Components: email, Library (Lib) Versions: Python 3.6, Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: akira, barry, belopolsky, berker.peksag, mitya57, python-dev, r.david.murray, rbcollins
Priority: normal Keywords: patch

Created on 2014-11-24 16:17 by mitya57, last changed 2015-07-31 20:23 by rbcollins. This issue is now closed.

Files
File name Uploaded Description Edit
issue22932.patch mitya57, 2014-11-24 17:17
issue22932_v2.patch mitya57, 2014-12-09 06:53
issue22932_v3.patch mitya57, 2014-12-09 16:03
issue22932_test.patch mitya57, 2015-01-11 11:39
issue22932_combined.diff mitya57, 2015-01-12 11:57 review
issue22932_combined_v2.diff mitya57, 2015-01-16 06:37 review
Messages (23)
msg231608 - (view) Author: Dmitry Shachnev (mitya57) * Date: 2014-11-24 16:17
The value of time.timezone may be wrong sometimes (see http://bugs.python.org/issue22752), so I think the email library should not use it:

$ python3 -c "from email.utils import formatdate; print(formatdate(localtime=True))"
Mon, 24 Nov 2014 19:16:32 +0400
$ date -R
Mon, 24 Nov 2014 19:16:32 +0300

Using something from datetime module may be a better solution.
msg231609 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2014-11-24 16:20
See also issue 665194.
msg231610 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2014-11-24 16:28
I was able to reproduce the problem on a Mac as follows:

$ TZ=Europe/Moscow date "+%c %z"
Mon Nov 24 19:27:51 2014 +0300
$ TZ=Europe/Moscow python3 -c "from email.utils import formatdate; print(formatdate(localtime=True))"
Mon, 24 Nov 2014 19:28:03 +0400
msg231611 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2014-11-24 16:31
"Using something from datetime module" works as expected:

$ TZ=Europe/Moscow python3 -c "from datetime import datetime, timezone; print(datetime.now(timezone.utc).astimezone())"
2014-11-24 19:30:27.141542+03:00
msg231613 - (view) Author: Dmitry Shachnev (mitya57) * Date: 2014-11-24 17:17
This patch fixes the issue for me.
msg231614 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-11-24 17:30
email.utils.format_datetime uses datetime.  formatdate wasn't touched, for backward compatibility reasons.  If it has an actual bug we can fix it.

If it can be converted to use datetime without sacrificing backward compatibility, that's also fine with me.

Frankly I don't know enough at the moment to know if the proposed patch is correct.  Alexander?
msg231615 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2014-11-24 17:43
The proposed patch will not work on platforms that don't support tm_gmtoff.  A proper fix may look like this:

dt = datetime.fromtimestamp(timeval, timezone.utc)
if localtime:
  dt = dt.astimezone()
return format_datetime(dt, usegmt)
msg232347 - (view) Author: Dmitry Shachnev (mitya57) * Date: 2014-12-09 06:53
Here is the updated patch based on Alexander’s suggestion.

Works fine, the only difference with previous behavior is that the zero time zone is now +0000 instead of -0000.
msg232378 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-12-09 14:44
That's a bug (you will note that it causes a test failure in the email test suite).  -0000 means "this date is in UTC, but that may or may not be the local timezone of the message".  +0000 means "this date really is in the UTC timezone".  utils.format_datetime uses +0000 if and only if the datetime is an aware UTC datetime.
msg232386 - (view) Author: Dmitry Shachnev (mitya57) * Date: 2014-12-09 16:03
Thanks, I did not know that. Here is version 3.
msg232388 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-12-09 16:09
Yeah, only someone who has studied the RFCs would know that detail.  I have no idea whether or not any email clients/processors actually *use* that distinction for anything, and I doubt there are very many humans who do :).
msg232392 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2014-12-09 16:45
v3 looks good to me.  BTW, I knew that the sign of TZ 0000 was significant, but would have to look up the RFC to recall what the significance was. Thanks, David, for the explanation.
msg233091 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-12-25 02:27
Is there any way to write a test for this?
msg233841 - (view) Author: Dmitry Shachnev (mitya57) * Date: 2015-01-11 11:39
This patch adds a test case for formatdate() function.

Before applying issue22932_v3.patch it fails, after applying that patch it succeeds.

Sorry for the delay caused by holidays.
msg233856 - (view) Author: Akira Li (akira) * Date: 2015-01-11 14:22
@mitya57: Please, combine the code changes, tests, docs into a single rietveld-compatible patch (hg diff); read devguide and http://bugs.python.org/issue13963 

Make sure "review" link appears on the right near the patch. Example: 
http://bugs.python.org/issue22798
msg233901 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-01-13 01:51
The tests fail for me the same way both before and after the code patch:

======================================================================
FAIL: test_formatdate (test.test_email.test_utils.FormatDateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rdmurray/python/p34/Lib/test/support/__init__.py", line 1525, in inner
    return func(*args, **kwds)
  File "/home/rdmurray/python/p34/Lib/test/test_email/test_utils.py", line 145, in test_formatdate
    self.assertEqual(string, 'Mon, 01 Dec 2014 15:00:00 -0000')
AssertionError: 'Mon, 01 Dec 2014 14:00:00 -0000' != 'Mon, 01 Dec 2014 15:00:00 -0000'
- Mon, 01 Dec 2014 14:00:00 -0000
?                   ^
+ Mon, 01 Dec 2014 15:00:00 -0000
?                   ^


======================================================================
FAIL: test_formatdate_with_localtime (test.test_email.test_utils.FormatDateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rdmurray/python/p34/Lib/test/support/__init__.py", line 1525, in inner
    return func(*args, **kwds)
  File "/home/rdmurray/python/p34/Lib/test/test_email/test_utils.py", line 157, in test_formatdate_with_localtime
    self.assertEqual(string, 'Mon, 01 Dec 2014 18:00:00 +0300')
AssertionError: 'Mon, 01 Dec 2014 18:00:00 +0400' != 'Mon, 01 Dec 2014 18:00:00 +0300'
- Mon, 01 Dec 2014 18:00:00 +0400
?                             ^
+ Mon, 01 Dec 2014 18:00:00 +0300
?                             ^
msg233905 - (view) Author: Dmitry Shachnev (mitya57) * Date: 2015-01-13 08:24
Can it be that you have outdated tzdata?
msg233934 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-01-13 13:08
Yes, that's possible.  I will check.
msg233996 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-01-14 02:09
Upgrading the timezone data results in passed tests.  Without the fix, only one of the tests fails.  Is this intentional?

This, however, brings up the issue that the tests may fail on the buildbots, which may also not have up to date timezone data :(
msg234001 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2015-01-14 03:17
Maybe Dmitry can come up with the "skipif" logic that will test for up-to date tzinfo and skip the test if it is not.  Otherwise we can try to come up with a test case which is sufficiently far in the past.
msg234113 - (view) Author: Dmitry Shachnev (mitya57) * Date: 2015-01-16 06:37
Now the patch works with tzdata >= 2011k, which is quite old.

@David: yes, with the unpatched version only one of the tests should fail.
msg247768 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-07-31 20:20
New changeset fa8b76dfd138 by Robert Collins in branch '3.4':
Issue #22932: Fix timezones in email.utils.formatdate.
https://hg.python.org/cpython/rev/fa8b76dfd138

New changeset 94b43b36e464 by Robert Collins in branch '3.5':
Issue #22932: Fix timezones in email.utils.formatdate.
https://hg.python.org/cpython/rev/94b43b36e464

New changeset 479787100b91 by Robert Collins in branch 'default':
Issue #22932: Fix timezones in email.utils.formatdate.
https://hg.python.org/cpython/rev/479787100b91
msg247769 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-07-31 20:23
Applied to 3.4 and up. Thanks for the patch!
History
Date User Action Args
2015-07-31 20:23:56rbcollinssetstatus: open -> closed

nosy: + rbcollins
messages: + msg247769

resolution: fixed
stage: commit review -> resolved
2015-07-31 20:20:24python-devsetnosy: + python-dev
messages: + msg247768
2015-07-31 20:14:56rbcollinssetversions: + Python 3.6
2015-04-11 17:13:06r.david.murraysetstage: patch review -> commit review
2015-01-16 06:37:35mitya57setfiles: + issue22932_combined_v2.diff

messages: + msg234113
2015-01-14 03:17:28belopolskysetmessages: + msg234001
2015-01-14 02:09:08r.david.murraysetmessages: + msg233996
2015-01-13 13:08:25r.david.murraysetmessages: + msg233934
2015-01-13 08:24:01mitya57setmessages: + msg233905
2015-01-13 01:51:04r.david.murraysetmessages: + msg233901
components: + email
2015-01-12 17:20:03berker.peksagsetnosy: + berker.peksag
2015-01-12 11:57:21mitya57setfiles: + issue22932_combined.diff
2015-01-11 14:22:14akirasetmessages: + msg233856
2015-01-11 11:39:53mitya57setfiles: + issue22932_test.patch

messages: + msg233841
2014-12-25 05:41:37akirasetnosy: + akira
2014-12-25 02:27:49r.david.murraysetmessages: + msg233091
2014-12-09 16:45:18belopolskysetmessages: + msg232392
2014-12-09 16:09:16r.david.murraysetmessages: + msg232388
2014-12-09 16:03:49mitya57setfiles: + issue22932_v3.patch

messages: + msg232386
2014-12-09 14:44:14r.david.murraysetmessages: + msg232378
2014-12-09 06:53:33mitya57setfiles: + issue22932_v2.patch

messages: + msg232347
2014-11-24 17:43:30belopolskysetmessages: + msg231615
2014-11-24 17:30:45r.david.murraysetstage: needs patch -> patch review
versions: + Python 3.5
2014-11-24 17:30:32r.david.murraysetmessages: + msg231614
2014-11-24 17:17:13mitya57setfiles: + issue22932.patch
keywords: + patch
messages: + msg231613
2014-11-24 16:32:33belopolskysettype: behavior
stage: needs patch
2014-11-24 16:31:15belopolskysetmessages: + msg231611
2014-11-24 16:28:33belopolskysetmessages: + msg231610
2014-11-24 16:20:35belopolskysetnosy: + barry, r.david.murray
messages: + msg231609
2014-11-24 16:17:20mitya57create