classification
Title: _ssl: support surrogates in filenames, and bytes/bytearray filenames
Type: Stage:
Components: Library (Lib), Unicode Versions: Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: pitrou, vstinner
Priority: normal Keywords: patch

Created on 2010-04-20 23:38 by vstinner, last changed 2010-05-16 21:38 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
ssl_surrogates-2.patch vstinner, 2010-04-23 11:37
Messages (7)
msg103793 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2010-04-20 23:38
_ssl.sslwrap() has 3 filename arguments: key_file, cert_file and cacerts_file. It uses "z" format to parse them.

Attached patch uses PyUnicode_FSConverter() to support surrogates, bytes and bytearray. It fixes also test_decode_certificate() function.
msg103794 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-20 23:45
Some comments:
- your ssl_convert_filename() function has a strange signature
- using PyByteArray_AsString() is wrong, we release the GIL later and the buffer could be reallocated, leading to a crash; the right approach is to use PyObject_GetBuffer() and later PyBuffer_Release()
- as such, your function could simply return the bytes/bytearray PyObject *, and not fill the char * pointer
- adding tests for both the bytes and unicode cases would be nice
msg104005 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2010-04-23 11:37
key_file and cert_file are mandatory (see newPySSLObject()) and bytearray are now more accepted by PyUnicode_FSConverter() (see #8485). New version of the patch is simpler and shorter.

Support bytearray is a little bit to much complex for me (because of the GIL and bytearray lock), I prefer to leave 3.1 unchanged (I will not backport this patch).
msg104011 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-23 13:10
> key_file and cert_file are mandatory

No, they are not. Not for a client connection, at least.
Also, please add at least a simple test.
msg104936 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-04 13:57
Most of this should be solved if the patch in issue8550 gets accepted. As for test_decode_certificate, it seems it isn't used anywhere, and could therefore be deleted.
msg105878 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2010-05-16 21:26
I commited the fix on _ssl._test_decode_cert() as r81237 (py3k, blocked in 3.1: r81238). Issue #8550 got accepted and uses PyUnicode_FSConverter for the filenames (except _ssl._test_decode_cert() which is now fixed).
msg105880 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2010-05-16 21:38
Oh, I forgot to patch RAND_egd()! Fixed: r81239 (py3k, blocked in 3.1: r81240).

The whole ssl module should now be fully "str+surrogates/bytes filenames" compliant :-)
History
Date User Action Args
2010-05-16 21:38:32vstinnersetmessages: + msg105880
2010-05-16 21:26:17vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg105878
2010-05-04 13:57:44pitrousetmessages: + msg104936
2010-04-23 13:10:14pitrousetmessages: + msg104011
2010-04-23 11:38:07vstinnersetfiles: - ssl_surrogates.patch
2010-04-23 11:37:41vstinnersetfiles: + ssl_surrogates-2.patch

messages: + msg104005
2010-04-20 23:45:38pitrousetnosy: + pitrou
messages: + msg103794
2010-04-20 23:38:50vstinnerlinkissue8242 dependencies
2010-04-20 23:38:37vstinnercreate