From 7dbb30ae1006d0b5f506bba19d4b53398c44f139 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Mon, 24 Feb 2020 14:51:28 +0100 Subject: [PATCH 1/2] Fix failing AppVeyor Python2.7 tests Since #885 the tests in TestUpdater and TestKeyRevocation fail on Appveyor Python 2.7 builds. After some live debugging, it turns out that the tests fail due to the extra amount of http requests to the simple http server (see tests/simple_server.py) that were added in #885. The simple server runs in a subprocess and is re-used for the entire TestCase. After a certain amount of requests it becomes unresponsive. Note that neither the subprocess exits (ps -W), nor does the port get closed (netstat -a). It just doesn't serve the request, making it time out and fail the test. The following script can be used to reproduce the issue (run in tests directory): ```python import subprocess import requests import random counter = 0 port = random.randint(30000, 45000) command = ['python', 'simple_server.py', str(port)] server_process = subprocess.Popen(command, stderr=subprocess.PIPE) url = 'http://localhost:'+str(port) + '/' sess = requests.Session() try: while True: sess.get(url, timeout=3) counter +=1 finally: print(counter) server_process.kill() ``` It fails repeatedly on the 69th request, but only if `stderr=subprocess.PIPE` is passed to Popen. Given that for each request the simple server writes about ~60 characters to stderr, e.g. ... ``` 127.0.0.1 - - [24/Feb/2020 12:01:23] "GET / HTTP/1.1" 200 - ``` ... it looks a lot like a full pipe buffer of size 4096. Note that the `bufsize` argument to Popen does not change anything. As a simple work around we silence the test server on Windows/Python2 to not fill the buffer. Signed-off-by: Lukas Puehringer --- tests/simple_server.py | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/simple_server.py b/tests/simple_server.py index 26d0d513..d1343430 100755 --- a/tests/simple_server.py +++ b/tests/simple_server.py @@ -35,8 +35,10 @@ import sys import random +import platform import six +from six.moves.SimpleHTTPServer import SimpleHTTPRequestHandler PORT = 0 @@ -55,7 +57,22 @@ def _port_gen(): else: PORT = _port_gen() -Handler = six.moves.SimpleHTTPServer.SimpleHTTPRequestHandler -httpd = six.moves.socketserver.TCPServer(('', PORT), Handler) + +class QuietHTTPRequestHandler(SimpleHTTPRequestHandler): + """A SimpleHTTPRequestHandler that does not write incoming requests to + stderr. """ + def log_request(self, code='-', size='-'): + pass + +# NOTE: On Windows/Python2 tests that use this simple_server.py in a +# subprocesses hang after a certain amount of requests (~68), if a PIPE is +# passed as Popen's stderr argument. As a simple workaround we silence the +# server on those Windows/Py2 to not fill the buffer. +if six.PY2 and platform.system() == 'Windows': + handler = QuietHTTPRequestHandler +else: + handler = SimpleHTTPRequestHandler + +httpd = six.moves.socketserver.TCPServer(('', PORT), handler) httpd.serve_forever() From 842f8432102b24f5c6a62f2521591a66c2052ca9 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Tue, 25 Feb 2020 12:28:00 +0100 Subject: [PATCH 2/2] Remove duplicate testing simple_server.py tests/simple_server.py was copied to tuf/scripts/ to "make testing easier" (cf84d3f51f1c7093d5f37e9caaaac10552cc9d66), although with the current test setup the original (and recently patched to fix an Windows/Py2 test issue) test simple_server.py can be used just as well. This commit: - removes tuf/scripts/simple_server.py Note: that version slightly differed from the original test server, probably due to demands by the linter that is only executed on the tuf core code and not on the tests. However, for the testing purposes of simple_server.py these changes (i.e., `SystemRandom()`, `if __name__ =='__main__':`) are not necessary. - updates the tests that used tuf.scripts.simple_server to instead use tests.simple_server, - updates setup.py to not install the simple_server module as script, when installing tuf, as it is only a testing script and not meant for end-user usage. Signed-off-by: Lukas Puehringer --- setup.py | 3 +- .../test_multiple_repositories_integration.py | 4 +- tests/test_updater.py | 12 ++-- tuf/scripts/simple_server.py | 66 ------------------- 4 files changed, 9 insertions(+), 76 deletions(-) delete mode 100755 tuf/scripts/simple_server.py diff --git a/setup.py b/setup.py index b3d6099d..9e598e10 100755 --- a/setup.py +++ b/setup.py @@ -124,7 +124,6 @@ packages = find_packages(exclude=['tests']), scripts = [ 'tuf/scripts/repo.py', - 'tuf/scripts/client.py', - 'tuf/scripts/simple_server.py', + 'tuf/scripts/client.py' ] ) diff --git a/tests/test_multiple_repositories_integration.py b/tests/test_multiple_repositories_integration.py index 327d9dad..aae18722 100755 --- a/tests/test_multiple_repositories_integration.py +++ b/tests/test_multiple_repositories_integration.py @@ -128,8 +128,8 @@ def setUp(self): while self.SERVER_PORT == self.SERVER_PORT2: self.SERVER_PORT2 = random.SystemRandom().randint(30000, 45000) - command = ['python', '-m', 'tuf.scripts.simple_server', str(self.SERVER_PORT)] - command2 = ['python', '-m', 'tuf.scripts.simple_server', str(self.SERVER_PORT2)] + command = ['python', '-m', 'tests.simple_server', str(self.SERVER_PORT)] + command2 = ['python', '-m', 'tests.simple_server', str(self.SERVER_PORT2)] self.server_process = subprocess.Popen(command, stderr=subprocess.PIPE, cwd=self.repository_directory) diff --git a/tests/test_updater.py b/tests/test_updater.py index 9f5c94d6..1fa89698 100644 --- a/tests/test_updater.py +++ b/tests/test_updater.py @@ -98,7 +98,7 @@ def setUpClass(cls): # as a delegated role 'targets/role1', three target files, five key files, # etc. cls.SERVER_PORT = random.randint(30000, 45000) - command = ['python', '-m', 'tuf.scripts.simple_server', str(cls.SERVER_PORT)] + command = ['python', '-m', 'tests.simple_server', str(cls.SERVER_PORT)] cls.server_process = subprocess.Popen(command, stderr=subprocess.PIPE) logger.info('\n\tServer process started.') logger.info('\tServer process id: '+str(cls.server_process.pid)) @@ -1094,7 +1094,7 @@ def test_6_get_one_valid_targetinfo(self): # The SimpleHTTPServer started in the setupclass has a tendency to # timeout in Windows after a few tests. SERVER_PORT = random.randint(30000, 45000) - command = ['python', '-m', 'tuf.scripts.simple_server', str(SERVER_PORT)] + command = ['python', '-m', 'tests.simple_server', str(SERVER_PORT)] server_process = subprocess.Popen(command, stderr=subprocess.PIPE) # NOTE: Following error is raised if a delay is not long enough: @@ -1361,7 +1361,7 @@ def test_7_updated_targets(self): # The SimpleHTTPServer started in the setupclass has a tendency to # timeout in Windows after a few tests. SERVER_PORT = random.randint(30000, 45000) - command = ['python', '-m', 'tuf.scripts.simple_server', str(SERVER_PORT)] + command = ['python', '-m', 'tests.simple_server', str(SERVER_PORT)] server_process = subprocess.Popen(command, stderr=subprocess.PIPE) # NOTE: Following error is raised if a delay is not long enough to allow @@ -1493,7 +1493,7 @@ def test_8_remove_obsolete_targets(self): # The SimpleHTTPServer started in the setupclass has a tendency to # timeout in Windows after a few tests. SERVER_PORT = random.randint(30000, 45000) - command = ['python', '-m', 'tuf.scripts.simple_server', str(SERVER_PORT)] + command = ['python', '-m', 'tests.simple_server', str(SERVER_PORT)] server_process = subprocess.Popen(command, stderr=subprocess.PIPE) # NOTE: Following error is raised if a delay is not long enough to allow @@ -1877,8 +1877,8 @@ def setUp(self): self.SERVER_PORT = 30001 self.SERVER_PORT2 = 30002 - command = ['python', '-m', 'tuf.scripts.simple_server', str(self.SERVER_PORT)] - command2 = ['python', '-m', 'tuf.scripts.simple_server', str(self.SERVER_PORT2)] + command = ['python', '-m', 'tests.simple_server', str(self.SERVER_PORT)] + command2 = ['python', '-m', 'tests.simple_server', str(self.SERVER_PORT2)] self.server_process = subprocess.Popen(command, stderr=subprocess.PIPE, cwd=self.repository_directory) diff --git a/tuf/scripts/simple_server.py b/tuf/scripts/simple_server.py deleted file mode 100755 index cef776f0..00000000 --- a/tuf/scripts/simple_server.py +++ /dev/null @@ -1,66 +0,0 @@ -#!/usr/bin/env python - -# Copyright 2012 - 2017, New York University and the TUF contributors -# SPDX-License-Identifier: MIT OR Apache-2.0 - -""" - - simple_server.py - - - Konstantin Andrianov. - - - February 15, 2012. - - - See LICENSE-MIT OR LICENSE for licensing information. - - - This is a basic server that was designed to be used in conjunction with - test_download.py to test download.py module. - - - SimpleHTTPServer: - https://docs.python.org/2/library/simplehttpserver.html -""" - -# Help with Python 3 compatibility, where the print statement is a function, an -# implicit relative import is invalid, and the '/' operator performs true -# division. Example: print 'hello world' raises a 'SyntaxError' exception. -from __future__ import print_function -from __future__ import absolute_import -from __future__ import division -from __future__ import unicode_literals - -import sys -import random - -import six - -PORT = 0 - -def _port_gen(): - return random.SystemRandom().randint(30000, 45000) - -if len(sys.argv) > 1: - try: - PORT = int(sys.argv[1]) - - # Enforce arbitrarily chosen port range. - if PORT < 30000 or PORT > 45000: - raise ValueError - - except ValueError: - PORT = _port_gen() - -else: - PORT = _port_gen() - - -if __name__ == '__main__': - - Handler = six.moves.SimpleHTTPServer.SimpleHTTPRequestHandler - httpd = six.moves.socketserver.TCPServer(('', PORT), Handler) - - httpd.serve_forever()