Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crash in datetime.strftime #57883

Closed
patrickvrijlandt mannequin opened this issue Dec 29, 2011 · 21 comments
Closed

crash in datetime.strftime #57883

patrickvrijlandt mannequin opened this issue Dec 29, 2011 · 21 comments
Assignees
Labels
OS-windows stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@patrickvrijlandt
Copy link
Mannequin

patrickvrijlandt mannequin commented Dec 29, 2011

BPO 13674
Nosy @abalkin, @vstinner, @tjguk, @florentx
Files
  • issue13674.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 = 'https://github.com/tjguk'
    closed_at = <Date 2013-11-12.13:00:20.682>
    created_at = <Date 2011-12-29.09:04:17.354>
    labels = ['library', 'OS-windows', 'type-crash']
    title = 'crash in datetime.strftime'
    updated_at = <Date 2013-11-12.13:33:53.146>
    user = 'https://bugs.python.org/patrickvrijlandt'

    bugs.python.org fields:

    activity = <Date 2013-11-12.13:33:53.146>
    actor = 'python-dev'
    assignee = 'tim.golden'
    closed = True
    closed_date = <Date 2013-11-12.13:00:20.682>
    closer = 'tim.golden'
    components = ['Library (Lib)', 'Windows']
    creation = <Date 2011-12-29.09:04:17.354>
    creator = 'patrick.vrijlandt'
    dependencies = []
    files = ['32516']
    hgrepos = []
    issue_num = 13674
    keywords = ['patch']
    message_count = 21.0
    messages = ['150323', '150325', '150335', '150341', '150343', '150345', '150353', '150368', '150369', '150372', '198688', '198692', '198693', '198694', '198700', '202268', '202269', '202271', '202683', '202684', '202688']
    nosy_count = 8.0
    nosy_names = ['belopolsky', 'vstinner', 'tim.golden', 'flox', 'python-dev', 'patrick.vrijlandt', 'Ramchandra Apte', 'gladman']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue13674'
    versions = ['Python 3.3', 'Python 3.4']

    @patrickvrijlandt
    Copy link
    Mannequin Author

    patrickvrijlandt mannequin commented Dec 29, 2011

    This causes a crash in python 3.2.2 and 3.2, but not in 2.7.2

    C:\Python32>python
    Python 3.2 (r32:88445, Feb 20 2011, 21:29:02) [MSC v.1500 32 bit (Intel)] on win32
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import datetime
    >>> datetime.datetime(1899,12,31).strftime("%y")

    The crash happens with %y but not with %Y.
    The crash happens with any year < 1900.
    On 2.7.2 a ValueError is raised because strftime requires year >= 1900. This is what IMHO should happen (and would have saved me a lot of time)

    @patrickvrijlandt patrickvrijlandt mannequin added stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump labels Dec 29, 2011
    @RamchandraApte
    Copy link
    Mannequin

    RamchandraApte mannequin commented Dec 29, 2011

    This bug can not be reproduced in Python 3.2.2 on Ubuntu.
    Since Python 2.7.2 on your system raises a ValueError for dates below 1900 ,your system's strftime probably does not allow dates below 1900 (unlike Ubuntu).
    Python 3.2.2's datetime.strftime should handle this error from strftime().

    @tjguk
    Copy link
    Member

    tjguk commented Dec 29, 2011

    This is happening on Windows x86 against the current tip. The MS C runtime can handle older dates; it's just that we're taking 1900 off the year at some point. (At least, I think that's what's happening). FWIW you only need time.strftime to reproduce the error:

      import time
      time.strftime("%y", (1899, 1, 1, 0, 0, 0, 0, 0, 0))

    If no-one gets there first I'll dig into the timemodule strftime wrapper.

    @tjguk
    Copy link
    Member

    tjguk commented Dec 29, 2011

    ... and that's because the tm struct defines the tm_year field as an offset from 1900. Sorry for the false start. I'll look at the MS runtime stuff instead

    @vstinner
    Copy link
    Member

    timemodule.c has the following check:

    #if defined(_MSC_VER) || defined(sun)
        if (buf.tm_year + 1900 < 1 || 9999 < buf.tm_year + 1900) {
            PyErr_SetString(PyExc_ValueError,
                            "strftime() requires year in [1; 9999]");
            return NULL;
        }
    #endif

    @tjguk
    Copy link
    Member

    tjguk commented Dec 29, 2011

    Yes, but the MS crt strftime *for %y* requires a year >= 1900 (ie
    tm_year >= 0). Looks like we need a special check.

    @patrickvrijlandt
    Copy link
    Mannequin Author

    patrickvrijlandt mannequin commented Dec 29, 2011

    Is it relevant that 2.7.2 _does_ throw a correct exception?

    @tjguk
    Copy link
    Member

    tjguk commented Dec 30, 2011

    Well, the code in 2.x is quite different from that in 3.x.
    Specifically, the 2.x code assumes that, for Windows, no
    year before 1900 is valid for any of the formats. So a
    simple check throws a ValueError for tm_year < 0. For 3.x
    the assumption was that Windows can handle any year as far
    back as 0; in fact that's not true for the %y format.

    I'll propose a patch to timemodule.c but I'll have to take
    it to python-dev as I'm not 100% sure of the best place for
    it.

    @patrickvrijlandt
    Copy link
    Mannequin Author

    patrickvrijlandt mannequin commented Dec 30, 2011

    Somewhere in the code is also/still a seperate check concerning strftime:

    PythonWin 3.2 (r32:88445, Feb 20 2011, 21:29:02) [MSC v.1500 32 bit (Intel)] on win32.
    Portions Copyright 1994-2008 Mark Hammond - see 'Help/About PythonWin' for further copyright information.
    >>> import datetime
    >>> datetime.datetime(-1, 1, 1)
    Traceback (most recent call last):
      File "<interactive input>", line 1, in <module>
    ValueError: year is out of range
    >>> datetime.datetime(0, 1, 1)
    Traceback (most recent call last):
      File "<interactive input>", line 1, in <module>
    ValueError: year is out of range
    >>> datetime.datetime(1, 1, 1)
    datetime.datetime(1, 1, 1, 0, 0)
    >>> datetime.datetime(1, 1, 1).strftime("Y")
    Traceback (most recent call last):
      File "<interactive input>", line 1, in <module>
    ValueError: year=1 is before 1000; the datetime strftime() methods require year >= 1000
    >>>

    @tjguk
    Copy link
    Member

    tjguk commented Dec 30, 2011

    Yes, sorry. I wasn't clear enough. There *are* still checks
    in the 3.x code (for the kind of thing you're showing). But
    the checks assume 1000 <= year <= maxint is ok for all format
    parameters on Windows. In fact, for %y, only 1900 <= year is ok.

    @gladman
    Copy link
    Mannequin

    gladman mannequin commented Sep 30, 2013

    On IDLE this:

    Python 3.3.2 (v3.3.2:d047928ae3f6, May 16 2013, 00:06:53) [MSC v.1600 64 bit (AMD64)] on win32
    Type "copyright", "credits" or "license()" for more information.
    >>> from datetime import datetime
    >>> datetime(1878, 12, 31).strftime('%d %b %y')

    causes a crash on Windows. I am surprised that this bug still exists as it is not far off two years old now.

    @vstinner
    Copy link
    Member

    I am surprised that this bug still exists as it is not far off two years old now.

    You should report the bug to Microsoft who distributes a buggy C runtime library.

    @gladman
    Copy link
    Mannequin

    gladman mannequin commented Sep 30, 2013

    On 30/09/2013 12:39, STINNER Victor wrote:

    STINNER Victor added the comment:

    > I am surprised that this bug still exists as it is not far off two years old now.

    You should report the bug to Microsoft who distributes a buggy C runtime library.

    ----------


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue13674\>


    Thank you for your suggestion.

    But, given that this bug has been present for some two years, I would
    hope that Python developers would have already done what you suggest and
    this leads me to doubt that any approach that I made to Microsoft would
    achieve any practical benefit.

    In practice it is pretty well always necessary in building applications
    on top of widely used compilers and libraries to have to workaround the
    many bugs that they contain.

    Since there is evidently no workaround for this issue in the Python
    3.3.2 code, I have to assume that one is not considered to be
    necessary, presumably because Microsoft has fixed (or intends to fix)
    this issue.

    @tjguk
    Copy link
    Member

    tjguk commented Sep 30, 2013

    In reality (as I'm sure you can guess) it's just that no-one's got to
    the point of fixing it. I did start off, but it's not a trivial fix and
    clearly it got sidelined (with no-one shouting). Sometimes that's just
    the way it is.

    I'll see if I can dig out whatever code I had managed to change. I'm
    certainly happy to review and apply a patch if someone wants to supply one.

    @gladman
    Copy link
    Mannequin

    gladman mannequin commented Sep 30, 2013

    On 30/09/2013 13:14, Tim Golden wrote:

    Tim Golden added the comment:

    In reality (as I'm sure you can guess) it's just that no-one's got to
    the point of fixing it. I did start off, but it's not a trivial fix and
    clearly it got sidelined (with no-one shouting). Sometimes that's just
    the way it is.

    I'll see if I can dig out whatever code I had managed to change. I'm
    certainly happy to review and apply a patch if someone wants to supply one.

    Thank you for your comment Tim.

    To be honest, Python is just so reliable that I was genuinely very
    surprised to find something that actually crashes it :-)

    It's hardly a priority but I thought it might be worth a comment in case
    it was a bug that had simply been forgotten about.

    @tjguk tjguk self-assigned this Nov 6, 2013
    @tjguk
    Copy link
    Member

    tjguk commented Nov 6, 2013

    Attached is a patch with tests

    @vstinner
    Copy link
    Member

    vstinner commented Nov 6, 2013

    + if (strchr("y", outbuf[1]) && buf.tm_year < 0)

    hum... why not simply outbuf[1] == 'y' ? It would be more explicit and less surprising.

    For the unit test, it would be nice to test also asctime(), even if time.asctime() doesn't use asctime() of the C library. And it's better to run tests on all platforms. Only test_y_before_1900() should behave differently on other platforms, but it would be nice to run test_y_before_1900() on platforms supporting "%y" with year < 1900. In my experience, other operating systems have also their own issues. For example, time.strftime() has a specific test to Windows, but also Solaris and AIX.

    @tjguk
    Copy link
    Member

    tjguk commented Nov 6, 2013

    On 06/11/2013 15:23, STINNER Victor wrote:

    •    if (strchr("y", outbuf[1]) && buf.tm_year \< 0)
      

    hum... why not simply outbuf[1] == 'y' ? It would be more explicit
    and less surprising.

    Ummm. I have no idea what I was thinking about there. I think it was
    somehow connected with the strchr check a few lines earlier. Anyhow,
    fixed now, thanks.

    For the unit test, it would be nice to test also asctime(), even if
    time.asctime() doesn't use asctime() of the C library. And it's
    better to run tests on all platforms. Only test_y_before_1900()
    should behave differently on other platforms, but it would be nice to
    run test_y_before_1900() on platforms supporting "%y" with year <
    1900. In my experience, other operating systems have also their own
    issues. For example, time.strftime() has a specific test to Windows,
    but also Solaris and AIX.

    I'm not sure where time.asctime comes into it. The implementation
    doesn't use time.strftime, but even if it did, I don't see the need to
    add a test under this issue: the unit test for strftime should be enough
    to cover any direct or indirect use of the function.

    I'm happy to open up the other tests.

    TJG

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 12, 2013

    New changeset 1537f14cc690 by Tim Golden in branch '3.3':
    bpo-13674 Correct crash with strftime %y format under Windows
    http://hg.python.org/cpython/rev/1537f14cc690

    New changeset 41a4c55db7f2 by Tim Golden in branch 'default':
    bpo-13674 Correct crash with strftime %y format under Windows
    http://hg.python.org/cpython/rev/41a4c55db7f2

    @tjguk
    Copy link
    Member

    tjguk commented Nov 12, 2013

    I've committed the changes with a variant of the pre-1900 test running on all platforms. I think there's scope for more testing of the boundary conditions of strftime but that would be for another issue. I want to get this one in now as it's a crasher on Windows.

    @tjguk tjguk closed this as completed Nov 12, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 12, 2013

    New changeset c61147d66843 by Tim Golden in branch 'default':
    Issue bpo-13674 Updated NEWS
    http://hg.python.org/cpython/rev/c61147d66843

    New changeset 49db4851c63b by Tim Golden in branch '3.3':
    Issue bpo-13674 Updated NEWS
    http://hg.python.org/cpython/rev/49db4851c63b

    New changeset 3ff7602ee543 by Tim Golden in branch 'default':
    Issue bpo-13674 Null merge with 3.3
    http://hg.python.org/cpython/rev/3ff7602ee543

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    OS-windows stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants