email.utils.formatdate uses unreliable time.timezone constant

[mitya57]

BPO	22932
Nosy	@warsaw, @abalkin, @rbtcollins, @bitdancer, @4kir4, @berkerpeksag, @mitya57
Files	issue22932.patch issue22932_v2.patch issue22932_v3.patch issue22932_test.patch issue22932_combined.diff issue22932_combined_v2.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2015-07-31.20:23:56.997>
created_at = <Date 2014-11-24.16:17:20.017>
labels = ['type-bug', 'library', 'expert-email']
title = 'email.utils.formatdate uses unreliable time.timezone constant'
updated_at = <Date 2015-07-31.20:23:56.995>
user = 'https://github.com/mitya57'

bugs.python.org fields:

activity = <Date 2015-07-31.20:23:56.995>
actor = 'rbcollins'
assignee = 'none'
closed = True
closed_date = <Date 2015-07-31.20:23:56.997>
closer = 'rbcollins'
components = ['Library (Lib)', 'email']
creation = <Date 2014-11-24.16:17:20.017>
creator = 'mitya57'
dependencies = []
files = ['37266', '37394', '37402', '37675', '37683', '37721']
hgrepos = []
issue_num = 22932
keywords = ['patch']
message_count = 23.0
messages = ['231608', '231609', '231610', '231611', '231613', '231614', '231615', '232347', '232378', '232386', '232388', '232392', '233091', '233841', '233856', '233901', '233905', '233934', '233996', '234001', '234113', '247768', '247769']
nosy_count = 8.0
nosy_names = ['barry', 'belopolsky', 'rbcollins', 'r.david.murray', 'akira', 'python-dev', 'berker.peksag', 'mitya57']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue22932'
versions = ['Python 3.4', 'Python 3.5', 'Python 3.6']

[mitya57]

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.

[abalkin]

See also bpo-665194.

[abalkin]

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

[abalkin]

"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

[mitya57]

This patch fixes the issue for me.

[bitdancer]

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?

[abalkin]

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)

[mitya57]

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.

[bitdancer]

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.

[mitya57]

Thanks, I did not know that. Here is version 3.

[bitdancer]

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 :).

[abalkin]

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.

[bitdancer]

Is there any way to write a test for this?

[mitya57]

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.

[4kir4]

@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

[bitdancer]

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
?                             ^

[mitya57]

Can it be that you have outdated tzdata?

[bitdancer]

Yes, that's possible. I will check.

[bitdancer]

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 :(

[abalkin]

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.

[mitya57]

Now the patch works with tzdata >= 2011k, which is quite old.

@david: yes, with the unpatched version only one of the tests should fail.

[python-dev]

New changeset fa8b76dfd138 by Robert Collins in branch '3.4':
Issue bpo-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 bpo-22932: Fix timezones in email.utils.formatdate.
https://hg.python.org/cpython/rev/94b43b36e464

New changeset 479787100b91 by Robert Collins in branch 'default':
Issue bpo-22932: Fix timezones in email.utils.formatdate.
https://hg.python.org/cpython/rev/479787100b91

[rbtcollins]

Applied to 3.4 and up. Thanks for the patch!