From c508119f62da6e484cee316a069e99669dd66c4e Mon Sep 17 00:00:00 2001 From: maipbui Date: Tue, 7 Mar 2023 19:09:33 +0000 Subject: [PATCH 1/2] replace shell=True Signed-off-by: maipbui --- consutil/lib.py | 27 +++++++++++++++++++-------- tests/console_test.py | 4 ++-- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/consutil/lib.py b/consutil/lib.py index e2ebf1da06..f90d8d3e59 100644 --- a/consutil/lib.py +++ b/consutil/lib.py @@ -12,6 +12,7 @@ import click from sonic_py_common import device_info +from sonic_py_common.general import getstatusoutput_noshell_pipe ERR_DISABLE = 1 ERR_CMD = 2 @@ -199,7 +200,7 @@ def clear_session(self): try: if not self._session: pid = self.session_pid - cmd = "sudo kill -SIGTERM " + pid + cmd = ['sudo', 'kill', '-SIGTERM', str(pid)] SysInfoProvider.run_command(cmd) else: self._session.close() @@ -276,7 +277,7 @@ def init_device_prefix(): @staticmethod def list_console_ttys(): """Lists all console tty devices""" - cmd = "ls " + SysInfoProvider.DEVICE_PREFIX + "*" + cmd = ["ls", SysInfoProvider.DEVICE_PREFIX, "*"] output, _ = SysInfoProvider.run_command(cmd, abort=False) ttys = output.split('\n') ttys = list([dev for dev in ttys if re.match(SysInfoProvider.DEVICE_PREFIX + r"\d+", dev) != None]) @@ -285,15 +286,17 @@ def list_console_ttys(): @staticmethod def list_active_console_processes(): """Lists all active console session processes""" - cmd = 'ps -eo pid,lstart,cmd | grep -E "(mini|pico)com"' - output = SysInfoProvider.run_command(cmd) + cmd0 = ['ps', '-eo', 'pid,lstart,cmd'] + cmd1 = ['grep', '-E', "(mini|pico)com"] + output = SysInfoProvider.run_command_pipe(cmd0, cmd1) return SysInfoProvider._parse_processes_info(output) @staticmethod def get_active_console_process_info(pid): """Gets active console process information by PID""" - cmd = 'ps -p {} -o pid,lstart,cmd | grep -E "(mini|pico)com"'.format(pid) - output = SysInfoProvider.run_command(cmd) + cmd0 = ['ps', '-p', str(pid), '-o', 'pid,lstart,cmd'] + cmd1 = ['grep', '-E', "(mini|pico)com"] + output = SysInfoProvider.run_command_pipe(cmd0, cmd1) processes = SysInfoProvider._parse_processes_info(output) if len(list(processes.keys())) == 1: return (list(processes.keys())[0],) + list(processes.values())[0] @@ -326,8 +329,8 @@ def _parse_processes_info(output): @staticmethod def run_command(cmd, abort=True): - """runs command, exit if stderr is written to and abort argument is ture, returns stdout, stderr otherwise""" - proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True, text=True) + """runs command, exit if stderr is written to and abort argument is true, returns stdout, stderr otherwise""" + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) output = proc.stdout.read() error = proc.stderr.read() if abort and error != "": @@ -335,6 +338,14 @@ def run_command(cmd, abort=True): sys.exit(ERR_CMD) return output if abort else (output, error) + @staticmethod + def run_command_pipe(*args, abort=True): + exitcodes, output = getstatusoutput_noshell_pipe(*args) + if abort and any(exitcodes) and output != '': + click.echo("Command resulted in error: {}".format(output)) + sys.exit(ERR_CMD) + return output if abort else (output, output) + class DbUtils(object): def __init__(self, db): self._db = db diff --git a/tests/console_test.py b/tests/console_test.py index 8161eda7dd..1c4d33b146 100644 --- a/tests/console_test.py +++ b/tests/console_test.py @@ -460,7 +460,7 @@ def test_sys_info_provider_list_console_ttys_device_not_exists(self): """ PID STARTED CMD 8 Mon Nov 2 04:29:41 2020 picocom /dev/ttyUSB0 """ - @mock.patch('consutil.lib.SysInfoProvider.run_command', mock.MagicMock(return_value=all_active_processes_output)) + @mock.patch('consutil.lib.SysInfoProvider.run_command_pipe', mock.MagicMock(return_value=all_active_processes_output)) def test_sys_info_provider_list_active_console_processes(self): SysInfoProvider.DEVICE_PREFIX == "/dev/ttyUSB" procs = SysInfoProvider.list_active_console_processes() @@ -469,7 +469,7 @@ def test_sys_info_provider_list_active_console_processes(self): assert procs["0"] == ("8", "Mon Nov 2 04:29:41 2020") active_process_output = "13751 Wed Mar 6 08:31:35 2019 /usr/bin/sudo picocom -b 9600 -f n /dev/ttyUSB1" - @mock.patch('consutil.lib.SysInfoProvider.run_command', mock.MagicMock(return_value=active_process_output)) + @mock.patch('consutil.lib.SysInfoProvider.run_command_pipe', mock.MagicMock(return_value=active_process_output)) def test_sys_info_provider_get_active_console_process_info_exists(self): SysInfoProvider.DEVICE_PREFIX == "/dev/ttyUSB" proc = SysInfoProvider.get_active_console_process_info("13751") From 33645efe5d62e6c868432914425b181790a0c2c8 Mon Sep 17 00:00:00 2001 From: maipbui Date: Wed, 8 Mar 2023 16:57:22 +0000 Subject: [PATCH 2/2] address PR comments Signed-off-by: maipbui --- consutil/lib.py | 19 ++++--------------- tests/console_test.py | 4 ++-- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/consutil/lib.py b/consutil/lib.py index f90d8d3e59..1d7f967bd3 100644 --- a/consutil/lib.py +++ b/consutil/lib.py @@ -277,7 +277,7 @@ def init_device_prefix(): @staticmethod def list_console_ttys(): """Lists all console tty devices""" - cmd = ["ls", SysInfoProvider.DEVICE_PREFIX, "*"] + cmd = ["ls", SysInfoProvider.DEVICE_PREFIX + "*"] output, _ = SysInfoProvider.run_command(cmd, abort=False) ttys = output.split('\n') ttys = list([dev for dev in ttys if re.match(SysInfoProvider.DEVICE_PREFIX + r"\d+", dev) != None]) @@ -288,7 +288,7 @@ def list_active_console_processes(): """Lists all active console session processes""" cmd0 = ['ps', '-eo', 'pid,lstart,cmd'] cmd1 = ['grep', '-E', "(mini|pico)com"] - output = SysInfoProvider.run_command_pipe(cmd0, cmd1) + output = SysInfoProvider.run_command(cmd0, cmd1) return SysInfoProvider._parse_processes_info(output) @staticmethod @@ -296,7 +296,7 @@ def get_active_console_process_info(pid): """Gets active console process information by PID""" cmd0 = ['ps', '-p', str(pid), '-o', 'pid,lstart,cmd'] cmd1 = ['grep', '-E', "(mini|pico)com"] - output = SysInfoProvider.run_command_pipe(cmd0, cmd1) + output = SysInfoProvider.run_command(cmd0, cmd1) processes = SysInfoProvider._parse_processes_info(output) if len(list(processes.keys())) == 1: return (list(processes.keys())[0],) + list(processes.values())[0] @@ -328,18 +328,7 @@ def _parse_processes_info(output): return console_processes @staticmethod - def run_command(cmd, abort=True): - """runs command, exit if stderr is written to and abort argument is true, returns stdout, stderr otherwise""" - proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) - output = proc.stdout.read() - error = proc.stderr.read() - if abort and error != "": - click.echo("Command resulted in error: {}".format(error)) - sys.exit(ERR_CMD) - return output if abort else (output, error) - - @staticmethod - def run_command_pipe(*args, abort=True): + def run_command(*args, abort=True): exitcodes, output = getstatusoutput_noshell_pipe(*args) if abort and any(exitcodes) and output != '': click.echo("Command resulted in error: {}".format(output)) diff --git a/tests/console_test.py b/tests/console_test.py index 1c4d33b146..8161eda7dd 100644 --- a/tests/console_test.py +++ b/tests/console_test.py @@ -460,7 +460,7 @@ def test_sys_info_provider_list_console_ttys_device_not_exists(self): """ PID STARTED CMD 8 Mon Nov 2 04:29:41 2020 picocom /dev/ttyUSB0 """ - @mock.patch('consutil.lib.SysInfoProvider.run_command_pipe', mock.MagicMock(return_value=all_active_processes_output)) + @mock.patch('consutil.lib.SysInfoProvider.run_command', mock.MagicMock(return_value=all_active_processes_output)) def test_sys_info_provider_list_active_console_processes(self): SysInfoProvider.DEVICE_PREFIX == "/dev/ttyUSB" procs = SysInfoProvider.list_active_console_processes() @@ -469,7 +469,7 @@ def test_sys_info_provider_list_active_console_processes(self): assert procs["0"] == ("8", "Mon Nov 2 04:29:41 2020") active_process_output = "13751 Wed Mar 6 08:31:35 2019 /usr/bin/sudo picocom -b 9600 -f n /dev/ttyUSB1" - @mock.patch('consutil.lib.SysInfoProvider.run_command_pipe', mock.MagicMock(return_value=active_process_output)) + @mock.patch('consutil.lib.SysInfoProvider.run_command', mock.MagicMock(return_value=active_process_output)) def test_sys_info_provider_get_active_console_process_info_exists(self): SysInfoProvider.DEVICE_PREFIX == "/dev/ttyUSB" proc = SysInfoProvider.get_active_console_process_info("13751")