From 9cff280b248dad80ffcaf4b2b626645ce02ee997 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Thu, 8 Oct 2020 12:24:04 +0300 Subject: [PATCH 01/12] Remove test_slow_retrieval expected failure test Remove the test with mode 2 ('mode_2': During the download process, the server blocks the download by sending just several characters every few seconds.) from test_slow_retrieval. This test is marked as "expected failure" with the purpose of rewriting it one day, but slow retrievals have been removed from the specification and soon it will be removed from the tuf reference implementation as a whole. That means that the chances of making this test useful are close to 0 if not none. The other test (with mode 1) in test_slow_retrieval is not removed. For reference: - https://github.com/theupdateframework/specification/pull/111 - https://github.com/theupdateframework/tuf/pull/1156 Signed-off-by: Martin Vrachev --- tests/slow_retrieval_server.py | 42 ++------- tests/test_slow_retrieval_attack.py | 130 +++++++--------------------- 2 files changed, 37 insertions(+), 135 deletions(-) diff --git a/tests/slow_retrieval_server.py b/tests/slow_retrieval_server.py index 61e5c4745a..e2c525d2da 100755 --- a/tests/slow_retrieval_server.py +++ b/tests/slow_retrieval_server.py @@ -37,14 +37,6 @@ import six -# Modify the HTTPServer class to pass the 'test_mode' argument to -# do_GET() function. -class HTTPServer_Test(six.moves.BaseHTTPServer.HTTPServer): - def __init__(self, server_address, Handler, test_mode): - six.moves.BaseHTTPServer.HTTPServer.__init__(self, server_address, Handler) - self.test_mode = test_mode - - # HTTP request handler. class Handler(six.moves.BaseHTTPServer.BaseHTTPRequestHandler): @@ -62,38 +54,18 @@ def do_GET(self): self.send_header('Content-length', str(len(data))) self.end_headers() - if self.server.test_mode == 'mode_1': - # Before sending any data, the server does nothing for a long time. - DELAY = 40 - time.sleep(DELAY) - self.wfile.write(data) - - return - - # 'mode_2' - else: - DELAY = 1 - # Throttle the file by sending a character every DELAY seconds. - for i in range(len(data)): - self.wfile.write(data[i].encode('utf-8')) - time.sleep(DELAY) - - return + # Before sending any data, the server does nothing for a long time. + DELAY = 40 + time.sleep(DELAY) + self.wfile.write((data.encode('utf-8'))) except IOError as e: self.send_error(404, 'File Not Found!') -def run(port, test_mode): - server_address = ('localhost', port) - httpd = HTTPServer_Test(server_address, Handler, test_mode) - httpd.handle_request() - - - if __name__ == '__main__': port = int(sys.argv[1]) - test_mode = sys.argv[2] - assert test_mode in ('mode_1', 'mode_2') - run(port, test_mode) + server_address = ('localhost', port) + httpd = six.moves.BaseHTTPServer.HTTPServer(server_address, Handler) + httpd.handle_request() diff --git a/tests/test_slow_retrieval_attack.py b/tests/test_slow_retrieval_attack.py index 4407b59ffc..adb01f157e 100755 --- a/tests/test_slow_retrieval_attack.py +++ b/tests/test_slow_retrieval_attack.py @@ -68,55 +68,8 @@ repo_tool.disable_console_log_messages() -class TestSlowRetrievalAttack(unittest_toolbox.Modified_TestCase): - - @classmethod - def setUpClass(cls): - # Create a temporary directory to store the repository, metadata, and target - # files. 'temporary_directory' must be deleted in TearDownModule() so that - # temporary files are always removed, even when exceptions occur. - cls.temporary_directory = tempfile.mkdtemp(dir=os.getcwd()) - cls.SERVER_PORT = random.randint(30000, 45000) - - - - @classmethod - def tearDownClass(cls): - # Remove the temporary repository directory, which should contain all the - # metadata, targets, and key files generated of all the test cases. - shutil.rmtree(cls.temporary_directory) - - - - def _start_slow_server(self, mode): - # Launch a SimpleHTTPServer (serves files in the current directory). - # Test cases will request metadata and target files that have been - # pre-generated in 'tuf/tests/repository_data', which will be served by the - # SimpleHTTPServer launched here. The test cases of this unit test assume - # the pre-generated metadata files have a specific structure, such - # as a delegated role 'targets/role1', three target files, five key files, - # etc. - self.server_process_handler = utils.TestServerProcess(log=logger, - server='slow_retrieval_server.py', port=self.SERVER_PORT, - timeout=0, extra_cmd_args=[mode]) - - logger.info('Slow Retrieval Server process started.') - - # NOTE: Following error is raised if a delay is not long enough: - # - # or, on Windows: - # Failed to establish a new connection: [Errno 111] Connection refused' - # 1s led to occasional failures in automated builds on AppVeyor, so - # increasing this to 3s, sadly. - time.sleep(3) - - - - def _stop_slow_server(self): - # Logs stdout and stderr from the server subprocess and then it - # kills it and closes the temp file used for logging. - self.server_process_handler.clean() +class TestSlowRetrieval(unittest_toolbox.Modified_TestCase): def setUp(self): # We are inheriting from custom class. @@ -124,6 +77,11 @@ def setUp(self): self.repository_name = 'test_repository1' + # Create a temporary directory to store the repository, metadata, and target + # files. 'temporary_directory' must be deleted in TearDownModule() so that + # temporary files are always removed, even when exceptions occur. + self.temporary_directory = tempfile.mkdtemp(dir=os.getcwd()) + # Copy the original repository files provided in the test folder so that # any modifications made to repository files are restricted to the copies. # The 'repository_data' directory is expected to exist in 'tuf/tests/'. @@ -209,8 +167,22 @@ def setUp(self): # Set the url prefix required by the 'tuf/client/updater.py' updater. # 'path/to/tmp/repository' -> 'localhost:8001/tmp/repository'. repository_basepath = self.repository_directory[len(os.getcwd()):] - url_prefix = \ - 'http://localhost:' + str(self.SERVER_PORT) + repository_basepath + + self.server_process_handler = utils.TestServerProcess(log=logger, + server='slow_retrieval_server.py', timeout=0) + + logger.info('Slow Retrieval Server process started.') + + # NOTE: Following error is raised if a delay is not long enough: + # + # or, on Windows: + # Failed to establish a new connection: [Errno 111] Connection refused' + # 1s led to occasional failures in automated builds on AppVeyor, so + # increasing this to 3s, sadly. + time.sleep(3) + + url_prefix = 'http://localhost:' \ + + str(self.server_process_handler.port) + repository_basepath # Setting 'tuf.settings.repository_directory' with the temporary client # directory copied from the original repository files. @@ -233,16 +205,21 @@ def tearDown(self): tuf.roledb.clear_roledb(clear_all=True) tuf.keydb.clear_keydb(clear_all=True) + # Logs stdout and stderr from the server subprocess and then it + # kills it and closes the temp file used for logging. + self.server_process_handler.clean() + # Remove the temporary repository directory, which should contain all the + # metadata, targets, and key files generated of all the test cases. + shutil.rmtree(self.temporary_directory) - def test_with_tuf_mode_1(self): + + def test_delay_before_send(self): # Simulate a slow retrieval attack. # 'mode_1': When download begins,the server blocks the download for a long # time by doing nothing before it sends the first byte of data. - self._start_slow_server('mode_1') - # Verify that the TUF client detects replayed metadata and refuses to # continue the update process. client_filepath = os.path.join(self.client_directory, 'file1.txt') @@ -264,53 +241,6 @@ def test_with_tuf_mode_1(self): else: self.fail('TUF did not prevent a slow retrieval attack.') - finally: - self._stop_slow_server() - - - - # The following test fails as a result of a change to TUF's download code. - # Rather than constructing urllib2 requests, we now use the requests library. - # This solves an HTTPS proxy issue, but has for the moment deprived us of a - # way to prevent certain this kind of slow retrieval attack. - # See conversation in PR: https://github.com/theupdateframework/tuf/pull/781 - # TODO: Update download code to resolve the slow retrieval vulnerability. - @unittest.expectedFailure - def test_with_tuf_mode_2(self): - # Simulate a slow retrieval attack. - # 'mode_2': During the download process, the server blocks the download - # by sending just several characters every few seconds. - - self._start_slow_server('mode_2') - client_filepath = os.path.join(self.client_directory, 'file1.txt') - original_average_download_speed = tuf.settings.MIN_AVERAGE_DOWNLOAD_SPEED - tuf.settings.MIN_AVERAGE_DOWNLOAD_SPEED = 3 - - try: - file1_target = self.repository_updater.get_one_valid_targetinfo('file1.txt') - self.repository_updater.download_target(file1_target, self.client_directory) - - # Verify that the specific 'tuf.exceptions.SlowRetrievalError' exception is - # raised by each mirror. 'file1.txt' should be large enough to trigger a - # slow retrieval attack, otherwise the expected exception may not be - # consistently raised. - except tuf.exceptions.NoWorkingMirrorError as exception: - for mirror_url, mirror_error in six.iteritems(exception.mirror_errors): - url_prefix = self.repository_mirrors['mirror1']['url_prefix'] - url_file = os.path.join(url_prefix, 'targets', 'file1.txt') - - # Verify that 'file1.txt' is the culprit. - self.assertEqual(url_file.replace('\\', '/'), mirror_url) - self.assertTrue(isinstance(mirror_error, tuf.exceptions.SlowRetrievalError)) - - else: - # Another possibility is to check for a successfully downloaded - # 'file1.txt' at this point. - self.fail('TUF did not prevent a slow retrieval attack.') - - finally: - self._stop_slow_server() - tuf.settings.MIN_AVERAGE_DOWNLOAD_SPEED = original_average_download_speed if __name__ == '__main__': From f8730aec8818a109f347e2db2e3add8d5dd81aeb Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Tue, 27 Oct 2020 17:28:01 +0200 Subject: [PATCH 02/12] Delegate port generation for the tests to the OS By giving 0 to the port argument we ask the OS to give us an arbitrary unused port. This method is much better than if we generate a port, because even though we are generating a random port there is always the chance that the port could be already in use. In this commit, I also make sure that there is no place in the tests where we are manually generating and passing ports. Also, because none of the tests pass a port to the TestServerProcess class and its __init__ function I have removed the "port" argument. Finally, until now slow_retrieval.py couldn't use the TestServerProcess class from utils.py port generation because we were using httpd.handle_request() which handles only ONE request. Then, what happened was that when we use wait_for_server() to make a test connection and verify that the server is up, the slow_retrieval server handles that connection (which it accepts as a request) and exits. We avoided that use-case by passing timeout = 0 and avoiding calling wait_for_server() on this special value. Now, when we use httpd.serve_forever() this problem is resolved and no longer we need to make those checks. Signed-off-by: Martin Vrachev --- tests/proxy_server.py | 31 ++--- tests/repository_data/map.json | 8 +- tests/simple_https_server.py | 33 +++-- tests/simple_server.py | 18 ++- tests/slow_retrieval_server.py | 14 +- tests/test_download.py | 24 ++-- .../test_multiple_repositories_integration.py | 23 ++-- tests/test_proxy_use.py | 11 +- tests/test_slow_retrieval_attack.py | 2 +- tests/test_updater.py | 43 +++--- tests/utils.py | 122 ++++++++++++++---- 11 files changed, 194 insertions(+), 135 deletions(-) diff --git a/tests/proxy_server.py b/tests/proxy_server.py index 910fc4868b..2faf160a90 100644 --- a/tests/proxy_server.py +++ b/tests/proxy_server.py @@ -462,25 +462,21 @@ def test(HandlerClass=ProxyRequestHandler, ServerClass=ThreadingHTTPServer, prot global INTERCEPT global TARGET_SERVER_CA_FILEPATH - if sys.argv[1:]: - port = int(sys.argv[1]) - else: - port = 8080 - server_address = ('localhost', port) + server_address = ('localhost', 0) # MODIFIED: Argument added, conditional below added to control INTERCEPT # setting. - if len(sys.argv) > 2: - if sys.argv[2].lower() == 'intercept': + if len(sys.argv) > 1: + if sys.argv[1].lower() == 'intercept': INTERCEPT = True # MODIFIED: Argument added to control certificate(s) the proxy expects of # the target server(s), and added default value. - if len(sys.argv) > 3: - if os.path.exists(sys.argv[3]): - TARGET_SERVER_CA_FILEPATH = sys.argv[3] + if len(sys.argv) > 2: + if os.path.exists(sys.argv[2]): + TARGET_SERVER_CA_FILEPATH = sys.argv[2] else: - raise Exception('Target server cert file not found: ' + sys.argv[3]) + raise Exception('Target server cert file not found: ' + sys.argv[2]) # MODIFIED: Create the target-host-specific proxy certificates directory if # it doesn't already exist. @@ -489,11 +485,16 @@ def test(HandlerClass=ProxyRequestHandler, ServerClass=ThreadingHTTPServer, prot HandlerClass.protocol_version = protocol - httpd = ServerClass(server_address, HandlerClass) + try: + httpd = ServerClass(server_address, HandlerClass) + sa = httpd.socket.getsockname() + port_message = 'bind succeeded, server port is: ' + str(sa[1]) + print(port_message) + print("Serving HTTP Proxy on", sa[0], "port", sa[1], "...") + httpd.serve_forever() + except: + print("bind failed") - sa = httpd.socket.getsockname() - print "Serving HTTP Proxy on", sa[0], "port", sa[1], "..." - httpd.serve_forever() if __name__ == '__main__': diff --git a/tests/repository_data/map.json b/tests/repository_data/map.json index d683880441..c2e0fb8b0f 100644 --- a/tests/repository_data/map.json +++ b/tests/repository_data/map.json @@ -23,11 +23,7 @@ } ], "repositories": { - "test_repository1": [ - "http://localhost:30001" - ], - "test_repository2": [ - "http://localhost:30002" - ] + "test_repository1": [], + "test_repository2": [] } } diff --git a/tests/simple_https_server.py b/tests/simple_https_server.py index 8e4c1ddab1..791b0695eb 100755 --- a/tests/simple_https_server.py +++ b/tests/simple_https_server.py @@ -43,30 +43,27 @@ import os import six -PORT = 0 - keyfile = os.path.join('ssl_certs', 'ssl_cert.key') certfile = os.path.join('ssl_certs', 'ssl_cert.crt') -if len(sys.argv) > 1: - PORT = int(sys.argv[1]) - -else: - PORT = random.randint(30000, 45000) -if len(sys.argv) > 2: - - if os.path.exists(sys.argv[2]): - certfile = sys.argv[2] +if len(sys.argv) > 1: + if os.path.exists(sys.argv[1]): + certfile = sys.argv[1] else: - print('simple_https_server: cert file not found: ' + sys.argv[2] + + print('simple_https_server: cert file not found: ' + sys.argv[1] + '; using default: ' + certfile) -httpd = six.moves.BaseHTTPServer.HTTPServer(('localhost', PORT), - six.moves.SimpleHTTPServer.SimpleHTTPRequestHandler) +try: + httpd = six.moves.BaseHTTPServer.HTTPServer(('localhost', 0), + six.moves.SimpleHTTPServer.SimpleHTTPRequestHandler) -httpd.socket = ssl.wrap_socket( - httpd.socket, keyfile=keyfile, certfile=certfile, server_side=True) + httpd.socket = ssl.wrap_socket( + httpd.socket, keyfile=keyfile, certfile=certfile, server_side=True) -#print('Starting https server on port: ' + str(PORT)) -httpd.serve_forever() + port_message = 'bind succeeded, server port is: ' \ + + str(httpd.server_address[1]) + print(port_message) + httpd.serve_forever() +except: + print("bind failed") diff --git a/tests/simple_server.py b/tests/simple_server.py index 8c19acc5a7..35a1392a82 100755 --- a/tests/simple_server.py +++ b/tests/simple_server.py @@ -39,14 +39,6 @@ import six from six.moves.SimpleHTTPServer import SimpleHTTPRequestHandler -PORT = 0 - -if len(sys.argv) > 1: - PORT = int(sys.argv[1]) - -else: - PORT = random.randint(30000, 45000) - class QuietHTTPRequestHandler(SimpleHTTPRequestHandler): """A SimpleHTTPRequestHandler that does not write incoming requests to @@ -73,6 +65,12 @@ def log_request(self, code='-', size='-'): # Allow re-use so you can re-run tests as often as you want even if the # tests re-use ports. Otherwise TCP TIME-WAIT prevents reuse for ~1 minute six.moves.socketserver.TCPServer.allow_reuse_address = True -httpd = six.moves.socketserver.TCPServer(('', PORT), handler) -httpd.serve_forever() +try: + httpd = six.moves.socketserver.TCPServer(('localhost', 0), handler) + port_message = 'bind succeeded, server port is: ' \ + + str(httpd.server_address[1]) + print(port_message) + httpd.serve_forever() +except: + print("bind failed") diff --git a/tests/slow_retrieval_server.py b/tests/slow_retrieval_server.py index e2c525d2da..afe478a49f 100755 --- a/tests/slow_retrieval_server.py +++ b/tests/slow_retrieval_server.py @@ -65,7 +65,13 @@ def do_GET(self): if __name__ == '__main__': - port = int(sys.argv[1]) - server_address = ('localhost', port) - httpd = six.moves.BaseHTTPServer.HTTPServer(server_address, Handler) - httpd.handle_request() + server_address = ('localhost', 0) + + try: + httpd = six.moves.BaseHTTPServer.HTTPServer(server_address, Handler) + port_message = 'bind succeeded, server port is: ' \ + + str(httpd.server_address[1]) + print(port_message) + httpd.serve_forever() + except: + print("bind failed") diff --git a/tests/test_download.py b/tests/test_download.py index f768c96df6..1107442c83 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -258,29 +258,29 @@ def test_https_connection(self): # 4: run with an HTTPS certificate that is expired # Be sure to offset from the port used in setUp to avoid collision. - port1 = self.server_process_handler.port + 1 - port2 = self.server_process_handler.port + 2 - port3 = self.server_process_handler.port + 3 - port4 = self.server_process_handler.port + 4 good_https_server_handler = utils.TestServerProcess(log=logger, - server='simple_https_server.py', port=port1, + server='simple_https_server.py', extra_cmd_args=[good_cert_fname]) good2_https_server_handler = utils.TestServerProcess(log=logger, - server='simple_https_server.py', port=port2, + server='simple_https_server.py', extra_cmd_args=[good2_cert_fname]) bad_https_server_handler = utils.TestServerProcess(log=logger, - server='simple_https_server.py', port=port3, + server='simple_https_server.py', extra_cmd_args=[bad_cert_fname]) expd_https_server_handler = utils.TestServerProcess(log=logger, - server='simple_https_server.py', port=port4, + server='simple_https_server.py', extra_cmd_args=[expired_cert_fname]) suffix = '/' + os.path.basename(target_filepath) - good_https_url = 'https://localhost:' + str(port1) + suffix - good2_https_url = 'https://localhost:' + str(port2) + suffix - bad_https_url = 'https://localhost:' + str(port3) + suffix - expired_https_url = 'https://localhost:' + str(port4) + suffix + good_https_url = 'https://localhost:' \ + + str(good_https_server_handler.port) + suffix + good2_https_url = 'https://localhost:' \ + + str(good2_https_server_handler.port) + suffix + bad_https_url = 'https://localhost:' \ + + str(bad_https_server_handler.port) + suffix + expired_https_url = 'https://localhost:' \ + + str(expd_https_server_handler.port) + suffix # Download the target file using an HTTPS connection. diff --git a/tests/test_multiple_repositories_integration.py b/tests/test_multiple_repositories_integration.py index ea6b12f12f..59921a71ac 100755 --- a/tests/test_multiple_repositories_integration.py +++ b/tests/test_multiple_repositories_integration.py @@ -119,13 +119,6 @@ def setUp(self): # the pre-generated metadata files have a specific structure, such # as a delegated role 'targets/role1', three target files, five key files, # etc. - self.SERVER_PORT = random.SystemRandom().randint(30000, 45000) - self.SERVER_PORT2 = random.SystemRandom().randint(30000, 45000) - - # Avoid duplicate port numbers, to prevent multiple localhosts from - # listening on the same port. - while self.SERVER_PORT == self.SERVER_PORT2: - self.SERVER_PORT2 = random.SystemRandom().randint(30000, 45000) # Needed because in some tests simple_server.py cannot be found. # The reason is that the current working directory @@ -134,20 +127,18 @@ def setUp(self): # Creates a subprocess running server and uses temp file for logging. self.server_process_handler = utils.TestServerProcess(log=logger, - port=self.SERVER_PORT, server=SIMPLE_SERVER_PATH, - popen_cwd=self.repository_directory) + server=SIMPLE_SERVER_PATH, popen_cwd=self.repository_directory) logger.debug('Server process started.') # Creates a subprocess running server and uses temp file for logging. self.server_process_handler2 = utils.TestServerProcess(log=logger, - port=self.SERVER_PORT2, server=SIMPLE_SERVER_PATH, - popen_cwd=self.repository_directory2) + server=SIMPLE_SERVER_PATH, popen_cwd=self.repository_directory2) logger.debug('Server process 2 started.') - url_prefix = 'http://localhost:' + str(self.SERVER_PORT) - url_prefix2 = 'http://localhost:' + str(self.SERVER_PORT2) + url_prefix = 'http://localhost:' + str(self.server_process_handler.port) + url_prefix2 = 'http://localhost:' + str(self.server_process_handler2.port) self.repository_mirrors = {'mirror1': {'url_prefix': url_prefix, 'metadata_path': 'metadata', @@ -265,8 +256,10 @@ def test_repository_tool(self): # Test the behavior of the multi-repository updater. map_file = securesystemslib.util.load_json_file(self.map_file) - map_file['repositories'][self.repository_name] = ['http://localhost:' + str(self.SERVER_PORT)] - map_file['repositories'][self.repository_name2] = ['http://localhost:' + str(self.SERVER_PORT2)] + map_file['repositories'][self.repository_name] = ['http://localhost:' \ + + str(self.server_process_handler.port)] + map_file['repositories'][self.repository_name2] = ['http://localhost:' \ + + str(self.server_process_handler2.port)] with open(self.map_file, 'w') as file_object: file_object.write(json.dumps(map_file)) diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index 5c98361f9a..a92b61dbbb 100755 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -80,15 +80,13 @@ def setUpClass(cls): # Launch an HTTPS server (serves files in the current dir). cls.https_server_handler = utils.TestServerProcess(log=logger, - server='simple_https_server.py', - port=cls.http_server_handler.port + 1) + server='simple_https_server.py') # Launch an HTTP proxy server derived from inaz2/proxy2. # This one is able to handle HTTP CONNECT requests, and so can pass HTTPS # requests on to the target server. cls.http_proxy_handler = utils.TestServerProcess(log=logger, - server='proxy_server.py', - port=cls.http_server_handler.port + 2) + server='proxy_server.py') # Note that the HTTP proxy server's address uses http://, regardless of the # type of connection used with the target server. @@ -109,9 +107,8 @@ def setUpClass(cls): # This is only relevant if the proxy is in intercept mode. good_cert_fpath = os.path.join('ssl_certs', 'ssl_cert.crt') cls.https_proxy_handler = utils.TestServerProcess(log=logger, - server='proxy_server.py', - port=cls.http_server_handler.port + 3, - extra_cmd_args=['intercept', good_cert_fpath]) + server='proxy_server.py', extra_cmd_args=['intercept', + good_cert_fpath]) # Note that the HTTPS proxy server's address uses https://, regardless of # the type of connection used with the target server. diff --git a/tests/test_slow_retrieval_attack.py b/tests/test_slow_retrieval_attack.py index adb01f157e..899ba8a914 100755 --- a/tests/test_slow_retrieval_attack.py +++ b/tests/test_slow_retrieval_attack.py @@ -169,7 +169,7 @@ def setUp(self): repository_basepath = self.repository_directory[len(os.getcwd()):] self.server_process_handler = utils.TestServerProcess(log=logger, - server='slow_retrieval_server.py', timeout=0) + server='slow_retrieval_server.py') logger.info('Slow Retrieval Server process started.') diff --git a/tests/test_updater.py b/tests/test_updater.py index a9cf90b384..78e9e178d6 100755 --- a/tests/test_updater.py +++ b/tests/test_updater.py @@ -58,6 +58,7 @@ import errno import sys import unittest +import json import tuf import tuf.exceptions @@ -1863,27 +1864,35 @@ def setUp(self): # as a delegated role 'targets/role1', three target files, five key files, # etc. - # The ports are harcoded because the urls to the repositories are harcoded - # in map.json. - self.SERVER_PORT = 30001 - self.SERVER_PORT2 = 30002 - # Creates a subprocess running server and uses temp file for logging. self.server_process_handler = utils.TestServerProcess(log=logger, - server=self.SIMPLE_SERVER_PATH, port=self.SERVER_PORT, - popen_cwd=self.repository_directory) + server=self.SIMPLE_SERVER_PATH, popen_cwd=self.repository_directory) logger.debug('Server process started.') # Creates a subprocess running server and uses temp file for logging. self.server_process_handler2 = utils.TestServerProcess(log=logger, - server=self.SIMPLE_SERVER_PATH, port=self.SERVER_PORT2, - popen_cwd=self.repository_directory2) + server=self.SIMPLE_SERVER_PATH, popen_cwd=self.repository_directory2) logger.debug('Server process 2 started.') - url_prefix = 'http://localhost:' + str(self.SERVER_PORT) - url_prefix2 = 'http://localhost:' + str(self.SERVER_PORT2) + url_prefix = 'http://localhost:' + str(self.server_process_handler.port) + url_prefix2 = 'http://localhost:' + str(self.server_process_handler2.port) + + # We have all of the necessary information for two repository mirrors + # in map.json, except for url prefixes. + # For the url prefixes, we create subprocesses that run a server script. + # In server scripts we get a free port from the OS which is sent + # back to the father process. + # That's why we dynamically add the ports to the url prefixes + # and changing the content of map.json. + self.map_file_path = os.path.join(self.client_directory, 'map.json') + data = securesystemslib.util.load_json_file(self.map_file_path) + + data['repositories']['test_repository1'] = [url_prefix] + data['repositories']['test_repository2'] = [url_prefix2] + with open(self.map_file_path, 'w') as f: + json.dump(data, f) self.repository_mirrors = {'mirror1': {'url_prefix': url_prefix, 'metadata_path': 'metadata', 'targets_path': 'targets'}} @@ -1957,14 +1966,12 @@ def test__init__(self): updater.MultiRepoUpdater, root_filepath) # Test for a valid instantiation. - map_file = os.path.join(self.client_directory, 'map.json') - multi_repo_updater = updater.MultiRepoUpdater(map_file) + multi_repo_updater = updater.MultiRepoUpdater(self.map_file_path) def test__target_matches_path_pattern(self): - map_file = os.path.join(self.client_directory, 'map.json') - multi_repo_updater = updater.MultiRepoUpdater(map_file) + multi_repo_updater = updater.MultiRepoUpdater(self.map_file_path) paths = ['foo*.tgz', 'bar*.tgz', 'file1.txt'] self.assertTrue( multi_repo_updater._target_matches_path_pattern('bar-1.0.tgz', paths)) @@ -1976,8 +1983,7 @@ def test__target_matches_path_pattern(self): def test_get_valid_targetinfo(self): - map_file = os.path.join(self.client_directory, 'map.json') - multi_repo_updater = updater.MultiRepoUpdater(map_file) + multi_repo_updater = updater.MultiRepoUpdater(self.map_file_path) # Verify the multi repo updater refuses to save targetinfo if # required local repositories are missing. @@ -2084,8 +2090,7 @@ def test_get_valid_targetinfo(self): def test_get_updater(self): - map_file = os.path.join(self.client_directory, 'map.json') - multi_repo_updater = updater.MultiRepoUpdater(map_file) + multi_repo_updater = updater.MultiRepoUpdater(self.map_file_path) # Test for a non-existent repository name. self.assertEqual(None, multi_repo_updater.get_updater('bad_repo_name')) diff --git a/tests/utils.py b/tests/utils.py index c3cd5a0a1c..43dd83fa1d 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -28,7 +28,6 @@ import time import subprocess import tempfile -import random import warnings import tuf.log @@ -90,7 +89,7 @@ def wait_for_server(host, server, port, timeout=10): if not succeeded: raise TimeoutError("Could not connect to the " + server \ - + " on port " + str(port) + " !") + + " on port " + str(port) + "!") def configure_test_logging(argv): @@ -129,15 +128,9 @@ class TestServerProcess(): Path to the server to run in the subprocess. Default is "simpler_server.py". - port: - The port used to access the server. If none is provided, - then one will be generated. - Default is None. - timeout: Time in seconds in which the server should start or otherwise TimeoutError error will be raised. - If 0 is given, no check if the server has started will be done. Default is 10. popen_cwd: @@ -154,43 +147,114 @@ class TestServerProcess(): def __init__(self, log, server='simple_server.py', - port=None, timeout=10, popen_cwd=".", - extra_cmd_args=[]): + timeout=10, popen_cwd=".", extra_cmd_args=[]): # Create temporary log file used for logging stdout and stderr - # of the subprocess. In the mode "r+"" stands for reading and writing + # of the subprocess. In the mode "r+" stands for reading and writing # and "t" stands for text mode. self.__temp_log_file = tempfile.TemporaryFile(mode='r+t') self.server = server - self.port = port or random.randint(30000, 45000) self.__logger = log + try: + self._start_server(timeout, extra_cmd_args, popen_cwd) + wait_for_server('localhost', self.server, self.port, timeout) + except Exception as e: + # Clean the resources and log the server errors if any exists. + self.clean() + raise e + + + + def _start_server(self, timeout, extra_cmd_args, popen_cwd): + """Start the server subprocess. Uses a retry mechanism + if the bind fails.""" + + success = False + retries = 0 + elapsed = 0 + start = time.time() + while not success and elapsed < timeout: + retries += 1 + self._start_process(extra_cmd_args, popen_cwd) + + # loop until bind succeeds, server exits or we timeout + while elapsed < timeout: + if not self.is_process_running(): + break + elif self._is_port_found_in_log(): + # If the port is in the log, then the bind was successful. + success = True + break + + time.sleep(0.01) + elapsed = time.time() - start + + if not success: + # If the server has not started for whatever reason + self.__logger.info('Failed to start ' + self.server + '! Retrying!') + self._kill_server_process() + self.__temp_log_file.truncate(0) + + if not success: + raise TimeoutError('Failure during ' + self.server + ' startup! ' \ + + 'Made ' + str(retries) + ' retries with random ports!') + + self.__logger.info(self.server + ' serving at ' + str(self.port)) + + + + def _start_process(self, extra_cmd_args, popen_cwd): + """Starts the process running the server.""" # The "-u" option forces stdin, stdout and stderr to be unbuffered. - command = ['python', '-u', server, str(self.port)] + extra_cmd_args + command = ['python', '-u', self.server] + extra_cmd_args # We are reusing one server subprocess in multiple unit tests, but we are # collecting the logs per test. self.__server_process = subprocess.Popen(command, stdout=self.__temp_log_file, stderr=subprocess.STDOUT, cwd=popen_cwd) - self.__logger.info('Server process with process id ' \ - + str(self.__server_process.pid) + " serving on port " \ - + str(self.port) + ' started.') - if timeout > 0: - try: - wait_for_server('localhost', self.server, self.port, timeout) - except Exception as e: - # Make sure that errors from the server side will be logged. - self.flush_log() - raise e + + def _is_port_found_in_log(self): + """Checks if the port number is sent from the server subprocess.""" + + # Seek is needed to move the pointer to the beginning of the file, because + # the subprocess could have read and/or write and thus moved the pointer. + self.__temp_log_file.seek(0) + # We have hardcoded the message we expect on a successful server startup. + expected_msg = 'bind succeeded, server port is: ' + log_message = self.__temp_log_file.read() + lines = log_message.splitlines() + + for line in lines: + if line.startswith(expected_msg): + self.port = int(line[len(expected_msg):]) + return True + + return False + + + + def _kill_server_process(self): + """Kills the server subprocess if it's running.""" + + if self.is_process_running(): + self.__logger.info('Server process ' + str(self.__server_process.pid) + + ' terminated.') + self.__server_process.kill() + self.__server_process.wait() def flush_log(self): """Logs contents from TempFile, truncates buffer""" + # Make sure we are only reading from opened files. + if self.__temp_log_file.closed: + return + # Seek is needed to move the pointer to the beginning of the file, because # the subprocess could have read and/or write and thus moved the pointer. self.__temp_log_file.seek(0) @@ -214,9 +278,11 @@ def clean(self): self.flush_log() self.__temp_log_file.close() + self._kill_server_process() - if self.__server_process.returncode is None: - self.__logger.info('Server process ' + str(self.__server_process.pid) + - ' terminated.') - self.__server_process.kill() - self.__server_process.wait() + + + def is_process_running(self): + """Returns a boolean value if the server process is currently running.""" + + return True if self.__server_process.poll() is None else False From 2c5617b7c0aaf611bb1f821edf29d14c6ed77522 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Tue, 20 Oct 2020 15:08:10 +0300 Subject: [PATCH 03/12] Remove unused random module imports Signed-off-by: Martin Vrachev --- tests/simple_https_server.py | 1 - tests/slow_retrieval_server.py | 1 - tests/test_mirrors.py | 1 - tests/test_multiple_repositories_integration.py | 1 - tests/test_slow_retrieval_attack.py | 2 -- 5 files changed, 6 deletions(-) diff --git a/tests/simple_https_server.py b/tests/simple_https_server.py index 791b0695eb..2be8508d80 100755 --- a/tests/simple_https_server.py +++ b/tests/simple_https_server.py @@ -38,7 +38,6 @@ from __future__ import unicode_literals import sys -import random import ssl import os import six diff --git a/tests/slow_retrieval_server.py b/tests/slow_retrieval_server.py index afe478a49f..1a6d56a55a 100755 --- a/tests/slow_retrieval_server.py +++ b/tests/slow_retrieval_server.py @@ -32,7 +32,6 @@ import os import sys import time -import random import six diff --git a/tests/test_mirrors.py b/tests/test_mirrors.py index 9318ef493d..170274bfac 100755 --- a/tests/test_mirrors.py +++ b/tests/test_mirrors.py @@ -38,7 +38,6 @@ import securesystemslib import securesystemslib.util -import six class TestMirrors(unittest_toolbox.Modified_TestCase): diff --git a/tests/test_multiple_repositories_integration.py b/tests/test_multiple_repositories_integration.py index 59921a71ac..0d5bd3f64a 100755 --- a/tests/test_multiple_repositories_integration.py +++ b/tests/test_multiple_repositories_integration.py @@ -31,7 +31,6 @@ import os import tempfile -import random import logging import shutil import unittest diff --git a/tests/test_slow_retrieval_attack.py b/tests/test_slow_retrieval_attack.py index 899ba8a914..0ce81e2567 100755 --- a/tests/test_slow_retrieval_attack.py +++ b/tests/test_slow_retrieval_attack.py @@ -46,7 +46,6 @@ import os import tempfile -import random import time import shutil import logging @@ -222,7 +221,6 @@ def test_delay_before_send(self): # Verify that the TUF client detects replayed metadata and refuses to # continue the update process. - client_filepath = os.path.join(self.client_directory, 'file1.txt') try: file1_target = self.repository_updater.get_one_valid_targetinfo('file1.txt') self.repository_updater.download_target(file1_target, self.client_directory) From 1e6970d6a1d9a49ee13d325c7728e0d43581cfc4 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Thu, 22 Oct 2020 16:41:02 +0300 Subject: [PATCH 04/12] Remove sleep from test_slow_retrieval_attack.py Now, after we can use wait_for_server and the retry mechanism of TestServerProcess in utils.py we no longer need to use sleep in this test file. Signed-off-by: Martin Vrachev --- tests/test_slow_retrieval_attack.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/test_slow_retrieval_attack.py b/tests/test_slow_retrieval_attack.py index 0ce81e2567..2fe54bfc26 100755 --- a/tests/test_slow_retrieval_attack.py +++ b/tests/test_slow_retrieval_attack.py @@ -172,14 +172,6 @@ def setUp(self): logger.info('Slow Retrieval Server process started.') - # NOTE: Following error is raised if a delay is not long enough: - # - # or, on Windows: - # Failed to establish a new connection: [Errno 111] Connection refused' - # 1s led to occasional failures in automated builds on AppVeyor, so - # increasing this to 3s, sadly. - time.sleep(3) - url_prefix = 'http://localhost:' \ + str(self.server_process_handler.port) + repository_basepath From 5b66cb4f8ca2d2dec257f1fe04fadbfd53796c1e Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Wed, 28 Oct 2020 15:11:49 +0200 Subject: [PATCH 05/12] Add tests for tests/utils.py We want to make sure that server are successfully started in the common use cases and that the new port generation works. Signed-off-by: Martin Vrachev --- tests/test_utils.py | 118 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 tests/test_utils.py diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 0000000000..fe210101aa --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,118 @@ +#!/usr/bin/env python + +# Copyright 2020, TUF contributors +# SPDX-License-Identifier: MIT OR Apache-2.0 + +""" + + test_utils.py + + + Martin Vrachev. + + + October 21, 2020. + + + See LICENSE-MIT OR LICENSE for licensing information. + + + Provide tests for some of the functions in utils.py module. +""" + +import os +import logging +import unittest +import socket +import sys + +import tuf.unittest_toolbox as unittest_toolbox + +import utils + +logger = logging.getLogger(__name__) + +class TestServerProcess(unittest_toolbox.Modified_TestCase): + + def tearDown(self): + # Make sure we are calling clean on existing attribute. + if hasattr(self, 'server_process_handler'): + self.server_process_handler.clean() + + + def can_connect(self): + succeed = False + try: + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.connect(('localhost', self.server_process_handler.port)) + succeed = True + except: + pass + finally: + if sock: + sock.close() + return succeed + + + def test_simple_server_startup(self): + # Test normal case + self.server_process_handler = utils.TestServerProcess(log=logger) + + # Make sure we can connect to the server + self.assertTrue(self.can_connect()) + + + def test_simple_https_server_startup(self): + # Test normal case + good_cert_path = os.path.join('ssl_certs', 'ssl_cert.crt') + self.server_process_handler = utils.TestServerProcess(log=logger, + server='simple_https_server.py', extra_cmd_args=[good_cert_path]) + + # Make sure we can connect to the server + self.assertTrue(self.can_connect()) + + + @unittest.skipIf(sys.version_info.major != 2, "Test for Python 2.X") + def test_proxy_server_startup(self): + # Test normal case + self.server_process_handler = utils.TestServerProcess(log=logger, + server='proxy_server.py') + + # Make sure we can connect to the server. + self.assertTrue(self.can_connect()) + + self.server_process_handler.clean() + + # Test start proxy_server using certificate files. + good_cert_fpath = os.path.join('ssl_certs', 'ssl_cert.crt') + self.server_process_handler = utils.TestServerProcess(log=logger, + server='proxy_server.py', extra_cmd_args=['intercept', + good_cert_fpath]) + + # Make sure we can connect to the server. + self.assertTrue(self.can_connect()) + + + def test_slow_retrieval_server_startup(self): + # Test normal case + self.server_process_handler = utils.TestServerProcess(log=logger, + server='slow_retrieval_server.py') + + # Make sure we can connect to the server + self.assertTrue(self.can_connect()) + + + def test_cleanup(self): + # Test normal case + self.server_process_handler = utils.TestServerProcess(log=logger, + server='simple_server.py') + + self.server_process_handler.clean() + + # Check if the process has successfully been killed. + self.assertFalse(self.server_process_handler.is_process_running()) + + +if __name__ == '__main__': + utils.configure_test_logging(sys.argv) + unittest.main() From 8d9eef03505b4f16e50af39ed3018377532996ff Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Thu, 29 Oct 2020 14:43:27 +0200 Subject: [PATCH 06/12] Update on "Delegate port generation ..." commit Signed-off-by: Martin Vrachev --- tests/utils.py | 65 +++++++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index 43dd83fa1d..09e823bffc 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -167,40 +167,36 @@ def __init__(self, log, server='simple_server.py', def _start_server(self, timeout, extra_cmd_args, popen_cwd): - """Start the server subprocess. Uses a retry mechanism - if the bind fails.""" + """ + Start the server subprocess. + If the process hasn't started a Timeout exception will be raised. + """ success = False - retries = 0 elapsed = 0 start = time.time() - while not success and elapsed < timeout: - retries += 1 - self._start_process(extra_cmd_args, popen_cwd) - - # loop until bind succeeds, server exits or we timeout - while elapsed < timeout: - if not self.is_process_running(): - break - elif self._is_port_found_in_log(): - # If the port is in the log, then the bind was successful. - success = True - break - - time.sleep(0.01) - elapsed = time.time() - start - - if not success: - # If the server has not started for whatever reason - self.__logger.info('Failed to start ' + self.server + '! Retrying!') - self._kill_server_process() - self.__temp_log_file.truncate(0) + + self._start_process(extra_cmd_args, popen_cwd) + + # loop until bind succeeds, server exits or we timeout + while elapsed < timeout: + if not self.is_process_running(): + raise ChildProcessError('Child process running ' + self.server \ + + ' exited before the timeout has expired with code ' \ + + str(self.__server_process.poll()) + '!') + + elif self._set_port_if_in_logs(): + # If the port is in the log, then the bind was successful. + success = True + break + + time.sleep(0.01) + elapsed = time.time() - start if not success: - raise TimeoutError('Failure during ' + self.server + ' startup! ' \ - + 'Made ' + str(retries) + ' retries with random ports!') + raise TimeoutError('Failure during ' + self.server + ' startup!') - self.__logger.info(self.server + ' serving at ' + str(self.port)) + self.__logger.info(self.server + ' serving on ' + str(self.port)) @@ -217,8 +213,11 @@ def _start_process(self, extra_cmd_args, popen_cwd): - def _is_port_found_in_log(self): - """Checks if the port number is sent from the server subprocess.""" + def _set_port_if_in_logs(self): + """ + Checks if the port is logged from the server subprocess. + If it's found, self.port is set. + """ # Seek is needed to move the pointer to the beginning of the file, because # the subprocess could have read and/or write and thus moved the pointer. @@ -271,8 +270,10 @@ def flush_log(self): def clean(self): - """Kills the subprocess and closes the TempFile. - Calls flush_log to check for logged information, but not yet flushed.""" + """ + Kills the subprocess and closes the TempFile. + Calls flush_log to check for logged information, but not yet flushed. + """ # If there is anything logged, flush it before closing the resourses. self.flush_log() @@ -283,6 +284,4 @@ def clean(self): def is_process_running(self): - """Returns a boolean value if the server process is currently running.""" - return True if self.__server_process.poll() is None else False From eab043e13269e9ffdd6c374613c0fa9eaf61b24c Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Thu, 29 Oct 2020 14:48:25 +0200 Subject: [PATCH 07/12] Update "Remove slow_retrieval failure test" commit Signed-off-by: Martin Vrachev --- tests/test_slow_retrieval_attack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_slow_retrieval_attack.py b/tests/test_slow_retrieval_attack.py index 2fe54bfc26..4b9943bfb7 100755 --- a/tests/test_slow_retrieval_attack.py +++ b/tests/test_slow_retrieval_attack.py @@ -208,7 +208,7 @@ def tearDown(self): def test_delay_before_send(self): # Simulate a slow retrieval attack. - # 'mode_1': When download begins,the server blocks the download for a long + # When download begins,the server blocks the download for a long # time by doing nothing before it sends the first byte of data. # Verify that the TUF client detects replayed metadata and refuses to From b722eac3c154d5ebd57448299507723f8f8a7eea Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Thu, 29 Oct 2020 14:53:58 +0200 Subject: [PATCH 08/12] Update "Add tests for tests/utils.py" commit Signed-off-by: Martin Vrachev --- tests/test_utils.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index fe210101aa..bcaec5a9ab 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -41,17 +41,16 @@ def tearDown(self): def can_connect(self): - succeed = False try: sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) sock.connect(('localhost', self.server_process_handler.port)) - succeed = True + return True except: - pass + return False finally: + # The process will always enter in finally even we return. if sock: sock.close() - return succeed def test_simple_server_startup(self): From 097c78b4ecd90bab4b55b116673c9a67acb45475 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Thu, 29 Oct 2020 16:41:06 +0200 Subject: [PATCH 09/12] Add tests for server exit before timeout expires Signed-off-by: Martin Vrachev --- tests/fast_server_exit.py | 25 +++++++++++++++++++++++++ tests/test_utils.py | 10 ++++++++++ 2 files changed, 35 insertions(+) create mode 100644 tests/fast_server_exit.py diff --git a/tests/fast_server_exit.py b/tests/fast_server_exit.py new file mode 100644 index 0000000000..b54b7b9230 --- /dev/null +++ b/tests/fast_server_exit.py @@ -0,0 +1,25 @@ +#!/usr/bin/env python + +# Copyright 2020, TUF contributors +# SPDX-License-Identifier: MIT OR Apache-2.0 + +""" + + fast_server_exit.py + + + Martin Vrachev. + + + October 29, 2020. + + + See LICENSE-MIT OR LICENSE for licensing information. + + + Used for tests in tests/test_utils.py. +""" + +import sys + +sys.exit(0) diff --git a/tests/test_utils.py b/tests/test_utils.py index bcaec5a9ab..76becf8bd3 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -112,6 +112,16 @@ def test_cleanup(self): self.assertFalse(self.server_process_handler.is_process_running()) + def test_server_exit_before_timeout(self): + # Test starting a non existing server file." + self.assertRaises(ChildProcessError, utils.TestServerProcess, logger, + server='non_existing_server.py') + + # Test starting a server which immediately exits." + self.assertRaises(ChildProcessError, utils.TestServerProcess, logger, + server='fast_server_exit.py') + + if __name__ == '__main__': utils.configure_test_logging(sys.argv) unittest.main() From e6e70a781e08fcc2d1a6de13601c6df7749e05ee Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Thu, 29 Oct 2020 16:59:01 +0200 Subject: [PATCH 10/12] Shorten CildProcessError message Signed-off-by: Martin Vrachev --- tests/utils.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index 09e823bffc..8d9aba95a2 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -181,9 +181,8 @@ def _start_server(self, timeout, extra_cmd_args, popen_cwd): # loop until bind succeeds, server exits or we timeout while elapsed < timeout: if not self.is_process_running(): - raise ChildProcessError('Child process running ' + self.server \ - + ' exited before the timeout has expired with code ' \ - + str(self.__server_process.poll()) + '!') + raise ChildProcessError(self.server + ' exited unexpectedly ' \ + + 'with code ' + str(self.__server_process.poll()) + '!') elif self._set_port_if_in_logs(): # If the port is in the log, then the bind was successful. From 9e53c4a3beb2d17651fcd3a530874f022f43dbbc Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Thu, 29 Oct 2020 17:23:51 +0200 Subject: [PATCH 11/12] Remove try/except at server test files As discussed with Jussi, using try and except blocks when instantiating the servers don't bring a lot of value (the "bind failed") message could be useful, but there are other messages explaining almost the same thing which will be logged from the father process as well. Signed-off-by: Martin Vrachev --- tests/proxy_server.py | 15 ++++++--------- tests/simple_https_server.py | 23 ++++++++++------------- tests/simple_server.py | 13 +++++-------- tests/slow_retrieval_server.py | 13 +++++-------- 4 files changed, 26 insertions(+), 38 deletions(-) diff --git a/tests/proxy_server.py b/tests/proxy_server.py index 2faf160a90..7dec621f57 100644 --- a/tests/proxy_server.py +++ b/tests/proxy_server.py @@ -485,15 +485,12 @@ def test(HandlerClass=ProxyRequestHandler, ServerClass=ThreadingHTTPServer, prot HandlerClass.protocol_version = protocol - try: - httpd = ServerClass(server_address, HandlerClass) - sa = httpd.socket.getsockname() - port_message = 'bind succeeded, server port is: ' + str(sa[1]) - print(port_message) - print("Serving HTTP Proxy on", sa[0], "port", sa[1], "...") - httpd.serve_forever() - except: - print("bind failed") + httpd = ServerClass(server_address, HandlerClass) + sa = httpd.socket.getsockname() + port_message = 'bind succeeded, server port is: ' + str(sa[1]) + print(port_message) + print("Serving HTTP Proxy on", sa[0], "port", sa[1], "...") + httpd.serve_forever() diff --git a/tests/simple_https_server.py b/tests/simple_https_server.py index 2be8508d80..ccf6c4518d 100755 --- a/tests/simple_https_server.py +++ b/tests/simple_https_server.py @@ -53,16 +53,13 @@ print('simple_https_server: cert file not found: ' + sys.argv[1] + '; using default: ' + certfile) -try: - httpd = six.moves.BaseHTTPServer.HTTPServer(('localhost', 0), - six.moves.SimpleHTTPServer.SimpleHTTPRequestHandler) - - httpd.socket = ssl.wrap_socket( - httpd.socket, keyfile=keyfile, certfile=certfile, server_side=True) - - port_message = 'bind succeeded, server port is: ' \ - + str(httpd.server_address[1]) - print(port_message) - httpd.serve_forever() -except: - print("bind failed") +httpd = six.moves.BaseHTTPServer.HTTPServer(('localhost', 0), + six.moves.SimpleHTTPServer.SimpleHTTPRequestHandler) + +httpd.socket = ssl.wrap_socket( + httpd.socket, keyfile=keyfile, certfile=certfile, server_side=True) + +port_message = 'bind succeeded, server port is: ' \ + + str(httpd.server_address[1]) +print(port_message) +httpd.serve_forever() diff --git a/tests/simple_server.py b/tests/simple_server.py index 35a1392a82..bec8e7b07f 100755 --- a/tests/simple_server.py +++ b/tests/simple_server.py @@ -66,11 +66,8 @@ def log_request(self, code='-', size='-'): # tests re-use ports. Otherwise TCP TIME-WAIT prevents reuse for ~1 minute six.moves.socketserver.TCPServer.allow_reuse_address = True -try: - httpd = six.moves.socketserver.TCPServer(('localhost', 0), handler) - port_message = 'bind succeeded, server port is: ' \ - + str(httpd.server_address[1]) - print(port_message) - httpd.serve_forever() -except: - print("bind failed") +httpd = six.moves.socketserver.TCPServer(('localhost', 0), handler) +port_message = 'bind succeeded, server port is: ' \ + + str(httpd.server_address[1]) +print(port_message) +httpd.serve_forever() diff --git a/tests/slow_retrieval_server.py b/tests/slow_retrieval_server.py index 1a6d56a55a..7740d73b88 100755 --- a/tests/slow_retrieval_server.py +++ b/tests/slow_retrieval_server.py @@ -66,11 +66,8 @@ def do_GET(self): if __name__ == '__main__': server_address = ('localhost', 0) - try: - httpd = six.moves.BaseHTTPServer.HTTPServer(server_address, Handler) - port_message = 'bind succeeded, server port is: ' \ - + str(httpd.server_address[1]) - print(port_message) - httpd.serve_forever() - except: - print("bind failed") + httpd = six.moves.BaseHTTPServer.HTTPServer(server_address, Handler) + port_message = 'bind succeeded, server port is: ' \ + + str(httpd.server_address[1]) + print(port_message) + httpd.serve_forever() From 344493339ddb22f010f854e29aaa6b389dcae162 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Fri, 30 Oct 2020 13:55:23 +0200 Subject: [PATCH 12/12] Define ChildProcessError for Python2 Signed-off-by: Martin Vrachev --- tests/test_utils.py | 5 ++--- tests/utils.py | 10 ++++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 76becf8bd3..05947852c3 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -113,12 +113,11 @@ def test_cleanup(self): def test_server_exit_before_timeout(self): - # Test starting a non existing server file." - self.assertRaises(ChildProcessError, utils.TestServerProcess, logger, + self.assertRaises(utils.ChildProcessError, utils.TestServerProcess, logger, server='non_existing_server.py') # Test starting a server which immediately exits." - self.assertRaises(ChildProcessError, utils.TestServerProcess, logger, + self.assertRaises(utils.ChildProcessError, utils.TestServerProcess, logger, server='fast_server_exit.py') diff --git a/tests/utils.py b/tests/utils.py index 8d9aba95a2..793c4565f4 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -37,6 +37,7 @@ try: # is defined in Python 3 TimeoutError + ChildProcessError except NameError: # Define for Python 2 class TimeoutError(Exception): @@ -48,6 +49,15 @@ def __str__(self): return repr(self.value) + class ChildProcessError(Exception): + + def __init__(self, value="ChidProcess"): + self.value = value + + def __str__(self): + return repr(self.value) + + @contextmanager def ignore_deprecation_warnings(module): with warnings.catch_warnings():