From cb4465bfe67ec9c75722ea10923a6a75005e8f68 Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Tue, 9 Jun 2026 02:08:30 +0200 Subject: [PATCH] pytest: close file handles after use (cont.), and tidy-ups - dante.py, dnsd.py, sshd.py: drop redundant conditions. Spotted in sshd by GitHub Code Quality. - curl.py: comment out `if` to silence CodeQL warning. Reported by GitHub CodeQL Follow-up to 8145476d5dd97d0ec704e9ea65b2f2028b8a945c #21916 Closes #21917 --- tests/http/testenv/caddy.py | 12 ++++++++++-- tests/http/testenv/client.py | 8 ++++---- tests/http/testenv/curl.py | 22 ++++++++++++---------- tests/http/testenv/dante.py | 12 +++++++++--- tests/http/testenv/dnsd.py | 12 +++++++++--- tests/http/testenv/h2o.py | 14 ++++++++++++-- tests/http/testenv/nghttpx.py | 16 ++++++++++++---- tests/http/testenv/sshd.py | 18 +++++++++++++----- tests/http/testenv/vsftpd.py | 12 ++++++++++-- 9 files changed, 91 insertions(+), 35 deletions(-) diff --git a/tests/http/testenv/caddy.py b/tests/http/testenv/caddy.py index eece1a5a39..1d554b1906 100644 --- a/tests/http/testenv/caddy.py +++ b/tests/http/testenv/caddy.py @@ -55,6 +55,7 @@ class Caddy: self._conf_file = os.path.join(self._caddy_dir, 'Caddyfile') self._error_log = os.path.join(self._caddy_dir, 'caddy.log') self._tmp_dir = os.path.join(self._caddy_dir, 'tmp') + self._error_fd = None self._process = None self._http_port = 0 self._https_port = 0 @@ -68,6 +69,11 @@ class Caddy: def port(self) -> int: return self._https_port + def close_log(self): + if self._error_fd: + self._error_fd.close() + self._error_fd = None + def clear_logs(self): self._rmf(self._error_log) @@ -107,8 +113,8 @@ class Caddy: args = [ self._caddy, 'run' ] - caddyerr = open(self._error_log, 'a') - self._process = subprocess.Popen(args=args, cwd=self._caddy_dir, stderr=caddyerr) + self._error_fd = open(self._error_log, 'a') + self._process = subprocess.Popen(args=args, cwd=self._caddy_dir, stderr=self._error_fd) if self._process.returncode is not None: return False return not wait_live or self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT)) @@ -122,7 +128,9 @@ class Caddy: except Exception: self._process.kill() self._process = None + self.close_log() return not wait_dead or self.wait_dead(timeout=timedelta(seconds=5)) + self.close_log() return True def restart(self): diff --git a/tests/http/testenv/client.py b/tests/http/testenv/client.py index 1b4e303ac6..76c4822cbc 100644 --- a/tests/http/testenv/client.py +++ b/tests/http/testenv/client.py @@ -114,10 +114,10 @@ class LocalClient: def dump_logs(self): lines = [] lines.append('>>--stdout ----------------------------------------------\n') - with open(self._stdoutfile) as cstdout: - lines.extend(cstdout.readlines()) + with open(self._stdoutfile) as fd: + lines.extend(fd.readlines()) lines.append('>>--stderr ----------------------------------------------\n') - with open(self._stderrfile) as cstderr: - lines.extend(cstderr.readlines()) + with open(self._stderrfile) as fd: + lines.extend(fd.readlines()) lines.append('<<-------------------------------------------------------\n') return ''.join(lines) diff --git a/tests/http/testenv/curl.py b/tests/http/testenv/curl.py index 0864e2899e..308e29f110 100644 --- a/tests/http/testenv/curl.py +++ b/tests/http/testenv/curl.py @@ -191,13 +191,14 @@ class RunTcpDump: if self._proc: raise Exception('tcpdump still running') lines = [] - for line in open(self._stdoutfile): - m = re.match(r'.* IP 127\.0\.0\.1\.(\d+) [<>] 127\.0\.0\.1\.(\d+):.*', line) - if m: - sport = int(m.group(1)) - dport = int(m.group(2)) - if ports is None or sport in ports or dport in ports: - lines.append(line) + with open(self._stdoutfile) as fd: + for line in fd: + m = re.match(r'.* IP 127\.0\.0\.1\.(\d+) [<>] 127\.0\.0\.1\.(\d+):.*', line) + if m: + sport = int(m.group(1)) + dport = int(m.group(2)) + if ports is None or sport in ports or dport in ports: + lines.append(line) return lines @property @@ -208,7 +209,8 @@ class RunTcpDump: def stderr(self) -> List[str]: if self._proc: raise Exception('tcpdump still running') - return open(self._stderrfile).readlines() + with open(self._stderrfile) as fd: + return fd.readlines() def sample(self): # not sure how to make that detection reliable for all platforms @@ -1027,8 +1029,8 @@ class CurlClient: cwd=self._run_dir, shell=False, env=self._run_env) profile = RunProfile(p.pid, started_at, self._run_dir) - if intext is not None and False: - p.communicate(input=intext.encode(), timeout=1) + #if intext is not None and False: + # p.communicate(input=intext.encode(), timeout=1) if self._with_perf: perf = PerfProfile(p.pid, self._run_dir) perf.start() diff --git a/tests/http/testenv/dante.py b/tests/http/testenv/dante.py index 2feb42eb7c..b2ea4dd0cf 100644 --- a/tests/http/testenv/dante.py +++ b/tests/http/testenv/dante.py @@ -57,6 +57,7 @@ class Dante: self._dante_log = os.path.join(self._dante_dir, 'dante.log') self._error_log = os.path.join(self._dante_dir, 'error.log') self._pid_file = os.path.join(self._dante_dir, 'dante.pid') + self._error_fd = None self._process = None self.clear_logs() @@ -65,6 +66,11 @@ class Dante: def port(self) -> int: return self._port + def close_log(self): + if self._error_fd: + self._error_fd.close() + self._error_fd = None + def clear_logs(self): self._rmf(self._error_log) self._rmf(self._dante_log) @@ -89,7 +95,7 @@ class Dante: self._process.terminate() self._process.wait(timeout=2) self._process = None - return not wait_dead or True + self.close_log() return True def restart(self): @@ -122,8 +128,8 @@ class Dante: '-p', f'{self._pid_file}', '-d', '0', ] - procerr = open(self._error_log, 'a') - self._process = subprocess.Popen(args=args, stderr=procerr) + self._error_fd = open(self._error_log, 'a') + self._process = subprocess.Popen(args=args, stderr=self._error_fd) if self._process.returncode is not None: return False return self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT)) diff --git a/tests/http/testenv/dnsd.py b/tests/http/testenv/dnsd.py index b8ff097280..a7dc088557 100644 --- a/tests/http/testenv/dnsd.py +++ b/tests/http/testenv/dnsd.py @@ -56,6 +56,7 @@ class Dnsd: self._conf_file = os.path.join(self._log_dir, 'dnsd.cmd') self._pid_file = os.path.join(self._log_dir, 'dnsd.pid') self._error_log = os.path.join(self._log_dir, 'dnsd.err.log') + self._error_fd = None self._process = None self.clear_logs() @@ -64,6 +65,11 @@ class Dnsd: def port(self) -> int: return self._port + def close_log(self): + if self._error_fd: + self._error_fd.close() + self._error_fd = None + def clear_logs(self): self._rmf(self._log_file) self._rmf(self._error_log) @@ -87,7 +93,7 @@ class Dnsd: self._process.terminate() self._process.wait(timeout=2) self._process = None - return not wait_dead or True + self.close_log() return True def restart(self): @@ -122,8 +128,8 @@ class Dnsd: '--logfile', f'{self._log_file}', '--pidfile', f'{self._pid_file}', ] - procerr = open(self._error_log, 'a') - self._process = subprocess.Popen(args=args, stderr=procerr) + self._error_fd = open(self._error_log, 'a') + self._process = subprocess.Popen(args=args, stderr=self._error_fd) if self._process.returncode is not None: return False return self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT)) diff --git a/tests/http/testenv/h2o.py b/tests/http/testenv/h2o.py index 279cff8b90..deb3608d3a 100644 --- a/tests/http/testenv/h2o.py +++ b/tests/http/testenv/h2o.py @@ -48,6 +48,7 @@ class H2o: self._port = 0 # defaults to h3_port self._cred_name = cred_name self._loaded_cred_name = None + self._error_fd = None self._process = None self._tmp_dir = os.path.join(self.env.gen_dir, self._name) self._run_dir = os.path.join(self._tmp_dir, "run") @@ -72,6 +73,11 @@ class H2o: def h2_port(self) -> Optional[int]: return getattr(self, "_h2_port", None) + def close_log(self): + if self._error_fd: + self._error_fd.close() + self._error_fd = None + def clear_logs(self): self._rmf(self._error_log) self._rmf(self._stderr) @@ -127,8 +133,8 @@ class H2o: self._loaded_cred_name = self._cred_name self.write_config() args = [self._cmd, "-c", self._conf_file] - ngerr = open(self._stderr, "a") - self._process = subprocess.Popen(args=args, stderr=ngerr) + self._error_fd = open(self._stderr, "a") + self._process = subprocess.Popen(args=args, stderr=self._error_fd) if self._process.returncode is not None: return False if wait_live: @@ -155,15 +161,19 @@ class H2o: self._process.kill() self._process.wait(timeout=2) self._process = None + self.close_log() return not wait_dead or self.wait_for_state( live=False, timeout=timedelta(seconds=5) ) + self.close_log() return True def kill(self, wait_dead=True): if self._process: self._process.kill() + self.close_log() return True + self.close_log() return False def restart(self): diff --git a/tests/http/testenv/nghttpx.py b/tests/http/testenv/nghttpx.py index c72a7f7f6d..37c72ad945 100644 --- a/tests/http/testenv/nghttpx.py +++ b/tests/http/testenv/nghttpx.py @@ -55,6 +55,7 @@ class Nghttpx: self._error_log = os.path.join(self._run_dir, 'nghttpx.log') self._stderr = os.path.join(self._run_dir, 'nghttpx.stderr') self._tmp_dir = os.path.join(self._run_dir, 'tmp') + self._error_fd = None self._process: Optional[subprocess.Popen] = None self._cred_name = self._def_cred_name = cred_name self._loaded_cred_name = '' @@ -86,6 +87,11 @@ class Nghttpx: def exists(self): return self._cmd and os.path.exists(self._cmd) + def close_log(self): + if self._error_fd: + self._error_fd.close() + self._error_fd = None + def clear_logs(self): self._rmf(self._error_log) self._rmf(self._stderr) @@ -116,7 +122,9 @@ class Nghttpx: self._process.terminate() self._process.wait(timeout=2) self._process = None + self.close_log() return not wait_dead or self.wait_dead(timeout=timedelta(seconds=5)) + self.close_log() return True def restart(self): @@ -262,8 +270,8 @@ class NghttpxQuic(Nghttpx): '--frontend-http3-max-connection-window-size=100M', # f'--frontend-quic-debug-log', ]) - ngerr = open(self._stderr, 'a') - self._process = subprocess.Popen(args=args, stderr=ngerr) + self._error_fd = open(self._stderr, 'a') + self._process = subprocess.Popen(args=args, stderr=self._error_fd) if self._process.returncode is not None: return False return not wait_live or self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT)) @@ -312,8 +320,8 @@ class NghttpxFwd(Nghttpx): creds.pkey_file, creds.cert_file, ] - ngerr = open(self._stderr, 'a') - self._process = subprocess.Popen(args=args, stderr=ngerr) + self._error_fd = open(self._stderr, 'a') + self._process = subprocess.Popen(args=args, stderr=self._error_fd) if self._process.returncode is not None: return False return not wait_live or self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT)) diff --git a/tests/http/testenv/sshd.py b/tests/http/testenv/sshd.py index 83accd7492..de8078dd26 100644 --- a/tests/http/testenv/sshd.py +++ b/tests/http/testenv/sshd.py @@ -74,6 +74,7 @@ class Sshd: ] self._user_key_files = [] self._user_pub_files = [] + self._error_fd = None self._process = None self.clear_logs() @@ -164,14 +165,21 @@ class Sshd: pubkey = fp.read() fd.write(pubkey) + def close_log(self): + if self._error_fd: + self._error_fd.close() + self._error_fd = None + def clear_logs(self): self._rmf(self._sshd_log) def dump_log(self): lines = ['>>--sshd log ----------------------------------------------\n'] - lines.extend(open(self._sshd_log)) + with open(self._sshd_log) as fd: + lines.extend(fd.readlines()) lines.extend(['>>--curl log ----------------------------------------------\n']) - lines.extend(open(os.path.join(self._tmp_dir, 'curl.stderr'))) + with open(os.path.join(self._tmp_dir, 'curl.stderr')) as fd: + lines.extend(fd.readlines()) lines.append('<<-------------------------------------------------------\n') return ''.join(lines) @@ -195,7 +203,7 @@ class Sshd: self._process.terminate() self._process.wait(timeout=2) self._process = None - return not wait_dead or True + self.close_log() return True def restart(self): @@ -233,8 +241,8 @@ class Sshd: run_env = os.environ.copy() # does not have any effect, sadly # run_env['HOME'] = f'{self._home_dir}' - procerr = open(self._sshd_log, 'a') - self._process = subprocess.Popen(args=args, stderr=procerr, env=run_env) + self._error_fd = open(self._sshd_log, 'a') + self._process = subprocess.Popen(args=args, stderr=self._error_fd, env=run_env) if self._process.returncode is not None: return False return self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT)) diff --git a/tests/http/testenv/vsftpd.py b/tests/http/testenv/vsftpd.py index ace2884c7d..6c2d1db8c5 100644 --- a/tests/http/testenv/vsftpd.py +++ b/tests/http/testenv/vsftpd.py @@ -68,6 +68,7 @@ class VsFTPD: self._conf_file = os.path.join(self._vsftpd_dir, 'test.conf') self._pid_file = os.path.join(self._vsftpd_dir, 'vsftpd.pid') self._error_log = os.path.join(self._vsftpd_dir, 'vsftpd.log') + self._error_fd = None self._process = None self.clear_logs() @@ -84,6 +85,11 @@ class VsFTPD: def port(self) -> int: return self._port + def close_log(self): + if self._error_fd: + self._error_fd.close() + self._error_fd = None + def clear_logs(self): self._rmf(self._error_log) @@ -107,7 +113,9 @@ class VsFTPD: self._process.terminate() self._process.wait(timeout=2) self._process = None + self.close_log() return not wait_dead or self.wait_dead(timeout=timedelta(seconds=5)) + self.close_log() return True def restart(self): @@ -138,8 +146,8 @@ class VsFTPD: self._cmd, f'{self._conf_file}', ] - procerr = open(self._error_log, 'a') - self._process = subprocess.Popen(args=args, stderr=procerr) + self._error_fd = open(self._error_log, 'a') + self._process = subprocess.Popen(args=args, stderr=self._error_fd) if self._process.returncode is not None: return False return not wait_live or self.wait_live(timeout=timedelta(seconds=Env.SERVER_TIMEOUT))