_ssl: support surrogates in filenames, and bytes/bytearray filenames

[vstinner]

BPO	8477
Nosy	@pitrou, @vstinner
Files	ssl_surrogates-2.patch

^{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 = None
closed_at = <Date 2010-05-16.21:26:17.320>
created_at = <Date 2010-04-20.23:38:37.526>
labels = ['library', 'expert-unicode']
title = '_ssl: support surrogates in filenames, and bytes/bytearray filenames'
updated_at = <Date 2010-05-16.21:38:32.630>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2010-05-16.21:38:32.630>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2010-05-16.21:26:17.320>
closer = 'vstinner'
components = ['Library (Lib)', 'Unicode']
creation = <Date 2010-04-20.23:38:37.526>
creator = 'vstinner'
dependencies = []
files = ['17051']
hgrepos = []
issue_num = 8477
keywords = ['patch']
message_count = 7.0
messages = ['103793', '103794', '104005', '104011', '104936', '105878', '105880']
nosy_count = 2.0
nosy_names = ['pitrou', 'vstinner']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue8477'
versions = ['Python 2.7', 'Python 3.2']

[vstinner]

_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.

[pitrou]

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

[vstinner]

key_file and cert_file are mandatory (see newPySSLObject()) and bytearray are now more accepted by PyUnicode_FSConverter() (see bpo-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).

[pitrou]

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.

[pitrou]

Most of this should be solved if the patch in bpo-8550 gets accepted. As for test_decode_certificate, it seems it isn't used anywhere, and could therefore be deleted.

[vstinner]

I commited the fix on _ssl._test_decode_cert() as r81237 (py3k, blocked in 3.1: r81238). Issue bpo-8550 got accepted and uses PyUnicode_FSConverter for the filenames (except _ssl._test_decode_cert() which is now fixed).

[vstinner]

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 :-)