From 94d3175a00e28b09bef344165e0f8bec7687c0c3 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 3 Aug 2020 18:59:17 +0300 Subject: [PATCH 01/11] simple-server: Allow address re-use If a test happens to use the same port as a previous test run (either by bad luck or hardcoding like TestMultiRepoUpdater) that happened within a minute, the second run will fail because TCP by default keeps sockets open for a while. Avoid this by explicitly saying re-use is fine in this case. Signed-off-by: Jussi Kukkonen --- tests/simple_server.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/simple_server.py b/tests/simple_server.py index cc8e3db39f..5cfbd71383 100755 --- a/tests/simple_server.py +++ b/tests/simple_server.py @@ -80,6 +80,9 @@ def log_request(self, code='-', size='-'): else: handler = SimpleHTTPRequestHandler +# 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() From dc9c3b25571c59a415821ff65c5bf67c7f6ca8fe Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 3 Aug 2020 19:36:07 +0300 Subject: [PATCH 02/11] download: import tuf.formats before using it This seems to have only worked because someone else usually imports formats. This fixes e.g. python3 test_download.py \ TestDownload.test_download_url_to_tempfileobj Signed-off-by: Jussi Kukkonen --- tuf/download.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tuf/download.py b/tuf/download.py index 30e806a401..7065fe1c03 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -42,7 +42,9 @@ import securesystemslib import securesystemslib.util import six + import tuf.exceptions +import tuf.formats import urllib3.exceptions From 1367b79f387ece3e845fadb2e6c9ca4f6a5b3db1 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 3 Aug 2020 19:54:58 +0300 Subject: [PATCH 03/11] tests: Wait for subprocess kill() to happen This avoids the need to sleep() before removing the temporary directories used, and makes sure we don't get ResourceWarning: subprocess N is still running messages. Use subprocess.communicate() instead of wait() if the process has a pipe (currently the return values are just dropped though). Practical results should be more reliability and a slightly reduced test runtime. Signed-off-by: Jussi Kukkonen --- tests/test_arbitrary_package_attack.py | 1 + tests/test_download.py | 4 ++++ tests/test_endless_data_attack.py | 1 + tests/test_extraneous_dependencies_attack.py | 1 + tests/test_indefinite_freeze_attack.py | 1 + tests/test_key_revocation_integration.py | 1 + tests/test_mix_and_match_attack.py | 2 ++ tests/test_multiple_repositories_integration.py | 6 ++---- tests/test_proxy_use.py | 2 ++ tests/test_replay_attack.py | 1 + tests/test_slow_retrieval_attack.py | 1 + tests/test_updater.py | 14 ++++++++------ 12 files changed, 25 insertions(+), 10 deletions(-) diff --git a/tests/test_arbitrary_package_attack.py b/tests/test_arbitrary_package_attack.py index 97d9e5dcaf..639440ced8 100755 --- a/tests/test_arbitrary_package_attack.py +++ b/tests/test_arbitrary_package_attack.py @@ -106,6 +106,7 @@ def tearDownClass(cls): if cls.server_process.returncode is None: logger.info('Server process ' + str(cls.server_process.pid) + ' terminated.') cls.server_process.kill() + cls.server_process.wait() diff --git a/tests/test_download.py b/tests/test_download.py index adbadee385..46d27410ac 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -102,6 +102,8 @@ def tearDown(self): if self.server_proc.returncode is None: logger.info('\tServer process '+str(self.server_proc.pid)+' terminated.') self.server_proc.kill() + # Drop return values of communicate() + self.server_proc.communicate() self.target_fileobj.close() @@ -372,6 +374,8 @@ def test_https_connection(self): if proc.returncode is None: logger.info('Terminating server process ' + str(proc.pid)) proc.kill() + # drop return values + proc.communicate() diff --git a/tests/test_endless_data_attack.py b/tests/test_endless_data_attack.py index cf06b73c24..d5f9713e96 100755 --- a/tests/test_endless_data_attack.py +++ b/tests/test_endless_data_attack.py @@ -108,6 +108,7 @@ def tearDownClass(cls): if cls.server_process.returncode is None: logger.info('Server process '+str(cls.server_process.pid)+' terminated.') cls.server_process.kill() + cls.server_process.wait() diff --git a/tests/test_extraneous_dependencies_attack.py b/tests/test_extraneous_dependencies_attack.py index 113481a8f0..ae57e22d46 100755 --- a/tests/test_extraneous_dependencies_attack.py +++ b/tests/test_extraneous_dependencies_attack.py @@ -112,6 +112,7 @@ def tearDownClass(cls): if cls.server_process.returncode is None: logger.info('Server process '+str(cls.server_process.pid)+' terminated.') cls.server_process.kill() + cls.server_process.wait() diff --git a/tests/test_indefinite_freeze_attack.py b/tests/test_indefinite_freeze_attack.py index 6adb864638..f1b1fa43ef 100755 --- a/tests/test_indefinite_freeze_attack.py +++ b/tests/test_indefinite_freeze_attack.py @@ -123,6 +123,7 @@ def tearDownClass(cls): if cls.server_process.returncode is None: logger.info('Server process '+str(cls.server_process.pid)+' terminated.') cls.server_process.kill() + cls.server_process.wait() diff --git a/tests/test_key_revocation_integration.py b/tests/test_key_revocation_integration.py index 9233877c57..91a794e47b 100755 --- a/tests/test_key_revocation_integration.py +++ b/tests/test_key_revocation_integration.py @@ -108,6 +108,7 @@ def tearDownClass(cls): if cls.server_process.returncode is None: logger.info('\tServer process '+str(cls.server_process.pid)+' terminated.') cls.server_process.kill() + cls.server_process.wait() diff --git a/tests/test_mix_and_match_attack.py b/tests/test_mix_and_match_attack.py index 8f04595817..3fe578a85a 100755 --- a/tests/test_mix_and_match_attack.py +++ b/tests/test_mix_and_match_attack.py @@ -113,6 +113,8 @@ def tearDownClass(cls): if cls.server_process.returncode is None: logger.info('Server process '+str(cls.server_process.pid)+' terminated.') cls.server_process.kill() + cls.server_process.wait() + diff --git a/tests/test_multiple_repositories_integration.py b/tests/test_multiple_repositories_integration.py index 414e43774b..35b1b094a5 100755 --- a/tests/test_multiple_repositories_integration.py +++ b/tests/test_multiple_repositories_integration.py @@ -191,19 +191,17 @@ def tearDown(self): if self.server_process.returncode is None: logger.info('Server process ' + str(self.server_process.pid) + ' terminated.') self.server_process.kill() + self.server_process.wait() if self.server_process2.returncode is None: logger.info('Server 2 process ' + str(self.server_process2.pid) + ' terminated.') self.server_process2.kill() + self.server_process2.wait() # updater.Updater() populates the roledb with the name "test_repository1" tuf.roledb.clear_roledb(clear_all=True) tuf.keydb.clear_keydb(clear_all=True) - # Remove the temporary repository directory, which should contain all the - # metadata, targets, and key files generated of all the test cases. - # sleep for a bit to allow the kill'd server processes to terminate. - time.sleep(.3) shutil.rmtree(self.temporary_directory) diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index a2e1d9318e..76626ca79e 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -148,6 +148,8 @@ def tearDownClass(cls): if proc.returncode is None: logger.info('\tTerminating process ' + str(proc.pid) + ' in cleanup.') proc.kill() + # Drop return values of communicate() + proc.communicate() diff --git a/tests/test_replay_attack.py b/tests/test_replay_attack.py index 9c4620eaf2..e6fc5d9530 100755 --- a/tests/test_replay_attack.py +++ b/tests/test_replay_attack.py @@ -112,6 +112,7 @@ def tearDownClass(cls): if cls.server_process.returncode is None: logger.info('Server process '+str(cls.server_process.pid)+' terminated.') cls.server_process.kill() + cls.server_process.wait() diff --git a/tests/test_slow_retrieval_attack.py b/tests/test_slow_retrieval_attack.py index 90dd499640..50e9b89e97 100755 --- a/tests/test_slow_retrieval_attack.py +++ b/tests/test_slow_retrieval_attack.py @@ -123,6 +123,7 @@ def _stop_slow_server(self, server_process): if server_process.returncode is None: logger.info('Server process '+str(server_process.pid)+' terminated.') server_process.kill() + server_process.wait() diff --git a/tests/test_updater.py b/tests/test_updater.py index 9a55d3bc2f..0b06682712 100644 --- a/tests/test_updater.py +++ b/tests/test_updater.py @@ -130,11 +130,10 @@ def tearDownClass(cls): if cls.server_process.returncode is None: logger.info('\tServer process ' + str(cls.server_process.pid) + ' terminated.') cls.server_process.kill() + cls.server_process.wait() # Remove the temporary repository directory, which should contain all the - # metadata, targets, and key files generated for the test cases. sleep - # for a bit to allow the kill'd server process to terminate. - time.sleep(.3) + # metadata, targets, and key files generated for the test cases shutil.rmtree(cls.temporary_directory) @@ -1231,6 +1230,7 @@ def test_6_get_one_valid_targetinfo(self): '/foo/foo1.1.tar.gz') server_process.kill() + server_process.wait() @@ -1488,6 +1488,7 @@ def test_7_updated_targets(self): self.assertEqual(len(updated_targets), 1) server_process.kill() + server_process.wait() @@ -1597,6 +1598,7 @@ def test_8_remove_obsolete_targets(self): self.repository_updater.remove_obsolete_targets(destination_directory) server_process.kill() + server_process.wait() @@ -1953,19 +1955,19 @@ def tearDown(self): if self.server_process.returncode is None: logger.info('Server process ' + str(self.server_process.pid) + ' terminated.') self.server_process.kill() + self.server_process.wait() if self.server_process2.returncode is None: logger.info('Server 2 process ' + str(self.server_process2.pid) + ' terminated.') self.server_process2.kill() + self.server_process2.wait() # updater.Updater() populates the roledb with the name "test_repository1" tuf.roledb.clear_roledb(clear_all=True) tuf.keydb.clear_keydb(clear_all=True) # Remove the temporary repository directory, which should contain all the - # metadata, targets, and key files generated of all the test cases. sleep - # for a bit to allow the kill'd server processes to terminate. - time.sleep(.3) + # metadata, targets, and key files generated of all the test cases shutil.rmtree(self.temporary_directory) From 740be9cdb63bf66eb92aaa38b5973e91467eca8f Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 3 Aug 2020 20:58:02 +0300 Subject: [PATCH 04/11] tests: Avoid sleep(): make sure servers really start * Add utility function to wait on a socket until it responds * Use the function instead of sleeping in tests that need to wait for the server to start * Increase the max timeout to 5 seconds by default (as appveyor builds still seem to hit the 3 second mark sometimes) wait_for_server() functions quite differently depending on OS: Windows can take 2 seconds to respond with ECONNREFUSED whereas Linux is almost instant. There might be tricks to be faster on Windows (like setting a shorter socket timeout) but this was not done here. This makes a full Linux test run almost 40% faster and should be more reproducible on every platform. Signed-off-by: Jussi Kukkonen --- tests/test_arbitrary_package_attack.py | 6 +- tests/test_download.py | 18 ++--- tests/test_endless_data_attack.py | 6 +- tests/test_extraneous_dependencies_attack.py | 6 +- tests/test_indefinite_freeze_attack.py | 10 +-- tests/test_key_revocation_integration.py | 6 +- tests/test_mix_and_match_attack.py | 6 +- .../test_multiple_repositories_integration.py | 11 +-- tests/test_proxy_use.py | 15 ++-- tests/test_replay_attack.py | 6 +- tests/test_updater.py | 49 ++---------- .../test_updater_root_rotation_integration.py | 7 +- tests/utils.py | 76 +++++++++++++++++++ 13 files changed, 124 insertions(+), 98 deletions(-) create mode 100644 tests/utils.py diff --git a/tests/test_arbitrary_package_attack.py b/tests/test_arbitrary_package_attack.py index 639440ced8..a5d840ddad 100755 --- a/tests/test_arbitrary_package_attack.py +++ b/tests/test_arbitrary_package_attack.py @@ -55,6 +55,8 @@ import tuf.client.updater as updater import tuf.unittest_toolbox as unittest_toolbox +import utils + import securesystemslib import six @@ -87,9 +89,7 @@ def setUpClass(cls): logger.info('Serving on port: ' + str(cls.SERVER_PORT)) cls.url = 'http://localhost:' + str(cls.SERVER_PORT) + os.path.sep - # NOTE: Following error is raised if a delay is not applied: - # - time.sleep(1) + utils.wait_for_server('localhost', cls.SERVER_PORT) diff --git a/tests/test_download.py b/tests/test_download.py index 46d27410ac..c6cb595f88 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -48,6 +48,8 @@ import tuf.unittest_toolbox as unittest_toolbox import tuf.exceptions +import utils + import requests.exceptions import securesystemslib @@ -81,13 +83,7 @@ def setUp(self): junk, rel_target_filepath = os.path.split(target_filepath) self.url = 'http://localhost:'+str(self.PORT)+'/'+rel_target_filepath - # Provide a delay long enough to allow the HTTPS servers to start. - # Encountered an error on one test system at delay value of 0.2s, so - # increasing to 0.5s. Further increasing to 2s due to occasional failures - # in other tests in similar circumstances on AppVeyor. - # Expect to see "Connection refused" if this delay is not long enough - # (though other issues could cause that). - time.sleep(2) + utils.wait_for_server('localhost', self.PORT) # Computing hash of target file data. m = hashlib.md5() @@ -279,12 +275,8 @@ def test_https_connection(self): expd_https_server_proc = popen_python( ['simple_https_server.py', port4, expired_cert_fname]) - # Provide a delay long enough to allow the four HTTPS servers to start. - # Have encountered errors at 0.2s, 0.5s, and 2s, primarily on AppVeyor. - # Increasing to 4s for this test. - # Expect to see "Connection refused" if this delay is not long enough - # (though other issues could cause that). - time.sleep(3) + for port in range(self.PORT + 1, self.PORT + 5): + utils.wait_for_server('localhost', port) relative_target_fpath = os.path.basename(target_filepath) good_https_url = 'https://localhost:' + port1 + '/' + relative_target_fpath diff --git a/tests/test_endless_data_attack.py b/tests/test_endless_data_attack.py index d5f9713e96..4e81299631 100755 --- a/tests/test_endless_data_attack.py +++ b/tests/test_endless_data_attack.py @@ -57,6 +57,8 @@ import tuf.unittest_toolbox as unittest_toolbox import tuf.roledb +import utils + import securesystemslib import six @@ -89,9 +91,7 @@ def setUpClass(cls): logger.info('Serving on port: '+str(cls.SERVER_PORT)) cls.url = 'http://localhost:'+str(cls.SERVER_PORT) + os.path.sep - # NOTE: Following error is raised if a delay is not applied: - # - time.sleep(.8) + utils.wait_for_server('localhost', cls.SERVER_PORT) diff --git a/tests/test_extraneous_dependencies_attack.py b/tests/test_extraneous_dependencies_attack.py index ae57e22d46..fc5051a57b 100755 --- a/tests/test_extraneous_dependencies_attack.py +++ b/tests/test_extraneous_dependencies_attack.py @@ -60,6 +60,8 @@ import tuf.keydb import tuf.unittest_toolbox as unittest_toolbox +import utils + import securesystemslib import six @@ -93,9 +95,7 @@ def setUpClass(cls): logger.info('Serving on port: '+str(cls.SERVER_PORT)) cls.url = 'http://localhost:'+str(cls.SERVER_PORT) + os.path.sep - # NOTE: Following error is raised if a delay is not applied: - # - time.sleep(.7) + utils.wait_for_server('localhost', cls.SERVER_PORT) diff --git a/tests/test_indefinite_freeze_attack.py b/tests/test_indefinite_freeze_attack.py index f1b1fa43ef..1f3075acf9 100755 --- a/tests/test_indefinite_freeze_attack.py +++ b/tests/test_indefinite_freeze_attack.py @@ -64,6 +64,8 @@ import tuf.keydb import tuf.exceptions +import utils + import securesystemslib import six @@ -100,13 +102,7 @@ def setUpClass(cls): logger.info('Serving on port: '+str(cls.SERVER_PORT)) cls.url = 'http://localhost:'+str(cls.SERVER_PORT) + os.path.sep - # Provide a delay long enough to allow the HTTP server to start. - # Encountered an error on one test system at delay value of 0.2s, so - # increasing to 0.5s. Further increasing to 2s due to occasional failures - # in other tests in similar circumstances on AppVeyor. - # Expect to see "Connection refused" if this delay is not long enough - # (though other issues could cause that). - time.sleep(2) + utils.wait_for_server('localhost', cls.SERVER_PORT) diff --git a/tests/test_key_revocation_integration.py b/tests/test_key_revocation_integration.py index 91a794e47b..b0693d109d 100755 --- a/tests/test_key_revocation_integration.py +++ b/tests/test_key_revocation_integration.py @@ -56,6 +56,8 @@ import tuf.unittest_toolbox as unittest_toolbox import tuf.client.updater as updater +import utils + import securesystemslib import six @@ -89,9 +91,7 @@ def setUpClass(cls): logger.info('\tServing on port: '+str(cls.SERVER_PORT)) cls.url = 'http://localhost:'+str(cls.SERVER_PORT) + os.path.sep - # NOTE: Following error is raised if a delay is not applied: - # - time.sleep(1) + utils.wait_for_server('localhost', cls.SERVER_PORT) diff --git a/tests/test_mix_and_match_attack.py b/tests/test_mix_and_match_attack.py index 3fe578a85a..185bd22741 100755 --- a/tests/test_mix_and_match_attack.py +++ b/tests/test_mix_and_match_attack.py @@ -57,6 +57,8 @@ import tuf.roledb import tuf.keydb +import utils + import securesystemslib import six @@ -94,9 +96,7 @@ def setUpClass(cls): logger.info('Serving on port: '+str(cls.SERVER_PORT)) cls.url = 'http://localhost:'+str(cls.SERVER_PORT) + os.path.sep - # NOTE: Following error is raised if a delay is not applied: - # - time.sleep(.8) + utils.wait_for_server('localhost', cls.SERVER_PORT) diff --git a/tests/test_multiple_repositories_integration.py b/tests/test_multiple_repositories_integration.py index 35b1b094a5..8a29c457e5 100755 --- a/tests/test_multiple_repositories_integration.py +++ b/tests/test_multiple_repositories_integration.py @@ -49,6 +49,8 @@ import tuf.unittest_toolbox as unittest_toolbox import tuf.repository_tool as repo_tool +import utils + import six import securesystemslib @@ -153,13 +155,8 @@ def setUp(self): self.url = 'http://localhost:' + str(self.SERVER_PORT) + os.path.sep self.url2 = 'http://localhost:' + str(self.SERVER_PORT2) + os.path.sep - # 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' - # 0.3s led to occasional failures in automated builds, primarily on - # AppVeyor, so increasing this to 2s, sadly. - time.sleep(2) + utils.wait_for_server('localhost', self.SERVER_PORT) + utils.wait_for_server('localhost', self.SERVER_PORT2) url_prefix = 'http://localhost:' + str(self.SERVER_PORT) url_prefix2 = 'http://localhost:' + str(self.SERVER_PORT2) diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index 76626ca79e..86b8d013aa 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -50,6 +50,8 @@ import tuf.unittest_toolbox as unittest_toolbox import tuf.exceptions +import utils + import requests.exceptions import securesystemslib @@ -118,15 +120,10 @@ def setUpClass(cls): # the type of connection used with the target server. cls.https_proxy_addr = 'https://127.0.0.1:' + str(cls.https_proxy_port) - # Give the HTTP server and proxy server processes a little bit of time to - # start listening before allowing tests to begin, lest we get "Connection - # refused" errors. On the first test system. 0.1s was too short and 0.15s - # was long enough. Use 0.5s to be safe, and if issues arise, increase it. - # Observed occasional failures at 0.1s, 0.15s, 0.5s, and 2s, primarily on - # AppVeyor. Increasing to 4s. This setup runs once for the module. - time.sleep(4) - - + utils.wait_for_server('127.0.0.1', cls.http_port) + utils.wait_for_server('127.0.0.1', cls.https_port) + utils.wait_for_server('127.0.0.1', cls.http_proxy_port) + utils.wait_for_server('127.0.0.1', cls.https_proxy_port) diff --git a/tests/test_replay_attack.py b/tests/test_replay_attack.py index e6fc5d9530..27d915f2ef 100755 --- a/tests/test_replay_attack.py +++ b/tests/test_replay_attack.py @@ -56,6 +56,8 @@ import tuf.repository_tool as repo_tool import tuf.unittest_toolbox as unittest_toolbox +import utils + import securesystemslib import six @@ -93,9 +95,7 @@ def setUpClass(cls): logger.info('Serving on port: '+str(cls.SERVER_PORT)) cls.url = 'http://localhost:'+str(cls.SERVER_PORT) + os.path.sep - # NOTE: Following error is raised if a delay is not applied: - # - time.sleep(.8) + utils.wait_for_server('localhost', cls.SERVER_PORT) diff --git a/tests/test_updater.py b/tests/test_updater.py index 0b06682712..79af5c8d6b 100644 --- a/tests/test_updater.py +++ b/tests/test_updater.py @@ -72,6 +72,8 @@ import tuf.unittest_toolbox as unittest_toolbox import tuf.client.updater as updater +import utils + import securesystemslib import six @@ -110,14 +112,7 @@ def setUpClass(cls): logger.info('\tServing on port: '+str(cls.SERVER_PORT)) cls.url = 'http://localhost:'+str(cls.SERVER_PORT) + os.path.sep - # NOTE: Following error is raised if a delay is not long enough to allow - # the server process to set up and start listening: - # - # or, on Windows: - # Failed to establish a new connection: [Errno 111] Connection refused' - # While 0.3s has consistently worked on Travis and local builds, it led to - # occasional failures in AppVeyor builds, so increasing this to 2s, sadly. - time.sleep(2) + utils.wait_for_server('localhost', cls.SERVER_PORT) @@ -1100,13 +1095,7 @@ def test_6_get_one_valid_targetinfo(self): command = ['python', self.SIMPLE_SERVER_PATH, str(SERVER_PORT)] server_process = subprocess.Popen(command) - # 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' - # While 0.3s has consistently worked on Travis and local builds, it led to - # occasional failures in AppVeyor builds, so increasing this to 2s, sadly. - time.sleep(2) + utils.wait_for_server('localhost', SERVER_PORT) # 'path/to/tmp/repository' -> 'localhost:8001/tmp/repository'. repository_basepath = self.repository_directory[len(os.getcwd()):] @@ -1368,15 +1357,7 @@ def test_7_updated_targets(self): SERVER_PORT = random.randint(30000, 45000) command = ['python', self.SIMPLE_SERVER_PATH, str(SERVER_PORT)] server_process = subprocess.Popen(command) - - # NOTE: Following error is raised if a delay is not long enough to allow - # the server process to set up and start listening: - # - # or, on Windows: - # Failed to establish a new connection: [Errno 111] Connection refused' - # While 0.3s has consistently worked on Travis and local builds, it led to - # occasional failures in AppVeyor builds, so increasing this to 2s, sadly. - time.sleep(2) + utils.wait_for_server('localhost', SERVER_PORT) # 'path/to/tmp/repository' -> 'localhost:8001/tmp/repository'. repository_basepath = self.repository_directory[len(os.getcwd()):] @@ -1501,15 +1482,7 @@ def test_8_remove_obsolete_targets(self): SERVER_PORT = random.randint(30000, 45000) command = ['python', self.SIMPLE_SERVER_PATH, str(SERVER_PORT)] server_process = subprocess.Popen(command) - - # NOTE: Following error is raised if a delay is not long enough to allow - # the server process to set up and start listening: - # - # or, on Windows: - # Failed to establish a new connection: [Errno 111] Connection refused' - # While 0.3s has consistently worked on Travis and local builds, it led to - # occasional failures in AppVeyor builds, so increasing this to 2s, sadly. - time.sleep(2) + utils.wait_for_server('localhost', SERVER_PORT) # 'path/to/tmp/repository' -> 'localhost:8001/tmp/repository'. repository_basepath = self.repository_directory[len(os.getcwd()):] @@ -1908,14 +1881,8 @@ def setUp(self): self.url = 'http://localhost:' + str(self.SERVER_PORT) + os.path.sep self.url2 = 'http://localhost:' + str(self.SERVER_PORT2) + os.path.sep - # NOTE: Following error is raised if a delay is not long enough to allow - # the server process to set up and start listening: - # - # or, on Windows: - # Failed to establish a new connection: [Errno 111] Connection refused' - # While 0.3s has consistently worked on Travis and local builds, it led to - # occasional failures in AppVeyor builds, so increasing this to 2s, sadly. - time.sleep(2) + utils.wait_for_server('localhost', self.SERVER_PORT) + utils.wait_for_server('localhost', self.SERVER_PORT2) url_prefix = 'http://localhost:' + str(self.SERVER_PORT) url_prefix2 = 'http://localhost:' + str(self.SERVER_PORT2) diff --git a/tests/test_updater_root_rotation_integration.py b/tests/test_updater_root_rotation_integration.py index c73c9f7045..6450cb9504 100755 --- a/tests/test_updater_root_rotation_integration.py +++ b/tests/test_updater_root_rotation_integration.py @@ -63,6 +63,8 @@ import tuf.client.updater as updater import tuf.settings +import utils + import securesystemslib import six @@ -96,9 +98,8 @@ def setUpClass(cls): logger.info('\tServing on port: '+str(cls.SERVER_PORT)) cls.url = 'http://localhost:'+str(cls.SERVER_PORT) + os.path.sep - # NOTE: Following error is raised if a delay is not applied: - # - time.sleep(1) + utils.wait_for_server('localhost', cls.SERVER_PORT) + diff --git a/tests/utils.py b/tests/utils.py new file mode 100644 index 0000000000..b9c9768f6b --- /dev/null +++ b/tests/utils.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python + +# Copyright 2020, TUF contributors +# SPDX-License-Identifier: MIT OR Apache-2.0 + +""" + + utils.py + + + August 3, 2020. + + + Jussi Kukkonen + + + See LICENSE-MIT OR LICENSE for licensing information. + + + Provide common utilities for TUF tests +""" + +import errno +import logging +import socket +import time + +logger = logging.getLogger(__name__) + +try: + # is defined in Python 3 + TimeoutError +except NameError: + # Define for Python 2 + class TimeoutError(Exception): + + def __init__(self, value="Timeout"): + self.value = value + + def __str__(self): + return repr(self.value) + +# Wait until host:port accepts connections. +# Raises TimeoutError if this does not happen within timeout seconds +# There are major differences between operating systems on how this works +# but the current blocking connect() seems to work fast on Linux and seems +# to at least work on Windows (ECONNREFUSED unfortunately has a 2 second +# timeout on Windows) +def wait_for_server(host, port, timeout=5): + start = time.time() + remaining_timeout = timeout + succeeded = False + while not succeeded and remaining_timeout > 0: + try: + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.settimeout(remaining_timeout) + sock.connect((host, port)) + succeeded = True + except socket.timeout as e: + pass + except IOError as e: + # ECONNREFUSED is expected while the server is not started + if e.errno not in [errno.ECONNREFUSED]: + logger.warning("Unexpected error while waiting for server: " + str(e)) + # Avoid pegging a core just for this + time.sleep(0.01) + finally: + if sock: + sock.close() + sock = None + remaining_timeout = timeout - (time.time() - start) + + if not succeeded: + raise TimeoutError + + From f468007acde6b968066461680934fa8d1bedea05 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Tue, 4 Aug 2020 16:35:09 +0300 Subject: [PATCH 05/11] test_download: Use logger for output Signed-off-by: Jussi Kukkonen --- tests/test_download.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_download.py b/tests/test_download.py index c6cb595f88..6ac72c1dcc 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -308,13 +308,13 @@ def test_https_connection(self): # trusting the good certs (trusting the bad cert instead). Expect failure # because even though the server's cert file is otherwise OK, we don't # trust it. - print('Trying HTTPS download of target file: ' + good_https_url) + logger.info('Trying HTTPS download of target file: ' + good_https_url) with self.assertRaises(requests.exceptions.SSLError): download.safe_download(good_https_url, target_data_length) with self.assertRaises(requests.exceptions.SSLError): download.unsafe_download(good_https_url, target_data_length) - print('Trying HTTPS download of target file: ' + good2_https_url) + logger.info('Trying HTTPS download of target file: ' + good2_https_url) with self.assertRaises(requests.exceptions.SSLError): download.safe_download(good2_https_url, target_data_length) with self.assertRaises(requests.exceptions.SSLError): From 0dfa17038e04494670906b3128cbb6d4d0442b78 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Tue, 4 Aug 2020 17:03:21 +0300 Subject: [PATCH 06/11] tests: Remember to close file objects This silences "ResourceWarning: unclosed file" Signed-off-by: Jussi Kukkonen --- tests/test_download.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_download.py b/tests/test_download.py index 6ac72c1dcc..5e2facb4a4 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -127,8 +127,8 @@ def test_download_url_to_tempfileobj_and_lengths(self): # the server-reported length of the file does not match the # required_length. 'updater.py' *does* verify the hashes of downloaded # content. - download.safe_download(self.url, self.target_data_length - 4) - download.unsafe_download(self.url, self.target_data_length - 4) + download.safe_download(self.url, self.target_data_length - 4).close() + download.unsafe_download(self.url, self.target_data_length - 4).close() # We catch 'tuf.exceptions.DownloadLengthMismatchError' for safe_download() # because it will not download more bytes than requested (in this case, a @@ -138,7 +138,7 @@ def test_download_url_to_tempfileobj_and_lengths(self): # Calling unsafe_download() with a mismatched length should not raise an # exception. - download.unsafe_download(self.url, self.target_data_length + 1) + download.unsafe_download(self.url, self.target_data_length + 1).close() From 6b1011d748e551c9eacfb7c0598771dfd639342f Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Tue, 4 Aug 2020 17:46:30 +0300 Subject: [PATCH 07/11] log: Close the file handler when it's removed Silences "ResourceWarning: unclosed file" Signed-off-by: Jussi Kukkonen --- tuf/log.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tuf/log.py b/tuf/log.py index 6a17a18710..8a6a84d20a 100755 --- a/tuf/log.py +++ b/tuf/log.py @@ -450,6 +450,7 @@ def disable_file_logging(): if file_handler: logger.removeHandler(file_handler) + file_handler.close() file_handler = None logger.debug('Removed the file handler.') From a22f6182dda6468d343ceed568cdb98c64cbadc2 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 5 Aug 2020 16:05:07 +0300 Subject: [PATCH 08/11] tests: increase wait_for_server() default timeout Travis managed to still timeout with 5 seconds: increasing the timeout (now that it's not a sleep) doesn't really hurt normal non-VM use cases so let's bump it to 10 seconds. Signed-off-by: Jussi Kukkonen --- tests/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index b9c9768f6b..02b0820c3e 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -46,7 +46,7 @@ def __str__(self): # but the current blocking connect() seems to work fast on Linux and seems # to at least work on Windows (ECONNREFUSED unfortunately has a 2 second # timeout on Windows) -def wait_for_server(host, port, timeout=5): +def wait_for_server(host, port, timeout=10): start = time.time() remaining_timeout = timeout succeeded = False From fc44652b93a59218d19dc39ddfc277059d975c94 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Fri, 7 Aug 2020 14:33:12 +0300 Subject: [PATCH 09/11] tests: Use localhost consistently Our tests already expect localhost lookup to work to find test servers: use it consistently instead of sometimes using 127.0.0.1 Signed-off-by: Jussi Kukkonen --- tests/proxy_server.py | 2 +- tests/test_proxy_use.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/proxy_server.py b/tests/proxy_server.py index 3c76043c59..910fc4868b 100644 --- a/tests/proxy_server.py +++ b/tests/proxy_server.py @@ -466,7 +466,7 @@ def test(HandlerClass=ProxyRequestHandler, ServerClass=ThreadingHTTPServer, prot port = int(sys.argv[1]) else: port = 8080 - server_address = ('127.0.0.1', port) # MODIFIED: changed from '::1' + server_address = ('localhost', port) # MODIFIED: Argument added, conditional below added to control INTERCEPT # setting. diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index 86b8d013aa..6fdb3b47c2 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -118,12 +118,12 @@ def setUpClass(cls): os.path.join('ssl_certs', 'ssl_cert.crt')]) # Note that the HTTPS proxy server's address uses https://, regardless of # the type of connection used with the target server. - cls.https_proxy_addr = 'https://127.0.0.1:' + str(cls.https_proxy_port) + cls.https_proxy_addr = 'https://localhost:' + str(cls.https_proxy_port) - utils.wait_for_server('127.0.0.1', cls.http_port) - utils.wait_for_server('127.0.0.1', cls.https_port) - utils.wait_for_server('127.0.0.1', cls.http_proxy_port) - utils.wait_for_server('127.0.0.1', cls.https_proxy_port) + utils.wait_for_server('localhost', cls.http_port) + utils.wait_for_server('localhost', cls.https_port) + utils.wait_for_server('localhost', cls.http_proxy_port) + utils.wait_for_server('localhost', cls.https_proxy_port) From c7425bec13350d84bbb4fd5aaa9c4dfa5be2be3b Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Fri, 7 Aug 2020 14:39:37 +0300 Subject: [PATCH 10/11] test_proxy_use: Error out early if using Python3 proxy_server.py is python2 only, don't try to setup the class as it leads to a confusing TimeoutError. This may prevent me from debugging this apparent test failure a third time. Signed-off-by: Jussi Kukkonen --- tests/test_proxy_use.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index 6fdb3b47c2..ae6e6088dc 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -78,6 +78,10 @@ def setUpClass(cls): unittest_toolbox.Modified_TestCase.setUpClass() + if not six.PY2: + raise NotImplementedError("TestWithProxies only works with Python 2" + " (proxy_server.py is Python2 only)") + # Launch a simple HTTP server (serves files in the current dir). cls.http_port = random.randint(30000, 45000) cls.http_server_proc = popen_python( From b6661e024ae6612585005a7244c276f679ed9eb8 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Fri, 7 Aug 2020 15:00:31 +0300 Subject: [PATCH 11/11] tests: Remove unused imports Signed-off-by: Jussi Kukkonen --- tests/simple_server.py | 1 - tests/test_arbitrary_package_attack.py | 2 -- tests/test_developer_tool.py | 3 --- tests/test_download.py | 2 -- tests/test_endless_data_attack.py | 2 -- tests/test_extraneous_dependencies_attack.py | 2 -- tests/test_indefinite_freeze_attack.py | 1 - tests/test_key_revocation_integration.py | 3 --- tests/test_mirrors.py | 1 - tests/test_mix_and_match_attack.py | 4 ---- tests/test_multiple_repositories_integration.py | 3 --- tests/test_proxy_use.py | 5 ----- tests/test_replay_attack.py | 3 --- tests/test_repository_lib.py | 3 --- tests/test_repository_tool.py | 3 --- tests/test_root_versioning_integration.py | 1 - tests/test_updater.py | 1 - tests/test_updater_root_rotation_integration.py | 3 --- 18 files changed, 43 deletions(-) diff --git a/tests/simple_server.py b/tests/simple_server.py index 5cfbd71383..746b5ae456 100755 --- a/tests/simple_server.py +++ b/tests/simple_server.py @@ -35,7 +35,6 @@ import sys import random -import platform import six from six.moves.SimpleHTTPServer import SimpleHTTPRequestHandler diff --git a/tests/test_arbitrary_package_attack.py b/tests/test_arbitrary_package_attack.py index a5d840ddad..6ce0ed691d 100755 --- a/tests/test_arbitrary_package_attack.py +++ b/tests/test_arbitrary_package_attack.py @@ -39,12 +39,10 @@ import os import tempfile import random -import time import shutil import json import subprocess import logging -import sys import unittest import tuf diff --git a/tests/test_developer_tool.py b/tests/test_developer_tool.py index b595aeda61..27ceeb76e5 100755 --- a/tests/test_developer_tool.py +++ b/tests/test_developer_tool.py @@ -22,13 +22,10 @@ """ import os -import time -import datetime import unittest import logging import tempfile import shutil -import unittest import tuf import tuf.log diff --git a/tests/test_download.py b/tests/test_download.py index 5e2facb4a4..2f97048ac6 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -39,7 +39,6 @@ import random import subprocess import sys -import time import unittest import tuf @@ -53,7 +52,6 @@ import requests.exceptions import securesystemslib -import six logger = logging.getLogger(__name__) diff --git a/tests/test_endless_data_attack.py b/tests/test_endless_data_attack.py index 4e81299631..14452a6b85 100755 --- a/tests/test_endless_data_attack.py +++ b/tests/test_endless_data_attack.py @@ -42,12 +42,10 @@ import os import tempfile import random -import time import shutil import json import subprocess import logging -import sys import unittest import tuf diff --git a/tests/test_extraneous_dependencies_attack.py b/tests/test_extraneous_dependencies_attack.py index fc5051a57b..421b4983a6 100755 --- a/tests/test_extraneous_dependencies_attack.py +++ b/tests/test_extraneous_dependencies_attack.py @@ -45,12 +45,10 @@ import os import tempfile import random -import time import shutil import json import subprocess import logging -import sys import unittest import tuf.formats diff --git a/tests/test_indefinite_freeze_attack.py b/tests/test_indefinite_freeze_attack.py index 1f3075acf9..80ef4f7f02 100755 --- a/tests/test_indefinite_freeze_attack.py +++ b/tests/test_indefinite_freeze_attack.py @@ -52,7 +52,6 @@ import json import subprocess import logging -import sys import unittest import tuf.formats diff --git a/tests/test_key_revocation_integration.py b/tests/test_key_revocation_integration.py index b0693d109d..1e72443aa0 100755 --- a/tests/test_key_revocation_integration.py +++ b/tests/test_key_revocation_integration.py @@ -38,14 +38,11 @@ from __future__ import unicode_literals import os -import time import shutil -import copy import tempfile import logging import random import subprocess -import sys import unittest import tuf diff --git a/tests/test_mirrors.py b/tests/test_mirrors.py index fa68d6491e..aa5efc7fb3 100755 --- a/tests/test_mirrors.py +++ b/tests/test_mirrors.py @@ -30,7 +30,6 @@ import unittest -import tuf import tuf.mirrors as mirrors import tuf.unittest_toolbox as unittest_toolbox diff --git a/tests/test_mix_and_match_attack.py b/tests/test_mix_and_match_attack.py index 185bd22741..f077cb6ed1 100755 --- a/tests/test_mix_and_match_attack.py +++ b/tests/test_mix_and_match_attack.py @@ -41,12 +41,9 @@ import os import tempfile import random -import time import shutil -import json import subprocess import logging -import sys import unittest import tuf.exceptions @@ -59,7 +56,6 @@ import utils -import securesystemslib import six # The repository tool is imported and logs console messages by default. diff --git a/tests/test_multiple_repositories_integration.py b/tests/test_multiple_repositories_integration.py index 8a29c457e5..d3c0c9717c 100755 --- a/tests/test_multiple_repositories_integration.py +++ b/tests/test_multiple_repositories_integration.py @@ -30,12 +30,10 @@ from __future__ import unicode_literals import os -import sys import tempfile import random import subprocess import logging -import time import shutil import unittest import json @@ -45,7 +43,6 @@ import tuf.roledb import tuf.client.updater as updater import tuf.settings -import securesystemslib import tuf.unittest_toolbox as unittest_toolbox import tuf.repository_tool as repo_tool diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index ae6e6088dc..744880b78a 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -35,13 +35,11 @@ from __future__ import division from __future__ import unicode_literals -import hashlib import logging import os import random import subprocess import sys -import time import unittest import tuf @@ -52,9 +50,6 @@ import utils -import requests.exceptions - -import securesystemslib import six logger = logging.getLogger(__name__) diff --git a/tests/test_replay_attack.py b/tests/test_replay_attack.py index 27d915f2ef..1fdd8915d8 100755 --- a/tests/test_replay_attack.py +++ b/tests/test_replay_attack.py @@ -41,13 +41,10 @@ import os import tempfile import random -import time import datetime import shutil -import json import subprocess import logging -import sys import unittest import tuf.formats diff --git a/tests/test_repository_lib.py b/tests/test_repository_lib.py index 11fbf4aa55..7b9b8243c8 100755 --- a/tests/test_repository_lib.py +++ b/tests/test_repository_lib.py @@ -35,10 +35,7 @@ import tempfile import json import shutil -import stat -import sys import unittest -import platform import tuf import tuf.formats diff --git a/tests/test_repository_tool.py b/tests/test_repository_tool.py index e27919a6e9..bdea70d500 100755 --- a/tests/test_repository_tool.py +++ b/tests/test_repository_tool.py @@ -35,8 +35,6 @@ import logging import tempfile import shutil -import sys -import errno import tuf import tuf.log @@ -49,7 +47,6 @@ import securesystemslib import securesystemslib.storage -import six logger = logging.getLogger(__name__) diff --git a/tests/test_root_versioning_integration.py b/tests/test_root_versioning_integration.py index 815674040e..a73c2ade73 100755 --- a/tests/test_root_versioning_integration.py +++ b/tests/test_root_versioning_integration.py @@ -29,7 +29,6 @@ import logging import tempfile import shutil -import sys import unittest import tuf diff --git a/tests/test_updater.py b/tests/test_updater.py index 79af5c8d6b..068c298223 100644 --- a/tests/test_updater.py +++ b/tests/test_updater.py @@ -57,7 +57,6 @@ import logging import random import subprocess -import sys import errno import unittest diff --git a/tests/test_updater_root_rotation_integration.py b/tests/test_updater_root_rotation_integration.py index 6450cb9504..c49b82a726 100755 --- a/tests/test_updater_root_rotation_integration.py +++ b/tests/test_updater_root_rotation_integration.py @@ -42,14 +42,11 @@ from __future__ import unicode_literals import os -import time import shutil -import copy import tempfile import logging import random import subprocess -import sys import unittest import filecmp