msg205774 - (view) |
Author: Akira Li (akira) * |
Date: 2013-12-10 07:29 |
cert_time_to_seconds() uses `time.mktime()` [1] to convert utc time tuple to seconds since epoch. `mktime()` works with local time. It should use `calendar.timegm()` analog instead.
Example from the docs [2] is seven hours off (it shows utc offset of the local timezone of the person who created it):
>>> import ssl
>>> ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT")
1178694000.0
It should be `1178668800`:
>>> from datetime import datetime
>>> datetime.utcfromtimestamp(1178668800)
datetime.datetime(2007, 5, 9, 0, 0)
>>> import time
>>> time.gmtime(1178668800)
time.struct_time(tm_year=2007, tm_mon=5, tm_mday=9, tm_hour=0, tm_min=0, tm_sec=0, tm_wday=2, tm_yday=129, tm_isdst=0)
And `calendar.timegm` returns correct result:
>>> calendar.timegm(time.strptime("May 9 00:00:00 2007 GMT", "%b %d %H:%M:%S %Y GMT"))
1178668800
[1]: http://hg.python.org/cpython/file/96a68e369d13/Lib/ssl.py#l849
[2]: http://hg.python.org/cpython/file/96a68e369d13/Doc/library/ssl.rst#l359
|
msg205803 - (view) |
Author: gudge (gudge) |
Date: 2013-12-10 12:10 |
Will work on this.
Please assign the issue to me.
Instructions before proceeding by Tim Golden(python mailing list):
Having just glanced at that issue, I would point out that there's been a
lot of development around the ssl module for the 3.4 release, so you
definitely want to confirm the issue against the hg tip to ensure it
still applies.
|
msg205814 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-12-10 13:46 |
Indeed the example in the docs is wrong, and so is the current behaviour.
The example shows "round-tripping" using ssl.cert_time_to_seconds() and then time.ctime(), except that it is bogus as it takes a GMT time and ctime() returns a local time ("""Convert a time expressed in seconds since the epoch to a string representing local time""").
Still, we should only fix it in 3.4, as code written for prior versions may rely on the current (bogus) behaviour.
|
msg205815 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-12-10 13:48 |
gudge, your contribution is welcome! If you need guidance about how to write a patch, you can read the developer's guide: http://docs.python.org/devguide/
Also you will have to sign a contributor's agreement: http://www.python.org/psf/contrib/
|
msg205860 - (view) |
Author: Akira Li (akira) * |
Date: 2013-12-10 21:37 |
gudge,
There is also an issue with the current strptime format [1] (`"%b %d %H:%M:%S %Y GMT"`). It is locale-dependent and it may fail if a non-English locale is in effect. I don't know whether I should open a new issue on this or are you going to fix it too.
`cert_time_to_seconds()` is documented [2] to parse notBefore, notAfter fields from a certificate. As far as I can tell they do not depend on current locale. Thus the following code should not fail:
>>> timestr = "May 9 00:00:00 2007 GMT"
>>> import ssl
>>> ssl.cert_time_to_seconds(timestr)
1178661600.0
>>> import locale
>>> locale.setlocale(locale.LC_TIME, 'pl_PL.utf8')
'pl_PL.utf8'
>>> ssl.cert_time_to_seconds(timestr)
Traceback (most recent call last):
...[snip]...
ValueError: time data 'May 9 00:00:00 2007 GMT' does not match format '%b %d %H:%M:%S %Y GMT'
[1]: http://hg.python.org/cpython/file/96a68e369d13/Lib/ssl.py#l849
[2]: http://hg.python.org/cpython/file/96a68e369d13/Doc/library/ssl.rst#l359
|
msg206616 - (view) |
Author: gudge (gudge) |
Date: 2013-12-19 15:08 |
1) Can I get a list of failures. The summary of test results which I compare on my machine.
2)
-----------------------------------------------------------------------------------------------------
>>> import ssl
>>> ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT")
1178649000.0
>>> from datetime import datetime
>>> datetime.utcfromtimestamp(1178668800)
datetime.datetime(2007, 5, 9, 0, 0)
>>> import time
>>> time.gmtime(1178668800)
time.struct_time(tm_year=2007, tm_mon=5, tm_mday=9, tm_hour=0, tm_min=0, tm_sec=0, tm_wday=2, tm_yday=129, tm_isdst=0)
>>> import calender
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ImportError: No module named 'calender'
>>> import callendar
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ImportError: No module named 'callendar'
>>> import calendar
>>> calendar.timegm(time.strptime("May 9 00:00:00 2007 GMT", "%b %d %H:%M:%S %Y GMT"))
1178668800
----------------------------------------------------------------------------------------------------
I am running a VM on windows host machine.
In your comment ou have specified:
>>> import ssl
>>> ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT")
1178694000.0
It should be `1178668800`:
But I get also get the same answer with the Python build from latest sources?
Therefore I do not get you?
3)
3 tests omitted:
test___all__ test_site test_urllib2net
348 tests OK.
3 tests failed:
test_codecs test_distutils test_ioctl
2 tests altered the execution environment:
test___all__ test_site
33 tests skipped:
test_bz2 test_codecmaps_cn test_codecmaps_hk test_codecmaps_jp
test_codecmaps_kr test_codecmaps_tw test_curses test_dbm_gnu
test_dbm_ndbm test_devpoll test_gzip test_idle test_kqueue
test_lzma test_msilib test_ossaudiodev test_readline test_smtpnet
test_socketserver test_sqlite test_ssl test_startfile test_tcl
test_timeout test_tk test_ttk_guionly test_ttk_textonly
test_urllibnet test_winreg test_winsound test_xmlrpc_net
test_zipfile64 test_zlib
Are these results fine. These results are with no changes.
How can I make all tests (skipped and omiited pass)
What about the 3 tests which failed. Are these known failures?
4)
Now say I have to pull time again to get the latest code. Does it help
to do a make. Or I will have o do configure again.
5) I had posted a query on core-metorship? No answers? Not that I am entitled to.
Thanks
|
msg206617 - (view) |
Author: gudge (gudge) |
Date: 2013-12-19 15:20 |
Sorry I think I did not read msg205774 (1st comment) correctly.
It clearly says:
"cert_time_to_seconds() uses `time.mktime()` [1] to convert utc time tuple to seconds since epoch. `mktime()` works with local time. It should use `calendar.timegm()` analog instead."
So the function cert_time_to_seconds() has to be fixed?
Thanks
|
msg206620 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-12-19 15:23 |
> So the function cert_time_to_seconds() has to be fixed?
Yes!
|
msg206629 - (view) |
Author: gudge (gudge) |
Date: 2013-12-19 18:38 |
Patch is uploaded.
I will also copy paste it.
I have created the patch with git. Let me know if it is okay with you.
If it is unacceptable I will try and create one for mercury
Patch:
------------------------------------------------------------------
diff --combined Doc/library/ssl.rst
index a6ce5d6,30cb732..0000000
--- a/Doc/library/ssl.rst
+++ b/Doc/library/ssl.rst
@@@ -366,7 -366,7 +366,7 @@@ Certificate handlin
>>> import ssl
>>> ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT")
- 1178694000.0
+ 1178668800
>>> import time
>>> time.ctime(ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT"))
'Wed May 9 00:00:00 2007'
diff --combined Lib/ssl.py
index f81ef91,052a118..0000000
--- a/Lib/ssl.py
+++ b/Lib/ssl.py
@@@ -852,8 -852,7 +852,8 @@@ def cert_time_to_seconds(cert_time)
a Python time value in seconds past the epoch."""
import time
- return time.mktime(time.strptime(cert_time, "%b %d %H:%M:%S %Y GMT"))
+ import calendar
+ return calendar.timegm(time.strptime(cert_time, "%b %d %H:%M:%S %Y GMT"))
PEM_HEADER = "-----BEGIN CERTIFICATE-----"
PEM_FOOTER = "-----END CERTIFICATE-----"
-----------------------------------------------------------------
Test Results:
358 tests OK.
1 test failed:
test_compileall
1 test altered the execution environment:
test___all__
28 tests skipped:
test_codecmaps_cn test_codecmaps_hk test_codecmaps_jp
test_codecmaps_kr test_codecmaps_tw test_curses test_dbm_gnu
test_dbm_ndbm test_devpoll test_idle test_kqueue test_lzma
test_msilib test_ossaudiodev test_smtpnet test_socketserver
test_sqlite test_startfile test_tcl test_timeout test_tk
test_ttk_guionly test_ttk_textonly test_urllibnet test_winreg
test_winsound test_xmlrpc_net test_zipfile64
------------------------------------------------------------------------
Doc changes won't effect the code. The tests would not fail.
How would I check if the doc changes are coming up fine in the
final version.
>>> import ssl
>>> ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT")
1178668800
I do not have a printer curretly. I will sign the license agreement
in a few days.
|
msg206635 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-12-19 19:33 |
Answering to your questions:
> I have created the patch with git. Let me know if it is okay with you.
Yes, it's ok.
Also, please don't copy / paste it. Uploading is enough.
> Doc changes won't effect the code. The tests would not fail.
> How would I check if the doc changes are coming up fine in the
> final version.
The devguide has detailed documentation about how to modify and build
the documentation :)
http://docs.python.org/devguide/documenting.html#building-the-documentation
As for the tests:
1. for this issue you should probably concentrate on test_ssl: to run it
in verbose mode, "./python -m test -v test_ssl"
(please read http://docs.python.org/devguide/runtests.html)
2. you will need to add a new test to test_ssl, to check that this bug
is indeed fixed
|
msg206666 - (view) |
Author: Akira Li (akira) * |
Date: 2013-12-20 06:48 |
gudge, have you seen http://bugs.python.org/msg205860 (the locale issue)?
If you can't fix it; say so, I'll open another issue after this issue is fixed.
|
msg206810 - (view) |
Author: gudge (gudge) |
Date: 2013-12-22 07:31 |
Akira, I will fix it. I will put in the patch in the same bug.
|
msg206825 - (view) |
Author: gudge (gudge) |
Date: 2013-12-22 18:05 |
1) I understand I can run a whole test suite as
./python -m test -v test_abc
as mentioned in
http://docs.python.org/devguide/runtests.html
How do I run a particluar test case, like the test I added
test_cert_time_to_seconds
2) I have a added a test case test_cert_time_to_seconds to test_ssl.py.
3) ./python -m test -v test_ssl
is all PASS.
4) I will start my work on http://bugs.python.org/issue19940#msg205860.
5) The patch is attached.
|
msg207083 - (view) |
Author: gudge (gudge) |
Date: 2013-12-29 18:29 |
Can you please provide some hints on how to handle
http://bugs.python.org/issue19940#msg205860.
The value of format_regex
1) Without locale set:
re.compile('(?P<b>jan|feb|mar|apr|may|jun|jul|aug|sep|oct|nov|dec)\\s+(?P<d>3[0-1]|[1-2]\\d|0[1- 9]|[1-9]| [1-9])\\s+(?P<H>2[0-3]|[0-1]\\d|\\d):(?P<M>[0-5]\\d|\\d):(?P<S>6[0-1]|[0-5]\\d|\\d)\\s +(?P<Y>\\d\\d\\d\\d, re.IGNORECASE)
2) With locale set:
re.compile('(?P<b>sty|lut|mar|kwi|maj|cze|lip|sie|wrz|pa\\ź|lis|gru)\\s+(?P<d>3[0-1]|[1-2]\\d|0[ 1-9]|[1-9]| [1-9])\\s+(?P<H>2[0-3]|[0-1]\\d|\\d):(?P<M>[0-5]\\d|\\d):(?P<S>6[0-1]|[0-5]\\d|\\d)\ \s+(?P<Y>\\d\\d\\d\, re.IGNORECASE)
The value of months are different.
Thanks
|
msg211988 - (view) |
Author: Akira Li (akira) * |
Date: 2014-02-23 10:10 |
The point of the locale issue is that "notBefore", "notAfter" strings do not change if your locale changes. You don't need a new regex for each locale.
I've attached ssl_cert_time_seconds.py file that contains example cert_time_to_seconds(cert_time) implementation that fixes both the timezone and the locale issues.
|
msg211992 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2014-02-23 12:08 |
Akira, do you want to write a proper patch with tests? If you are interested, you can take a look at http://docs.python.org/devguide/
You'll also have to sign a contributor's agreement at http://www.python.org/psf/contrib/contrib-form/
|
msg212022 - (view) |
Author: Akira Li (akira) * |
Date: 2014-02-23 18:54 |
Antoine, I've signed the agreement. I've added ssl_cert_time_toseconds.patch with code, tests, and documention updates.
|
msg212380 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2014-02-27 19:46 |
Akira, thanks. I have posted a review; if you haven't received the e-mail notification, you can still access it at http://bugs.python.org/review/19940/#ps11142
|
msg214637 - (view) |
Author: Akira Li (akira) * |
Date: 2014-03-23 21:51 |
Antoine, I haven't received the e-mail notification.
I've replied to the comments on Rietveld.
Here's the updated patch with the corresponding changes.
|
msg217243 - (view) |
Author: Akira Li (akira) * |
Date: 2014-04-27 09:30 |
Here's a new patch with a simplified ssl.cert_time_to_seconds()
implementation that brings strptime() back.
The behaviour is changed:
- accept both %e and %d strftime formats for days as strptime-based implementation did before
- return an integer instead of a float (input date has not fractions of a second)
I've added more tests.
Please, review.
|
msg217245 - (view) |
Author: Akira Li (akira) * |
Date: 2014-04-27 09:49 |
Replace IndexError with ValueError in the patch because tuple.index raises ValueError.
|
msg217373 - (view) |
Author: Akira Li (akira) * |
Date: 2014-04-28 13:35 |
I've updated the patch:
- fixed the code example in the documentation to use int instead of
float result
- removed assertion on the int returned type (float won't lose precision
for the practical dates but guaranteeing an integer would be nice)
- reworded the scary comment
- removed tests that test the tests
Ready for review.
|
msg217392 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2014-04-28 18:49 |
Thanks for the updated patch, Akira! I'm gonna take a look right now.
|
msg217394 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-04-28 18:57 |
New changeset 7191c37238d5 by Antoine Pitrou in branch 'default':
Issue #19940: ssl.cert_time_to_seconds() now interprets the given time string in the UTC timezone (as specified in RFC 5280), not the local timezone.
http://hg.python.org/cpython/rev/7191c37238d5
|
msg217395 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2014-04-28 18:58 |
I've committed the patch. Thank you very much for contributing!
|
msg217435 - (view) |
Author: Akira Li (akira) * |
Date: 2014-04-28 22:46 |
Antoine, thank you for reviewing. I appreciate the patience.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:55 | admin | set | github: 64139 |
2014-04-28 22:46:14 | akira | set | messages:
+ msg217435 |
2014-04-28 18:58:17 | pitrou | set | status: open -> closed resolution: fixed messages:
+ msg217395
stage: patch review -> resolved |
2014-04-28 18:57:46 | python-dev | set | nosy:
+ python-dev messages:
+ msg217394
|
2014-04-28 18:49:35 | pitrou | set | messages:
+ msg217392 |
2014-04-28 13:35:43 | akira | set | files:
+ ssl_cert_time_to_seconds-ps6.patch
messages:
+ msg217373 |
2014-04-27 09:49:47 | akira | set | files:
+ ssl_cert_time_to_seconds-ps5.patch
messages:
+ msg217245 |
2014-04-27 09:30:49 | akira | set | files:
+ ssl_cert_time_to_seconds-462470859e57.patch
messages:
+ msg217243 |
2014-03-23 21:51:24 | akira | set | files:
+ ssl_cert_time_to_seconds-ps3.patch
messages:
+ msg214637 |
2014-02-27 19:46:15 | pitrou | set | stage: needs patch -> patch review messages:
+ msg212380 versions:
+ Python 3.5, - Python 3.4 |
2014-02-23 18:54:20 | akira | set | files:
+ ssl_cert_time_to_seconds.patch keywords:
+ patch messages:
+ msg212022
|
2014-02-23 12:08:53 | pitrou | set | messages:
+ msg211992 |
2014-02-23 10:10:11 | akira | set | files:
+ ssl_cert_time_seconds.py
messages:
+ msg211988 |
2013-12-29 18:29:54 | gudge | set | messages:
+ msg207083 |
2013-12-22 18:05:11 | gudge | set | files:
+ patch.txt
messages:
+ msg206825 |
2013-12-22 07:31:39 | gudge | set | messages:
+ msg206810 |
2013-12-20 06:48:26 | akira | set | messages:
+ msg206666 |
2013-12-19 19:33:29 | pitrou | set | messages:
+ msg206635 |
2013-12-19 18:38:12 | gudge | set | files:
+ patch.txt
messages:
+ msg206629 |
2013-12-19 15:23:42 | pitrou | set | messages:
+ msg206620 |
2013-12-19 15:20:31 | gudge | set | messages:
+ msg206617 |
2013-12-19 15:08:07 | gudge | set | messages:
+ msg206616 |
2013-12-10 21:37:28 | akira | set | messages:
+ msg205860 |
2013-12-10 20:04:07 | giampaolo.rodola | set | nosy:
- giampaolo.rodola
|
2013-12-10 13:48:13 | pitrou | set | messages:
+ msg205815 |
2013-12-10 13:46:54 | pitrou | set | messages:
+ msg205814 stage: needs patch |
2013-12-10 13:43:08 | pitrou | set | assignee: docs@python -> components:
- Documentation |
2013-12-10 13:42:55 | pitrou | set | nosy:
+ janssen, pitrou, giampaolo.rodola, christian.heimes
versions:
- Python 2.7, Python 3.3, Python 3.5 |
2013-12-10 12:10:46 | gudge | set | nosy:
+ gudge messages:
+ msg205803
|
2013-12-10 12:02:03 | tim.golden | set | versions:
- Python 2.6, Python 3.1, Python 3.2 |
2013-12-10 07:29:12 | akira | create | |