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

datetime operations spanning MINYEAR give bad results #51399

Closed
markleander mannequin opened this issue Oct 16, 2009 · 15 comments
Closed

datetime operations spanning MINYEAR give bad results #51399

markleander mannequin opened this issue Oct 16, 2009 · 15 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@markleander
Copy link
Mannequin

markleander mannequin commented Oct 16, 2009

BPO 7150
Nosy @amauryfa, @mdickinson, @abalkin
Files
  • datetimemodule.c.svndiff
  • issue7150.diff
  • issue7150a.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/abalkin'
    closed_at = <Date 2010-05-27.22:08:58.131>
    created_at = <Date 2009-10-16.09:45:28.820>
    labels = ['type-bug', 'library']
    title = 'datetime operations spanning MINYEAR give bad results'
    updated_at = <Date 2010-05-27.22:08:58.129>
    user = 'https://bugs.python.org/markleander'

    bugs.python.org fields:

    activity = <Date 2010-05-27.22:08:58.129>
    actor = 'belopolsky'
    assignee = 'belopolsky'
    closed = True
    closed_date = <Date 2010-05-27.22:08:58.131>
    closer = 'belopolsky'
    components = ['Library (Lib)']
    creation = <Date 2009-10-16.09:45:28.820>
    creator = 'mark.leander'
    dependencies = []
    files = ['15194', '16372', '17466']
    hgrepos = []
    issue_num = 7150
    keywords = ['patch']
    message_count = 15.0
    messages = ['94132', '94384', '94449', '100102', '100104', '100105', '100107', '100118', '102945', '103718', '106509', '106511', '106529', '106531', '106618']
    nosy_count = 5.0
    nosy_names = ['amaury.forgeotdarc', 'mark.dickinson', 'belopolsky', 'pythonhacker', 'mark.leander']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue7150'
    versions = ['Python 2.6', 'Python 2.7', 'Python 3.2', 'Python 3.3']

    @markleander
    Copy link
    Mannequin Author

    markleander mannequin commented Oct 16, 2009

    The datetime module documentation would imply that operations that cause
    dates to fall outside the MINYEAR--MAXYEAR range should raise
    OverflowError. The interpreter session below shows that this is not
    always the case, and that such operations may cause bogus and
    inconsistent results.

    Python 2.6.3 (r263rc1:75186, Oct  2 2009, 20:40:30) [MSC v.1500 32 bit
    (Intel)]
    on win32
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import datetime
    >>> t0=datetime.datetime(1,1,1)
    >>> d1, d2, d3 = map(datetime.timedelta, range(1,4))
    # The following is expected and accoring to the docs:
    >>> t0-d1
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    OverflowError: date value out of range
    
    # The following is completely bogus:
    >>> t0-d2
    datetime.datetime(1, 0, 255, 0, 0)
    
    # The two following behaving differently may be very confusing,
    # the second one is correct
    >>> t0-d2+d3
    datetime.datetime(1, 8, 15, 0, 0)
    >>> t0+d3-d2
    datetime.datetime(1, 1, 2, 0, 0)
    >>>

    @markleander markleander mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Oct 16, 2009
    @pythonhacker
    Copy link
    Mannequin

    pythonhacker mannequin commented Oct 23, 2009

    The issue is present in Python 3.0 and 2.5 as well.

    Python 2.5.1 (r251:54863, Jul 17 2008, 13:21:31)
    [GCC 4.3.1 20080708 (Red Hat 4.3.1-4)] on linux2
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import datetime
    >>> t0=datetime.datetime(1,1,1)
    >>> d1,d2,d3=map(datetime.timedelta, range(1,4))
    >>> t0-d1
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    OverflowError: date value out of range
    >>> t0-d2
    datetime.datetime(1, 0, 255, 0, 0)

    I think this is bug in datetime for all Python versions

    @pythonhacker
    Copy link
    Mannequin

    pythonhacker mannequin commented Oct 25, 2009

    The problem seems to be in the "normalize_date" function in
    datetimemodule.c. It is checking for a valid year range, but not
    checking for a valid month or day range.

    I have a patch which fixes this problem. It checks for month range
    (1<=m<=12) and day range(1<=d<=31). Here is Python with the patch.

    anand@anand-laptop:~/projects/python/py3k$ ./python
    Python 3.2a0 (py3k:75627, Oct 25 2009, 14:28:21) 
    [GCC 4.3.3] on linux2
    Type "help", "copyright", "credits" or "license" for more information.
    Traceback (most recent call last):
      File "/home/anand/.pythonrc", line 2, in <module>
        import readline
    ImportError: No module named readline
    >>> import datetime
    >>> t0=datetime.datetime(1,1,1)
    >>> d1,d2,d3=map(datetime.timedelta, range(1,4))
    >>> t0-d1
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    OverflowError: date value out of range
    >>> t0-d2
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    OverflowError: date value out of range
    >>> t0-d3
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    OverflowError: date value out of range
    >>> d0=datetime.timedelta(0)
    >>> t0-d0
    datetime.datetime(1, 1, 1, 0, 0)
    >>> 

    Svn diff is attached.

    @amauryfa
    Copy link
    Member

    The patch seems OK, but needs tests.

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Feb 25, 2010

    Given

        assert(*m > 0);
        assert(*d > 0);
    

    at the end of normalize_y_m_d(), it looks like at lest 1 <=*month and 1 <=*day are redundant.

    A closer look also reveals

        assert(1 <= *m && *m <= 12);
    

    in the middle of normalize_y_m_d(). This seems to leave only *day <=31 possibly relevant.

    I suspect that out of bounds day surviving normalize_y_m_d() is a logical error in that function that needs to be fixed and an assert() added at the end. The proposed patch appears to cure the symptom rather than the actual flaw.

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Feb 25, 2010

    Aha! My reliance on asserts() was misguided. With the debug build:

    >>> t0-d2
    Assertion failed: (ordinal >= 1), function ord_to_ymd, file /Users/sasha/Work/python-svn/trunk/Modules/datetimemodule.c, line 269.
    Abort trap

    Should we reclassify this bug as a crash?

    @amauryfa
    Copy link
    Member

    No, because normally distributions do not use debug builds.
    but that's the reason why tests are needed: they must pass with asserts enabled.

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Feb 25, 2010

    I am attaching my variant of the patch including additional unit tests.

    Note that my changes make normalize_y_m_d() and normalize_date() the same, but I am leaving the current structure intact to make reviewer's job easier.

    @pythonhacker
    Copy link
    Mannequin

    pythonhacker mannequin commented Apr 12, 2010

    Can someone update this issue ? Is the 2nd patch tested... ?

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Apr 20, 2010

    My patch includes unit tests and I tested it on Mac OS X. Anand, what kind of testing do you have in mind?

    @abalkin abalkin self-assigned this May 26, 2010
    @abalkin
    Copy link
    Member

    abalkin commented May 26, 2010

    I've untabified my last patch and added a NEWS entry. I believe it is ready for commit review. Mark?

    @mdickinson
    Copy link
    Member

    I'll take a look at this patch later today.

    @mdickinson mdickinson assigned mdickinson and unassigned abalkin May 26, 2010
    @mdickinson
    Copy link
    Member

    The patch looks good to me. Please apply!

    @mdickinson mdickinson assigned abalkin and unassigned mdickinson May 26, 2010
    @mdickinson
    Copy link
    Member

    As an aside, I dislike the fact that the datetime module uses a C 'int' for date ordinals, and clearly assumes that it'll be at least 32 bits. int could be as small as 16 bits on some systems (small embedded systems?). But that's another issue.

    @abalkin
    Copy link
    Member

    abalkin commented May 27, 2010

    Committed in r81566 (trunk), r81568 (py3k), r81569 (release26-maint), r81570 (release31-maint).

    @abalkin abalkin closed this as completed May 27, 2010
    @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
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants