classification
Title: rfc2822 formatdate functionality duplication
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ajaksu2, anthonybaxter, barry, berker.peksag, dalke, eric.araujo, gaul, karlcow, l0nwlf, orsenthil, python-dev, r.david.murray
Priority: normal Keywords: patch

Created on 2003-06-02 05:18 by dalke, last changed 2016-03-14 04:06 by berker.peksag. This issue is now closed.

Files
File name Uploaded Description Edit
server2.patch karlcow, 2013-02-25 21:16
issue-747320-1.patch karlcow, 2013-03-01 02:43 review
issue-747320-3.patch karlcow, 2013-03-08 19:38 review
issue747320.diff berker.peksag, 2015-09-26 16:12 review
issue747320_v2.diff berker.peksag, 2016-03-13 01:24 review
Messages (19)
msg60342 - (view) Author: Andrew Dalke (dalke) * (Python committer) Date: 2003-06-02 05:18
There are at least 4 places in the standard Python 2.3 library which 
build an RFC 2822 formatted string:

rfc822.formatstring
email.Utils.formatdate
logging.handlers.SMTPHandler.date_time
BaseHTTPServer.HTTPServer.date_time_string

Looking at them, it makes sense to me to
  - replace rfc822's implementation with email's
      (that's the most flexible of the bunch)

  - start a migration so that email uses the one
      from rfc822, if available, else it's own implementation
      (since email is distributed to older Pythons)

  - have logging use the one in rfc822.

  - have BaseHTTPServer use the one in rfc822

If this is deemed an appropriate change, I can send in
a patch.
msg60343 - (view) Author: Anthony Baxter (anthonybaxter) (Python triager) Date: 2003-06-02 06:13
Logged In: YES 
user_id=29957

It's not just formatdate - various email formatting tasks
are probably the
single largest source of duplication in the std library...

msg60344 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2003-06-02 12:37
Logged In: YES 
user_id=12800

Personally, I'd like to see the email package's date formatting (and other 
email related tasks) be the canonical standard, and for other modules to use 
its rules when appropriate.
msg60345 - (view) Author: Andrew Gaul (gaul) Date: 2003-08-20 08:31
Logged In: YES 
user_id=139865

Careful, date/time formats differ between RFCs.

RFC 850  = Wdy, DD-Mon-YYYY HH:MM:SS (UT/GMT/(+/-)DDDD/...)
RFC 2616 = Wdy, DD Mon YYYY HH:MM:SS GMT
RFC 2822 = Wdy, DD Mon YYYY HH:MM:SS (+/-)DDDD

rfc822.formatstring
    deprecated; let it rot
email.Utils.formatdate
    uses RFC 2822, the One True Date Format
logging.handlers.SMTPHandler.date_time
    uses RFC 2616, should be using RFC 2822
BaseHTTPServer.HTTPServer.date_time_string
    uses RFC 2616
Cookie._getdate
    uses RFC 850

Patch 791776 changes SMTPHandler to use rfc2822.formatdate.
 grepping for day names, month names, year, gmtime, and
localtime does not reveal further duplication.
msg81712 - (view) Author: Daniel Diniz (ajaksu2) (Python triager) Date: 2009-02-12 03:10
Still present in source, but doesn't seem too bad.
msg114240 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-08-18 16:14
Does this need to be addressed or can it be closed as out of date or what?
msg114288 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-08-18 22:33
It still needs to be addressed.  I'm marking it for 3.2 but I doubt it will get addressed before 3.3 in reality.  I also made the type 'performance' since we have no 'refactoring' type.
msg182844 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-02-24 01:04
I notice that logging has been updated.  Only http.server's date_time_string remains (email.utils.formatdate does support the GMT form).
msg182980 - (view) Author: karl (karlcow) * Date: 2013-02-25 20:07
http://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-22#section-7.1.1

quoting from HTTP 1.1 bis

   Prior to 1995, there were three different formats commonly used by
   servers to communicate timestamps.  For compatibility with old
   implementations, all three are defined here.  The preferred format is
   a fixed-length and single-zone subset of the date and time
   specification used by the Internet Message Format [RFC5322].

     HTTP-date    = IMF-fixdate / obs-date

   An example of the preferred format is

     Sun, 06 Nov 1994 08:49:37 GMT    ; IMF-fixdate

   Examples of the two obsolete formats are

     Sunday, 06-Nov-94 08:49:37 GMT   ; obsolete RFC 850 format
     Sun Nov  6 08:49:37 1994         ; ANSI C's asctime() format

   A recipient that parses a timestamp value in an HTTP header field
   MUST accept all three formats.  A sender MUST generate the IMF-
   fixdate format when sending an HTTP-date value in a header field.


What http.server.BaseHTTPRequestHandler.date_time_string is currently doing

>>> import time
>>> timestamp = time.time()
>>> weekdayname = ['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun']
>>> monthname = [None,'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun','Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec']
>>> year, month, day, hh, mm, ss, wd, y, z = time.gmtime(timestamp)
>>> s = "%s, %02d %3s %4d %02d:%02d:%02d GMT" % (weekdayname[wd],day, monthname[month], year,hh, mm, ss)
>>> s
'Mon, 25 Feb 2013 19:26:34 GMT'



what email.utils.formatdate is doing:

>>> import email.utils
>>> email.utils.formatdate(timeval=None,localtime=False, usegmt=True)
'Mon, 25 Feb 2013 19:40:04 GMT'
>>> import time
>>> ts = time.time()
>>> email.utils.formatdate(timeval=ts,localtime=False, usegmt=True)
'Mon, 25 Feb 2013 19:51:50 GMT'

I createad a patch 
s = email.utils.formatdate(timestamp, False, True)

I didn't touch the log method which has a different format which is anyway not compatible with email.utils.
msg182984 - (view) Author: karl (karlcow) * Date: 2013-02-25 21:16
Made a mistake in the previous server.patch, use server.2.patch
msg183239 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-02-28 22:34
Could you regenerate your patch using hg diff? (or at least diff -u)?
msg183242 - (view) Author: karl (karlcow) * Date: 2013-03-01 02:43
R. David Murray:

Sure. Is it better? issue-747320-1.patch

It seems there are already tests for gmt date format in Lib/test/test_email/test_utils.py
msg183758 - (view) Author: karl (karlcow) * Date: 2013-03-08 19:38
Ok after comments and review by Eric Araujo on the previous patch.
See issue-747320-3.patch

I have ran

→ ./python.exe Lib/test/test_httpservers.py

[…]

----------------------------------------------------------------------
Ran 39 tests in 3.734s

OK
[137158 refs]

That said there is no specific tests for date_time_string() and log_date_time_string(). That might be something to consider.

→ grep date_time  Lib/test/test_httpservers.py
[nil]
msg183773 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-03-08 23:30
Tests would be great, especially given that we can add them in 3.2 and when merging 3.2 into 3.3 and then default, it ensures that the new code has no regression.

(A minor thing: I would use “attribute” instead of “variable” in the docstrings.)

There are also test helpers to assert that a warning is sent, but we don’t have a full coverage policy so it’s okay if you don’t feel like adding them.
msg210157 - (view) Author: karl (karlcow) * Date: 2014-02-03 19:42
Eric,

what do you recommend to move forward with this bug and patches?
Need guidance.
Do you have an example for "(A minor thing: I would use “attribute” instead of “variable” in the docstrings.)"

Also which code base I should use? A lot of water has gone under the bridge in one year. :)
msg211186 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2014-02-13 22:42
> what do you recommend to move forward with this bug and patches?

