Skip to content

bpo-31339: time.asctime() raise on year > 9999 #3296

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

Closed
wants to merge 1 commit into from
Closed

bpo-31339: time.asctime() raise on year > 9999 #3296

wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Sep 4, 2017

time.asctime() now raises a ValueError for year larger than 9999 to prevent a crash when Python is run using the musl C library.

https://bugs.python.org/issue31339

time.asctime() now raises a ValueError for year larger than 9999 to
prevent a crash when Python is run using the musl C library.
@vstinner vstinner changed the title bpo-31339: Restrict time.asctime() to year < 10000 bpo-31339: time.asctime() raise on year > 9999 Sep 4, 2017
@vstinner vstinner requested a review from pitrou September 4, 2017 20:06
@pitrou
Copy link
Member

pitrou commented Sep 4, 2017

The thing is, a year greater than 9999 works right now on Linux (though for some reason it appends a "\n" character):

>>> time.asctime((2017,9,4,22,9,26,0,247,1))
'Mon Sep  4 22:09:26 2017'
>>> time.asctime((9999,9,4,22,9,26,0,247,1))
'Mon Sep  4 22:09:26 9999'
>>> time.asctime((19999,9,4,22,9,26,0,247,1))
'Mon Sep  4 22:09:26 19999\n'

So your PR is breaking this.

@vstinner
Copy link
Member Author

vstinner commented Sep 4, 2017

The thing is, a year greater than 9999 works right now on Linux (though for some reason it appends a "\n" character):

Currently, test_time.py contains:

        # XXX: Posix compiant asctime should refuse to convert
        # year > 9999, but Linux implementation does not.
        # self.assertRaises(ValueError, time.asctime,
        #                  (12345, 1, 0, 0, 0, 0, 0, 0, 0))
        # XXX: For now, just make sure we don't have a crash:

I tested:

haypo@selma$ python3.6 -c 'import time; print(repr(time.asctime((12345, 1, 1, 0, 0, 0, 0, 1, 0))))'
'Mon Jan  1 00:00:00 12345'
haypo@selma$ python2.7 -c 'import time; print(repr(time.asctime((12345, 1, 1, 0, 0, 0, 0, 1, 0))))'
'Mon Jan  1 00:00:00 12345\n'

Because of the newline, I would say that time.asctime() is already broken when running Python with the glibc. It's not like my change makes things worse.

@pitrou
Copy link
Member

pitrou commented Sep 4, 2017

The comment above is wrong. POSIX says behaviour with year > 9999 is undefined. The thing is, on Linux it works (mostly) fine (apart from the extraneous line feed). It may work fine as well on other platforms. So you're really proposing to break that behaviour.

@vstinner
Copy link
Member Author

vstinner commented Sep 5, 2017

@pitrou: "The thing is, a year greater than 9999 works right now on Linux"

Right, this PR introduces a regression on Linux, so I abandon it in favor of PR #3293.

@vstinner vstinner closed this Sep 5, 2017
@vstinner vstinner deleted the asctime_9999_27 branch September 5, 2017 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-security A security issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants