Issue 2227: time.strptime too strict? should it assume current year? (original) (raw)
Created on 2008-03-03 21:41 by gregory.p.smith, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Messages (5)
Author: Gregory P. Smith (gregory.p.smith) * 
Date: 2008-03-03 21:41
Some common python utilities had problems on Feb 29 this year when parsing dates using format strings that did not include a year in them.
time.strptime('Feb 29', '%b %d') Traceback (most recent call last): File "", line 1, in ? File "/usr/lib/python2.4/_strptime.py", line 425, in strptime julian = datetime_date(year, month, day).toordinal() -
ValueError: day is out of range for month
This is apparently because python assumes the year is 1900 unless it explicitly parses another year out of the string.
Applications can work around this by always adding a year and a %Y to the string they are parsing.
But not all date manipulating applications care about years. In this case the application was fail2ban, bug report and patches to it here:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=468382
Should the year default to 1900 (the equivalent of what the much more forgiving C API does by leaving struct tm tm_year = 0) or should this error be raised? If the answer is yes, works as is this is easy and just turns into us adding a note in the documentation to mention the behavior.
I do believe this was a valid bug in fail2ban as assuming the current year for date parsing is a bad idea and will do the wrong thing when parsing across a year change.
Python's strptime is much more strict than C strptime (glibc's C strptime is happy to return tm_mon 2 tm_mday 31. Its range checking is minimal.
here's a C test case to play with its behavior:
#include <assert.h> #include <stdio.h> #include <time.h> int main(int argc, char *argv[]) { unsigned long ret, parsed; assert(argc == 2); struct tm tm = { 0 }; ret = strptime(argv[1], "%b %d", &tm); parsed = ret - (unsigned long)(argv[1]); printf("ret 0x%x parsed %d tm_mon %d tm_mday %d tm_year %d\n", ret, parsed, tm.tm_mon, tm.tm_mday, tm.tm_year); }
% ./foo 'Feb 28' ret 0xffffda8a parsed 6 tm_mon 1 tm_mday 28 tm_year 0 % ./foo 'Feb 29' ret 0xffffda8a parsed 6 tm_mon 1 tm_mday 29 tm_year 0 % ./foo 'Feb 31' ret 0xffffda8a parsed 6 tm_mon 1 tm_mday 31 tm_year 0 % ./foo 'Feb 32' ret 0x0 parsed 9596 tm_mon 1 tm_mday 0 tm_year 0
Author: Brett Cannon (brett.cannon) *
Date: 2008-03-03 22:21
The documentation already mentions that the default values when information left out is (1900, 1, 1, 0, 0, 0, 0, 1, -1) so the docs are already clear. If you want to generate a patch to make the default year be this year I would be willing to review it and consider applying it. I doubt very much code would break because of this.
Author: Miki Tebeka (tebeka) *
Date: 2008-03-20 18:58
Here is a patch, hope it'll make it to 2.6
Author: Robert Schuppenies (schuppenies) *
Date: 2008-03-21 00:56
Applying the _strptime.diff patch broke the _strptime test("test_defaults"). Once you change the year, you also have to adapt the day of week, as this becomes dynamic, too. The rest remains the same, though. I attached a patch to this test which tests for the new-years day of the current year instead of 1900, but I feel like changing the semantic of the default value is no minor change. Also, I am not sure what the documentation should say then.
Author: Brett Cannon (brett.cannon) *
Date: 2009-04-02 05:42
After having thought about this I have decided I am going to stick with the current semantics. Having the year change underneath code based solely on when it executes will cause more problems than it will solve.
History
Date
User
Action
Args
2022-04-11 14:56:31
admin
set
github: 46480
2009-04-02 05:42:12
brett.cannon
set
status: open -> closed
resolution: rejected
messages: +
2009-02-11 03:14:59
brett.cannon
set
stage: patch review
2008-03-21 08:44:15
brett.cannon
set
assignee: brett.cannon
2008-03-21 00:56:22
schuppenies
set
files: + test_strptime.diff
nosy: + schuppenies
messages: +
2008-03-20 18:58:21
tebeka
set
files: + _strptime.diff
nosy: + tebeka
messages: +
keywords: + patch
2008-03-03 22:21:53
brett.cannon
set
nosy: + brett.cannon
messages: +
2008-03-03 21:41:25
gregory.p.smith
create