I would add tests to 3.4, to be sure that changing the code in 3.5 does not break compatibility.

> Do you have an example for "(A minor thing: I would use “attribute” instead of “variable” in the docstrings.)"

“Deprecated weekdayname variable” → “Deprecated weekdayname attribute”
msg251653 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-09-26 16:12
Here is an updated patch (including tests and documentation updates).
msg261669 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-03-13 01:24
-        now = time.time()
-        year, month, day, hh, mm, ss, x, y, z = time.localtime(now)
-        s = "%02d/%3s/%04d %02d:%02d:%02d" % (
-                day, self.monthname[month], year, hh, mm, ss)
+        s = time.strftime("%d/%b/%Y %H:%M:%S", time.localtime())

This part of the patch is incorrect. time.strftime() will return non-English values for different locales.

About deprecations of weekdayname and monthname attributes:

I know that they are undocumented, but they are already being used in the wild. For example, see https://github.com/natemago/srv/blob/master/srv.py#L419 Since we don't have any public API for this use case, I'd be +1 to keep them for now.

Here is an updated patch.
msg261722 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-14 04:05
New changeset ee64faffd46a by Berker Peksag in branch 'default':
Issue #747320: Use email.utils.formatdate() to avoid code duplication
https://hg.python.org/cpython/rev/ee64faffd46a
History
Date User Action Args
2016-03-14 04:06:13berker.peksagsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2016-03-14 04:05:20python-devsetnosy: + python-dev
messages: + msg261722
2016-03-13 01:24:08berker.peksagsetfiles: + issue747320_v2.diff

messages: + msg261669
2016-03-07 10:32:41berker.peksaglinkissue7370 superseder
2015-09-26 16:12:50berker.peksagsetfiles: + issue747320.diff

type: performance -> enhancement
components: - email
versions: + Python 3.6, - Python 3.5
nosy: + berker.peksag

messages: + msg251653
stage: needs patch -> patch review
2014-02-13 22:42:23eric.araujosetmessages: + msg211186
versions: + Python 3.5, - Python 3.4
2014-02-03 19:42:58karlcowsetmessages: + msg210157
2014-02-03 18:39:09BreamoreBoysetnosy: - BreamoreBoy
2013-03-08 23:30:31eric.araujosetnosy: + orsenthil, eric.araujo
messages: + msg183773
2013-03-08 19:38:08karlcowsetfiles: + issue-747320-3.patch

messages: + msg183758
2013-03-01 02:43:18karlcowsetfiles: + issue-747320-1.patch

messages: + msg183242
2013-02-28 22:34:24r.david.murraysetmessages: + msg183239
2013-02-25 21:17:14karlcowsetfiles: - server.patch
2013-02-25 21:16:49karlcowsetfiles: + server2.patch

messages: + msg182984
2013-02-25 20:07:21karlcowsetfiles: + server.patch

nosy: + karlcow
messages: + msg182980

keywords: + patch
2013-02-24 01:04:50r.david.murraysetmessages: + msg182844
components: + email
versions: + Python 3.4, - Python 3.2
2010-08-19 01:01:56l0nwlfsetnosy: + l0nwlf
2010-08-18 22:33:51r.david.murraysetversions: + Python 3.2, - Python 2.6
nosy: + r.david.murray

messages: + msg114288

type: performance
stage: needs patch
2010-08-18 16:14:50BreamoreBoysetnosy: + BreamoreBoy
messages: + msg114240
2009-02-12 03:10:11ajaksu2setnosy: + ajaksu2
messages: + msg81712
versions: + Python 2.6
2003-06-02 05:18:09dalkecreate