classification
Title: time.strftime does unnecessary range check
Type: behavior Stage: test needed
Components: Library (Lib) Versions: Python 2.6, Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: brett.cannon, georg.brandl, rshapiro
Priority: low Keywords: easy, patch

Created on 2009-09-02 15:40 by rshapiro, last changed 2009-09-22 00:34 by brett.cannon. This issue is now closed.

Messages (5)
msg92170 - (view) Author: Richard Shapiro (rshapiro) Date: 2009-09-02 15:40
in Modules/timemodule.c, in the routine time_strftime, there is a range
check on the tm_isdst field:

        if (buf.tm_isdst < -1 || buf.tm_isdst > 1) {
            PyErr_SetString(PyExc_ValueError,
                            "daylight savings flag out of range");
            return NULL;
        }

This check is incorrect, as the tm_isdst field is interpreted as less
than 0, 0, or greater than zero (see the comment at the top of the
routine, and the documentation for the strftime function).
Unfortunately, there exists at least one platform for which tm_isdst is
set to something other than -1, 0, or 1 (on IBM zOS it's apparently set
to 2 for daylight savings). This means that you can't do the obvious 
strftime(...,localtime(...)) to format a time.

The fix is to either remove the range check entirely, or else to
normalize the +1, -1, or 0. The former is preferred unless there is some
platform on which strftime doesn't do the right thing with values
outside the range of [-1,1]. 

This bug exists in 2.5.1 and 2.6.2. I haven't checked other versions.
msg92206 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2009-09-03 12:24
Assigning to Brett who added this check in r35368.
msg92224 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2009-09-03 21:37
Ugh, damn platforms and having to be different. =)

Normalization is the best solution as Python's docs says the expected 
value for the tm_dst field is -1, 0, or 1 (which is why I added the 
check).
msg92418 - (view) Author: Richard Shapiro (rshapiro) Date: 2009-09-08 14:43
Here's a patch to normalize the results of the various system calls
which return time information. This was against the source for Python 2.5.1.

*** timemodule.c	Tue Sep  8 10:28:31 2009
--- /home/rshapiro/216/redist/Python-2.5.1/Modules/timemodule.c.dist	Mon
Jan 12 22:53:07 2009
***************
*** 241,247 ****
  static PyObject *
  tmtotuple(struct tm *p)
  {
-         long dstval;
  	PyObject *v = PyStructSequence_New(&StructTimeType);
  	if (v == NULL)
  		return NULL;
--- 241,246 ----
***************
*** 256,271 ****
  	SET(5, p->tm_sec);
  	SET(6, (p->tm_wday + 6) % 7); /* Want Monday == 0 */
  	SET(7, p->tm_yday + 1);	   /* Want January, 1 == 1 */
!         /* Alas, on some platforms tm_isdst can be something other
than -1, 0, or 1 */
!         if (p->tm_isdst < 0) {
!           dstval = -1;
!         } else if (p->tm_isdst > 0) {
!           dstval = 1;
!         } else {
!           dstval = 0;
!         }
! 
! 	SET(8, dstval);
  #undef SET
  	if (PyErr_Occurred()) {
  		Py_XDECREF(v);
--- 255,261 ----
  	SET(5, p->tm_sec);
  	SET(6, (p->tm_wday + 6) % 7); /* Want Monday == 0 */
  	SET(7, p->tm_yday + 1);	   /* Want January, 1 == 1 */
! 	SET(8, p->tm_isdst);
  #undef SET
  	if (PyErr_Occurred()) {
  		Py_XDECREF(v);
(t)neo:/home/rshapiro/216/redist/patch/Python-2.5.1/Modules 140 %
msg92969 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2009-09-22 00:34
time.strftime() now normalizes tm_isdst.

2.7: r75011
3.2: r75012

This could probably be backported to 2.6/3.1, but since this is purely a 
convenience thing and it is a (very) minor change in semantics I am not 
going to bother.

Thanks, Robert, for the initial patch.
History
Date User Action Args
2009-09-22 00:34:35brett.cannonsetstatus: open -> closed
resolution: fixed
messages: + msg92969
2009-09-08 17:00:16brett.cannonsetkeywords: + patch
2009-09-08 14:43:04rshapirosetmessages: + msg92418
2009-09-03 21:37:20brett.cannonsetpriority: low
keywords: + easy
messages: + msg92224

stage: test needed
2009-09-03 12:24:23georg.brandlsetassignee: brett.cannon

messages: + msg92206
nosy: + georg.brandl, brett.cannon
2009-09-02 15:40:50rshapirocreate