From 38eb748b0aab32182c2704a5e16820fa16571b6c Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Thu, 16 Apr 2015 11:35:00 -0400 Subject: [PATCH 1/5] Reactivate cleanup of temp files --- OpenSSL/test/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OpenSSL/test/util.py b/OpenSSL/test/util.py index b8be91deb..12e5eb844 100644 --- a/OpenSSL/test/util.py +++ b/OpenSSL/test/util.py @@ -163,7 +163,7 @@ def tearDown(self): Subclasses must invoke this method if they override it or the cleanup will not occur. """ - if False and self._temporaryFiles is not None: + if self._temporaryFiles is not None: for temp in self._temporaryFiles: if os.path.isdir(temp): shutil.rmtree(temp) From 72e1e5abee17067ba87383d7fbb00cda12ef25e5 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Thu, 16 Apr 2015 13:05:27 -0400 Subject: [PATCH 2/5] Add suffix option to TestCase.mktemp Generating tmp files properly ensures the cleanup. --- OpenSSL/test/test_rand.py | 5 ++--- OpenSSL/test/test_ssl.py | 26 ++++++++++++++------------ OpenSSL/test/util.py | 4 ++-- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/OpenSSL/test/test_rand.py b/OpenSSL/test/test_rand.py index 3d5c2906d..f86e4bf9c 100644 --- a/OpenSSL/test/test_rand.py +++ b/OpenSSL/test/test_rand.py @@ -205,8 +205,7 @@ def test_bytes_paths(self): Random data can be saved and loaded to files with paths specified as bytes. """ - path = self.mktemp() - path += NON_ASCII.encode(sys.getfilesystemencoding()) + path = self.mktemp(suffix=NON_ASCII) self._read_write_test(path) @@ -215,7 +214,7 @@ def test_unicode_paths(self): Random data can be saved and loaded to files with paths specified as unicode. """ - path = self.mktemp().decode('utf-8') + NON_ASCII + path = self.mktemp(suffix=NON_ASCII).decode('utf-8') self._read_write_test(path) diff --git a/OpenSSL/test/test_ssl.py b/OpenSSL/test/test_ssl.py index bb1c9aedf..808b8a656 100644 --- a/OpenSSL/test/test_ssl.py +++ b/OpenSSL/test/test_ssl.py @@ -436,7 +436,7 @@ def test_use_privatekey_file_bytes(self): instance giving the file name to ``Context.use_privatekey_file``. """ self._use_privatekey_file_test( - self.mktemp() + NON_ASCII.encode(getfilesystemencoding()), + self.mktemp(suffix=NON_ASCII), FILETYPE_PEM, ) @@ -447,7 +447,7 @@ def test_use_privatekey_file_unicode(self): instance giving the file name to ``Context.use_privatekey_file``. """ self._use_privatekey_file_test( - self.mktemp().decode(getfilesystemencoding()) + NON_ASCII, + self.mktemp(suffix=NON_ASCII).decode(getfilesystemencoding()), FILETYPE_PEM, ) @@ -545,7 +545,7 @@ def test_use_certificate_file_bytes(self): ``bytes`` filename) which will be used to identify connections created using the context. """ - filename = self.mktemp() + NON_ASCII.encode(getfilesystemencoding()) + filename = self.mktemp(suffix=NON_ASCII) self._use_certificate_file_test(filename) @@ -555,7 +555,9 @@ def test_use_certificate_file_unicode(self): ``bytes`` filename) which will be used to identify connections created using the context. """ - filename = self.mktemp().decode(getfilesystemencoding()) + NON_ASCII + filename = self.mktemp( + suffix=NON_ASCII + ).decode(getfilesystemencoding()) self._use_certificate_file_test(filename) @@ -990,7 +992,7 @@ def test_load_verify_bytes_cafile(self): ``bytes`` instance and uses the certificates within for verification purposes. """ - cafile = self.mktemp() + NON_ASCII.encode(getfilesystemencoding()) + cafile = self.mktemp(suffix=NON_ASCII) self._load_verify_cafile(cafile) @@ -1001,7 +1003,7 @@ def test_load_verify_unicode_cafile(self): purposes. """ self._load_verify_cafile( - self.mktemp().decode(getfilesystemencoding()) + NON_ASCII + self.mktemp(suffix=NON_ASCII).decode(getfilesystemencoding()) ) @@ -1041,7 +1043,7 @@ def test_load_verify_directory_bytes_capath(self): purposes. """ self._load_verify_directory_locations_capath( - self.mktemp() + NON_ASCII.encode(getfilesystemencoding()) + self.mktemp(suffix=NON_ASCII) ) @@ -1052,7 +1054,7 @@ def test_load_verify_directory_unicode_capath(self): purposes. """ self._load_verify_directory_locations_capath( - self.mktemp().decode(getfilesystemencoding()) + NON_ASCII + self.mktemp(suffix=NON_ASCII) ) @@ -1287,7 +1289,7 @@ def test_use_certificate_chain_file_bytes(self): construct and verify a trust chain. """ self._use_certificate_chain_file_test( - self.mktemp() + NON_ASCII.encode(getfilesystemencoding()) + self.mktemp(suffix=NON_ASCII) ) @@ -1298,7 +1300,7 @@ def test_use_certificate_chain_file_unicode(self): to construct and verify a trust chain. """ self._use_certificate_chain_file_test( - self.mktemp().decode(getfilesystemencoding()) + NON_ASCII + self.mktemp(suffix=NON_ASCII) ) @@ -1395,7 +1397,7 @@ def test_load_tmp_dh_bytes(self): specified file (given as ``bytes``). """ self._load_tmp_dh_test( - self.mktemp() + NON_ASCII.encode(getfilesystemencoding()), + self.mktemp(suffix=NON_ASCII) ) @@ -1405,7 +1407,7 @@ def test_load_tmp_dh_unicode(self): specified file (given as ``unicode``). """ self._load_tmp_dh_test( - self.mktemp().decode(getfilesystemencoding()) + NON_ASCII, + self.mktemp(suffix=NON_ASCII).decode(getfilesystemencoding()) ) diff --git a/OpenSSL/test/util.py b/OpenSSL/test/util.py index 12e5eb844..c26a57a8d 100644 --- a/OpenSSL/test/util.py +++ b/OpenSSL/test/util.py @@ -296,13 +296,13 @@ def failUnlessRaises(self, exception, f, *args, **kwargs): _temporaryFiles = None - def mktemp(self): + def mktemp(self, suffix=""): """ Pathetic substitute for twisted.trial.unittest.TestCase.mktemp. """ if self._temporaryFiles is None: self._temporaryFiles = [] - temp = b(mktemp(dir=".")) + temp = mktemp(suffix=suffix, dir=".").encode("utf-8") self._temporaryFiles.append(temp) return temp From 4813c0e0284ca4224ca8005f89650bcdaa18cd10 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Thu, 16 Apr 2015 13:38:01 -0400 Subject: [PATCH 3/5] Use tmp dir instead of keeping track of files This allows to clean up behind ourselves more effectively and requires less logic. --- OpenSSL/test/test_rand.py | 5 ++-- OpenSSL/test/test_ssl.py | 44 ++++++++++++++-------------- OpenSSL/test/util.py | 61 ++++++++++++++++++++++++--------------- 3 files changed, 63 insertions(+), 47 deletions(-) diff --git a/OpenSSL/test/test_rand.py b/OpenSSL/test/test_rand.py index f86e4bf9c..3d5c2906d 100644 --- a/OpenSSL/test/test_rand.py +++ b/OpenSSL/test/test_rand.py @@ -205,7 +205,8 @@ def test_bytes_paths(self): Random data can be saved and loaded to files with paths specified as bytes. """ - path = self.mktemp(suffix=NON_ASCII) + path = self.mktemp() + path += NON_ASCII.encode(sys.getfilesystemencoding()) self._read_write_test(path) @@ -214,7 +215,7 @@ def test_unicode_paths(self): Random data can be saved and loaded to files with paths specified as unicode. """ - path = self.mktemp(suffix=NON_ASCII).decode('utf-8') + path = self.mktemp().decode('utf-8') + NON_ASCII self._read_write_test(path) diff --git a/OpenSSL/test/test_ssl.py b/OpenSSL/test/test_ssl.py index 808b8a656..38b566fe6 100644 --- a/OpenSSL/test/test_ssl.py +++ b/OpenSSL/test/test_ssl.py @@ -436,7 +436,7 @@ def test_use_privatekey_file_bytes(self): instance giving the file name to ``Context.use_privatekey_file``. """ self._use_privatekey_file_test( - self.mktemp(suffix=NON_ASCII), + self.mktemp() + NON_ASCII.encode(getfilesystemencoding()), FILETYPE_PEM, ) @@ -447,7 +447,7 @@ def test_use_privatekey_file_unicode(self): instance giving the file name to ``Context.use_privatekey_file``. """ self._use_privatekey_file_test( - self.mktemp(suffix=NON_ASCII).decode(getfilesystemencoding()), + self.mktemp().decode(getfilesystemencoding()) + NON_ASCII, FILETYPE_PEM, ) @@ -545,7 +545,7 @@ def test_use_certificate_file_bytes(self): ``bytes`` filename) which will be used to identify connections created using the context. """ - filename = self.mktemp(suffix=NON_ASCII) + filename = self.mktemp() + NON_ASCII.encode(getfilesystemencoding()) self._use_certificate_file_test(filename) @@ -555,9 +555,7 @@ def test_use_certificate_file_unicode(self): ``bytes`` filename) which will be used to identify connections created using the context. """ - filename = self.mktemp( - suffix=NON_ASCII - ).decode(getfilesystemencoding()) + filename = self.mktemp().decode(getfilesystemencoding()) + NON_ASCII self._use_certificate_file_test(filename) @@ -992,7 +990,7 @@ def test_load_verify_bytes_cafile(self): ``bytes`` instance and uses the certificates within for verification purposes. """ - cafile = self.mktemp(suffix=NON_ASCII) + cafile = self.mktemp() + NON_ASCII.encode(getfilesystemencoding()) self._load_verify_cafile(cafile) @@ -1003,7 +1001,7 @@ def test_load_verify_unicode_cafile(self): purposes. """ self._load_verify_cafile( - self.mktemp(suffix=NON_ASCII).decode(getfilesystemencoding()) + self.mktemp().decode(getfilesystemencoding()) + NON_ASCII ) @@ -1043,7 +1041,7 @@ def test_load_verify_directory_bytes_capath(self): purposes. """ self._load_verify_directory_locations_capath( - self.mktemp(suffix=NON_ASCII) + self.mktemp() + NON_ASCII.encode(getfilesystemencoding()) ) @@ -1054,7 +1052,7 @@ def test_load_verify_directory_unicode_capath(self): purposes. """ self._load_verify_directory_locations_capath( - self.mktemp(suffix=NON_ASCII) + self.mktemp().decode(getfilesystemencoding()) + NON_ASCII ) @@ -1201,11 +1199,11 @@ def verify_callback(*args): def test_add_extra_chain_cert(self): """ - :py:obj:`Context.add_extra_chain_cert` accepts an :py:obj:`X509` instance to add to - the certificate chain. + :py:obj:`Context.add_extra_chain_cert` accepts an :py:obj:`X509` + instance to add to the certificate chain. - See :py:obj:`_create_certificate_chain` for the details of the certificate - chain tested. + See :py:obj:`_create_certificate_chain` for the details of the + certificate chain tested. The chain is tested by starting a server with scert and connecting to it with a client which trusts cacert and requires verification to @@ -1216,13 +1214,15 @@ def test_add_extra_chain_cert(self): # Dump the CA certificate to a file because that's the only way to load # it as a trusted CA in the client context. - for cert, name in [(cacert, 'ca.pem'), (icert, 'i.pem'), (scert, 's.pem')]: - fObj = open(name, 'w') + for cert, name in [(cacert, 'ca.pem'), + (icert, 'i.pem'), + (scert, 's.pem')]: + fObj = open(join(self.tmpdir, name), 'w') fObj.write(dump_certificate(FILETYPE_PEM, cert).decode('ascii')) fObj.close() for key, name in [(cakey, 'ca.key'), (ikey, 'i.key'), (skey, 's.key')]: - fObj = open(name, 'w') + fObj = open(join(self.tmpdir, name), 'w') fObj.write(dump_privatekey(FILETYPE_PEM, key).decode('ascii')) fObj.close() @@ -1237,7 +1237,7 @@ def test_add_extra_chain_cert(self): clientContext = Context(TLSv1_METHOD) clientContext.set_verify( VERIFY_PEER | VERIFY_FAIL_IF_NO_PEER_CERT, verify_cb) - clientContext.load_verify_locations(b"ca.pem") + clientContext.load_verify_locations(join(self.tmpdir, "ca.pem")) # Try it out. self._handshake_test(serverContext, clientContext) @@ -1289,7 +1289,7 @@ def test_use_certificate_chain_file_bytes(self): construct and verify a trust chain. """ self._use_certificate_chain_file_test( - self.mktemp(suffix=NON_ASCII) + self.mktemp() + NON_ASCII.encode(getfilesystemencoding()) ) @@ -1300,7 +1300,7 @@ def test_use_certificate_chain_file_unicode(self): to construct and verify a trust chain. """ self._use_certificate_chain_file_test( - self.mktemp(suffix=NON_ASCII) + self.mktemp().decode(getfilesystemencoding()) + NON_ASCII ) @@ -1397,7 +1397,7 @@ def test_load_tmp_dh_bytes(self): specified file (given as ``bytes``). """ self._load_tmp_dh_test( - self.mktemp(suffix=NON_ASCII) + self.mktemp() + NON_ASCII.encode(getfilesystemencoding()), ) @@ -1407,7 +1407,7 @@ def test_load_tmp_dh_unicode(self): specified file (given as ``unicode``). """ self._load_tmp_dh_test( - self.mktemp(suffix=NON_ASCII).decode(getfilesystemencoding()) + self.mktemp().decode(getfilesystemencoding()) + NON_ASCII, ) diff --git a/OpenSSL/test/util.py b/OpenSSL/test/util.py index c26a57a8d..78b4a3fc9 100644 --- a/OpenSSL/test/util.py +++ b/OpenSSL/test/util.py @@ -7,18 +7,20 @@ U{Twisted}. """ +import os import shutil +import sys import traceback -import os, os.path -from tempfile import mktemp + +from tempfile import mktemp, mkdtemp from unittest import TestCase -import sys from six import PY3 from OpenSSL._util import exception_from_error_queue from OpenSSL.crypto import Error + try: import memdbg except Exception: @@ -28,14 +30,16 @@ class _memdbg(object): heap = None from OpenSSL._util import ffi, lib, byte_string as b + # This is the UTF-8 encoding of the SNOWMAN unicode code point. NON_ASCII = b("\xe2\x98\x83").decode("utf-8") + class TestCase(TestCase): """ - :py:class:`TestCase` adds useful testing functionality beyond what is available - from the standard library :py:class:`unittest.TestCase`. + :py:class:`TestCase` adds useful testing functionality beyond what is + available from the standard library :py:class:`unittest.TestCase`. """ def run(self, result): run = super(TestCase, self).run @@ -157,24 +161,38 @@ def format_leak(p): (None, Exception(stack % (allocs_report,)), None)) + _tmpdir = None + + + @property + def tmpdir(self): + """ + On demand create a temporary directory. + """ + if self._tmpdir is not None: + return self._tmpdir + + self._tmpdir = mkdtemp(dir=".") + return self._tmpdir + + def tearDown(self): """ - Clean up any files or directories created using :py:meth:`TestCase.mktemp`. - Subclasses must invoke this method if they override it or the - cleanup will not occur. + Clean up any files or directories created using + :py:meth:`TestCase.mktemp`. Subclasses must invoke this method if they + override it or the cleanup will not occur. """ - if self._temporaryFiles is not None: - for temp in self._temporaryFiles: - if os.path.isdir(temp): - shutil.rmtree(temp) - elif os.path.exists(temp): - os.unlink(temp) + if self._tmpdir is not None: + shutil.rmtree(self._tmpdir) + try: exception_from_error_queue(Error) except Error: e = sys.exc_info()[1] if e.args != ([],): - self.fail("Left over errors in OpenSSL error queue: " + repr(e)) + self.fail( + "Left over errors in OpenSSL error queue: " + repr(e) + ) def assertIsInstance(self, instance, classOrTuple, message=None): @@ -295,16 +313,13 @@ def failUnlessRaises(self, exception, f, *args, **kwargs): assertRaises = failUnlessRaises - _temporaryFiles = None - def mktemp(self, suffix=""): + def mktemp(self): """ - Pathetic substitute for twisted.trial.unittest.TestCase.mktemp. + Return UTF-8-encoded bytes of a path to a tmp file. + + The file will be cleaned up after the test run. """ - if self._temporaryFiles is None: - self._temporaryFiles = [] - temp = mktemp(suffix=suffix, dir=".").encode("utf-8") - self._temporaryFiles.append(temp) - return temp + return mktemp(dir=self.tmpdir).encode("utf-8") # Other stuff From 1902c01569f648e6f085a8fbb018298f90085423 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Thu, 16 Apr 2015 15:06:41 -0400 Subject: [PATCH 4/5] Consistent layout --- OpenSSL/test/test_ssl.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/OpenSSL/test/test_ssl.py b/OpenSSL/test/test_ssl.py index 38b566fe6..b104a098e 100644 --- a/OpenSSL/test/test_ssl.py +++ b/OpenSSL/test/test_ssl.py @@ -1221,7 +1221,9 @@ def test_add_extra_chain_cert(self): fObj.write(dump_certificate(FILETYPE_PEM, cert).decode('ascii')) fObj.close() - for key, name in [(cakey, 'ca.key'), (ikey, 'i.key'), (skey, 's.key')]: + for key, name in [(cakey, 'ca.key'), + (ikey, 'i.key'), + (skey, 's.key')]: fObj = open(join(self.tmpdir, name), 'w') fObj.write(dump_privatekey(FILETYPE_PEM, key).decode('ascii')) fObj.close() From e90680fe390d2053d050333b89ff4c0826f30c6e Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Thu, 16 Apr 2015 15:09:27 -0400 Subject: [PATCH 5/5] Use context managers for better readability --- OpenSSL/test/test_ssl.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/OpenSSL/test/test_ssl.py b/OpenSSL/test/test_ssl.py index b104a098e..1f231c9c0 100644 --- a/OpenSSL/test/test_ssl.py +++ b/OpenSSL/test/test_ssl.py @@ -1217,16 +1217,14 @@ def test_add_extra_chain_cert(self): for cert, name in [(cacert, 'ca.pem'), (icert, 'i.pem'), (scert, 's.pem')]: - fObj = open(join(self.tmpdir, name), 'w') - fObj.write(dump_certificate(FILETYPE_PEM, cert).decode('ascii')) - fObj.close() + with open(join(self.tmpdir, name), 'w') as f: + f.write(dump_certificate(FILETYPE_PEM, cert).decode('ascii')) for key, name in [(cakey, 'ca.key'), (ikey, 'i.key'), (skey, 's.key')]: - fObj = open(join(self.tmpdir, name), 'w') - fObj.write(dump_privatekey(FILETYPE_PEM, key).decode('ascii')) - fObj.close() + with open(join(self.tmpdir, name), 'w') as f: + f.write(dump_privatekey(FILETYPE_PEM, key).decode('ascii')) # Create the server context serverContext = Context(TLSv1_METHOD)