From 6a6dd70c8261fe27fb39180e257a2586df2b2b5c Mon Sep 17 00:00:00 2001 From: maipbui Date: Tue, 13 Sep 2022 22:51:57 +0000 Subject: [PATCH 01/10] Add getstatusoutput_noshell functions Signed-off-by: maipbui --- .../sonic_py_common/general.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/sonic-py-common/sonic_py_common/general.py b/src/sonic-py-common/sonic_py_common/general.py index 9e04f3e214ee..aeadda1a5b7f 100644 --- a/src/sonic-py-common/sonic_py_common/general.py +++ b/src/sonic-py-common/sonic_py_common/general.py @@ -1,4 +1,5 @@ import sys +import subprocess def load_module_from_source(module_name, file_path): @@ -23,3 +24,33 @@ def load_module_from_source(module_name, file_path): sys.modules[module_name] = module return module + + +def getstatusoutput_noshell(cmd): + """ + This function implements getstatusoutput API from subprocess module + but using shell=False to prevent shell injection. + """ + p = subprocess.run(cmd, capture_output=True, universal_newlines=True) + status, output = p.returncode, p.stdout + if output[-1:] == '\n': + output = output[:-1] + return (status, output) + + +def getstatusoutput_noshell_pipe(cmd1, cmd2): + """ + This function implements getstatusoutput API from subprocess module + but using shell=False to prevent shell injection. Input command includes + pipe command. + """ + status = 0 + with subprocess.Popen(cmd1, universal_newlines=True, stdout=subprocess.PIPE) as p1: + with subprocess.Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=subprocess.PIPE) as p2: + output = p2.communicate()[0] + p1.wait() + if p1.returncode != 0 and p2.returncode != 0: + status = 1 + if output[-1:] == '\n': + output = output[:-1] + return (status, output) From f7d90d0e6fa15e596ec50c50ab7782cdd41e78a4 Mon Sep 17 00:00:00 2001 From: maipbui Date: Thu, 15 Sep 2022 19:10:28 +0000 Subject: [PATCH 02/10] Fix getstatusoutput and add check_output_pipe Signed-off-by: maipbui --- .../sonic_py_common/general.py | 49 ++++++++++++++----- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/src/sonic-py-common/sonic_py_common/general.py b/src/sonic-py-common/sonic_py_common/general.py index aeadda1a5b7f..21550d06beb9 100644 --- a/src/sonic-py-common/sonic_py_common/general.py +++ b/src/sonic-py-common/sonic_py_common/general.py @@ -1,5 +1,5 @@ import sys -import subprocess +from subprocess import Popen, STDOUT, PIPE, CalledProcessError, check_output def load_module_from_source(module_name, file_path): @@ -31,11 +31,15 @@ def getstatusoutput_noshell(cmd): This function implements getstatusoutput API from subprocess module but using shell=False to prevent shell injection. """ - p = subprocess.run(cmd, capture_output=True, universal_newlines=True) - status, output = p.returncode, p.stdout + try: + output = check_output(cmd, text=True, stderr=STDOUT) + exitcode = 0 + except CalledProcessError as ex: + output = ex.output + exitcode = ex.returncode if output[-1:] == '\n': output = output[:-1] - return (status, output) + return exitcode, output def getstatusoutput_noshell_pipe(cmd1, cmd2): @@ -44,13 +48,36 @@ def getstatusoutput_noshell_pipe(cmd1, cmd2): but using shell=False to prevent shell injection. Input command includes pipe command. """ - status = 0 - with subprocess.Popen(cmd1, universal_newlines=True, stdout=subprocess.PIPE) as p1: - with subprocess.Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=subprocess.PIPE) as p2: + cmd = ' '.join(cmd1) + ' | ' + ' '.join(cmd2) + try: + with Popen(cmd1, universal_newlines=True, stdout=PIPE) as p1: + with Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=PIPE) as p2: + output = p2.communicate()[0] + p1.wait() + if p1.returncode != 0 and p2.returncode != 0: + returncode = p1.returncode if p2.returncode == 0 else p2.returncode + raise CalledProcessError(returncode=returncode, cmd=cmd, output=output) + exitcode = 0 + except CalledProcessError as e: + output = e.output + exitcode = e.returncode + if output[-1:] == '\n': + output = output[:-1] + return exitcode, output + + +def check_output_pipe(cmd1, cmd2): + """ + This function implements check_output API from subprocess module. + Input command includes pipe command. + """ + cmd = ' '.join(cmd1) + ' | ' + ' '.join(cmd2) + with Popen(cmd1, universal_newlines=True, stdout=PIPE) as p1: + with Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=PIPE) as p2: output = p2.communicate()[0] p1.wait() if p1.returncode != 0 and p2.returncode != 0: - status = 1 - if output[-1:] == '\n': - output = output[:-1] - return (status, output) + returncode = p1.returncode if p2.returncode == 0 else p2.returncode + raise CalledProcessError(returncode=returncode, cmd=cmd, output=output) + return output + From 0e8f4ceadd7bda1eefbd588dfc77fa0316edb67f Mon Sep 17 00:00:00 2001 From: maipbui Date: Thu, 15 Sep 2022 23:58:54 +0000 Subject: [PATCH 03/10] Fix PR commends and add UT Signed-off-by: maipbui --- .../sonic_py_common/general.py | 29 +++++++--------- src/sonic-py-common/tests/test_general.py | 33 +++++++++++++++++++ 2 files changed, 45 insertions(+), 17 deletions(-) create mode 100644 src/sonic-py-common/tests/test_general.py diff --git a/src/sonic-py-common/sonic_py_common/general.py b/src/sonic-py-common/sonic_py_common/general.py index 21550d06beb9..097b9b417fdb 100644 --- a/src/sonic-py-common/sonic_py_common/general.py +++ b/src/sonic-py-common/sonic_py_common/general.py @@ -48,22 +48,15 @@ def getstatusoutput_noshell_pipe(cmd1, cmd2): but using shell=False to prevent shell injection. Input command includes pipe command. """ - cmd = ' '.join(cmd1) + ' | ' + ' '.join(cmd2) - try: - with Popen(cmd1, universal_newlines=True, stdout=PIPE) as p1: - with Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=PIPE) as p2: - output = p2.communicate()[0] - p1.wait() - if p1.returncode != 0 and p2.returncode != 0: - returncode = p1.returncode if p2.returncode == 0 else p2.returncode - raise CalledProcessError(returncode=returncode, cmd=cmd, output=output) - exitcode = 0 - except CalledProcessError as e: - output = e.output - exitcode = e.returncode + with Popen(cmd1, universal_newlines=True, stdout=PIPE) as p1: + with Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=PIPE) as p2: + output = p2.communicate()[0] + p1.wait() + if p1.returncode != 0 or p2.returncode != 0: + return ([p1.returncode, p2.returncode], output) if output[-1:] == '\n': output = output[:-1] - return exitcode, output + return ([0, 0], output) def check_output_pipe(cmd1, cmd2): @@ -76,8 +69,10 @@ def check_output_pipe(cmd1, cmd2): with Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=PIPE) as p2: output = p2.communicate()[0] p1.wait() - if p1.returncode != 0 and p2.returncode != 0: - returncode = p1.returncode if p2.returncode == 0 else p2.returncode - raise CalledProcessError(returncode=returncode, cmd=cmd, output=output) + if p1.returncode != 0 or p2.returncode != 0: + if p1.returncode != 0: + raise CalledProcessError(returncode=p1.returncode, cmd=cmd, output=output) + else: + raise CalledProcessError(returncode=p2.returncode, cmd=cmd, output=output) return output diff --git a/src/sonic-py-common/tests/test_general.py b/src/sonic-py-common/tests/test_general.py new file mode 100644 index 000000000000..779125285670 --- /dev/null +++ b/src/sonic-py-common/tests/test_general.py @@ -0,0 +1,33 @@ +import os +import sys +import pytest +import subprocess +from sonic_py_common.general import getstatusoutput_noshell, getstatusoutput_noshell_pipe, check_output_pipe + + +def test_getstatusoutput_noshell(tmpdir): + exitcode, output = getstatusoutput_noshell(['echo', 'sonic']) + assert (exitcode, output) == (0, 'sonic') + + name = os.path.join(tmpdir, 'foo') + exitcode, output = getstatusoutput_noshell(['cat', name]) + assert exitcode != 0 + +def test_getstatusoutput_noshell_pipe(): + exitcode, output = getstatusoutput_noshell_pipe(['echo', 'sonic'], ['awk', '{print $1}']) + assert (exitcode, output) == ([0, 0], 'sonic') + + exitcode, output = getstatusoutput_noshell_pipe([sys.executable, "-c", "import sys; sys.exit(6)"], [sys.executable, "-c", "import sys; sys.exit(8)"]) + assert exitcode == [6, 8] + +def test_check_output_pipe(): + output = check_output_pipe(['echo', 'sonic'], ['awk', '{print $1}']) + assert output == 'sonic\n' + + with pytest.raises(subprocess.CalledProcessError) as e: + check_output_pipe([sys.executable, "-c", "import sys; sys.exit(6)"], [sys.executable, "-c", "import sys; sys.exit(0)"]) + assert e.returncode == [6, 0] + + with pytest.raises(subprocess.CalledProcessError) as e: + check_output_pipe([sys.executable, "-c", "import sys; sys.exit(0)"], [sys.executable, "-c", "import sys; sys.exit(6)"]) + assert e.returncode == [0, 6] From 8e3b376756d620221ed5edd11e1c7f431e6120cf Mon Sep 17 00:00:00 2001 From: maipbui Date: Fri, 16 Sep 2022 00:00:55 +0000 Subject: [PATCH 04/10] Add ref Signed-off-by: maipbui --- src/sonic-py-common/sonic_py_common/general.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sonic-py-common/sonic_py_common/general.py b/src/sonic-py-common/sonic_py_common/general.py index 097b9b417fdb..de9b16780045 100644 --- a/src/sonic-py-common/sonic_py_common/general.py +++ b/src/sonic-py-common/sonic_py_common/general.py @@ -30,6 +30,7 @@ def getstatusoutput_noshell(cmd): """ This function implements getstatusoutput API from subprocess module but using shell=False to prevent shell injection. + Ref: https://github.com/python/cpython/blob/3.10/Lib/subprocess.py#L602 """ try: output = check_output(cmd, text=True, stderr=STDOUT) From 5b256dfefc14cbe40b130837d234e3f035ac1d77 Mon Sep 17 00:00:00 2001 From: maipbui Date: Fri, 16 Sep 2022 03:28:44 +0000 Subject: [PATCH 05/10] Modify code for python 2 Signed-off-by: maipbui --- .../sonic_py_common/general.py | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/sonic-py-common/sonic_py_common/general.py b/src/sonic-py-common/sonic_py_common/general.py index de9b16780045..83e494afdfbf 100644 --- a/src/sonic-py-common/sonic_py_common/general.py +++ b/src/sonic-py-common/sonic_py_common/general.py @@ -33,7 +33,7 @@ def getstatusoutput_noshell(cmd): Ref: https://github.com/python/cpython/blob/3.10/Lib/subprocess.py#L602 """ try: - output = check_output(cmd, text=True, stderr=STDOUT) + output = check_output(cmd, universal_newlines=True, stderr=STDOUT) exitcode = 0 except CalledProcessError as ex: output = ex.output @@ -49,15 +49,14 @@ def getstatusoutput_noshell_pipe(cmd1, cmd2): but using shell=False to prevent shell injection. Input command includes pipe command. """ - with Popen(cmd1, universal_newlines=True, stdout=PIPE) as p1: - with Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=PIPE) as p2: - output = p2.communicate()[0] - p1.wait() - if p1.returncode != 0 or p2.returncode != 0: - return ([p1.returncode, p2.returncode], output) + p1 = Popen(cmd1, universal_newlines=True, stdout=PIPE) + p2 = Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=PIPE) + p1.stdout.close() + output = p2.communicate()[0] + p1.wait() if output[-1:] == '\n': output = output[:-1] - return ([0, 0], output) + return ([p1.returncode, p2.returncode], output) def check_output_pipe(cmd1, cmd2): @@ -66,14 +65,15 @@ def check_output_pipe(cmd1, cmd2): Input command includes pipe command. """ cmd = ' '.join(cmd1) + ' | ' + ' '.join(cmd2) - with Popen(cmd1, universal_newlines=True, stdout=PIPE) as p1: - with Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=PIPE) as p2: - output = p2.communicate()[0] - p1.wait() - if p1.returncode != 0 or p2.returncode != 0: - if p1.returncode != 0: - raise CalledProcessError(returncode=p1.returncode, cmd=cmd, output=output) - else: - raise CalledProcessError(returncode=p2.returncode, cmd=cmd, output=output) + p1 = Popen(cmd1, universal_newlines=True, stdout=PIPE) + p2 = Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=PIPE) + p1.stdout.close() + output = p2.communicate()[0] + p1.wait() + if p1.returncode != 0 or p2.returncode != 0: + if p1.returncode != 0: + raise CalledProcessError(returncode=p1.returncode, cmd=cmd1, output=output) + else: + raise CalledProcessError(returncode=p2.returncode, cmd=cmd, output=output) return output From bfd38624de5909e9b43bcd976d963ac53d5f25c3 Mon Sep 17 00:00:00 2001 From: maipbui Date: Fri, 16 Sep 2022 15:17:28 +0000 Subject: [PATCH 06/10] Modify UT to pass py2 Signed-off-by: maipbui --- src/sonic-py-common/tests/test_general.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/sonic-py-common/tests/test_general.py b/src/sonic-py-common/tests/test_general.py index 779125285670..a395cf9aeb6b 100644 --- a/src/sonic-py-common/tests/test_general.py +++ b/src/sonic-py-common/tests/test_general.py @@ -1,16 +1,14 @@ -import os import sys import pytest import subprocess from sonic_py_common.general import getstatusoutput_noshell, getstatusoutput_noshell_pipe, check_output_pipe -def test_getstatusoutput_noshell(tmpdir): +def test_getstatusoutput_noshell(tmp_path): exitcode, output = getstatusoutput_noshell(['echo', 'sonic']) assert (exitcode, output) == (0, 'sonic') - name = os.path.join(tmpdir, 'foo') - exitcode, output = getstatusoutput_noshell(['cat', name]) + exitcode, output = getstatusoutput_noshell([sys.executable, "-c", "import sys; sys.exit(6)"]) assert exitcode != 0 def test_getstatusoutput_noshell_pipe(): From 38ca2583a4ff9de517ef5939c13337db71b1cdf2 Mon Sep 17 00:00:00 2001 From: maipbui Date: Fri, 16 Sep 2022 15:55:04 +0000 Subject: [PATCH 07/10] Return p1.stdout in exception 1 Signed-off-by: maipbui --- src/sonic-py-common/sonic_py_common/general.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sonic-py-common/sonic_py_common/general.py b/src/sonic-py-common/sonic_py_common/general.py index 83e494afdfbf..8e9482f9c24b 100644 --- a/src/sonic-py-common/sonic_py_common/general.py +++ b/src/sonic-py-common/sonic_py_common/general.py @@ -72,7 +72,7 @@ def check_output_pipe(cmd1, cmd2): p1.wait() if p1.returncode != 0 or p2.returncode != 0: if p1.returncode != 0: - raise CalledProcessError(returncode=p1.returncode, cmd=cmd1, output=output) + raise CalledProcessError(returncode=p1.returncode, cmd=cmd1, output=p1.stdout) else: raise CalledProcessError(returncode=p2.returncode, cmd=cmd, output=output) return output From fbe43bff2a91d2ae5da811a06a47c555c2e5826b Mon Sep 17 00:00:00 2001 From: maipbui Date: Sat, 17 Sep 2022 21:27:45 +0000 Subject: [PATCH 08/10] Extend 2 functions to handle more pipe commands Signed-off-by: maipbui --- .../sonic_py_common/general.py | 78 +++++++++++++------ src/sonic-py-common/tests/test_general.py | 16 ++-- 2 files changed, 64 insertions(+), 30 deletions(-) diff --git a/src/sonic-py-common/sonic_py_common/general.py b/src/sonic-py-common/sonic_py_common/general.py index 8e9482f9c24b..98529f6ef55e 100644 --- a/src/sonic-py-common/sonic_py_common/general.py +++ b/src/sonic-py-common/sonic_py_common/general.py @@ -43,37 +43,71 @@ def getstatusoutput_noshell(cmd): return exitcode, output -def getstatusoutput_noshell_pipe(cmd1, cmd2): +def getstatusoutput_noshell_pipes(*args): """ This function implements getstatusoutput API from subprocess module - but using shell=False to prevent shell injection. Input command includes - pipe command. + but using shell=False to prevent shell injection. Input command + includes two or more pipe commands. """ - p1 = Popen(cmd1, universal_newlines=True, stdout=PIPE) - p2 = Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=PIPE) - p1.stdout.close() - output = p2.communicate()[0] - p1.wait() + if len(args) < 2: + raise ValueError("Need at least 2 processes") + # Set up more arguments in every processes + for arg in args: + arg["stdout"] = PIPE + arg["universal_newlines"] = True + arg["shell"] = False + + # Runs all subprocesses connecting stdins and previous arguments + # to create the pipeline. Closes stdouts to avoid deadlocks. + # Ref: https://docs.python.org/2/library/subprocess.html#replacing-shell-pipeline + popens = [Popen(**args[0])] + for i in range(1,len(args)): + args[i]["stdin"] = popens[i-1].stdout + popens.append(Popen(**args[i])) + popens[i-1].stdout.close() + output = popens[-1].communicate()[0] if output[-1:] == '\n': output = output[:-1] - return ([p1.returncode, p2.returncode], output) + + # Wait for the processes to terminate and return the exitcodes + exitcodes = [0] * len(popens) + while popens: + last = popens.pop(-1) + exitcodes[len(popens)] = last.wait() + return (exitcodes, output) -def check_output_pipe(cmd1, cmd2): +def check_output_pipes(*args): """ This function implements check_output API from subprocess module. - Input command includes pipe command. + Input command includes two or more pipe command. """ - cmd = ' '.join(cmd1) + ' | ' + ' '.join(cmd2) - p1 = Popen(cmd1, universal_newlines=True, stdout=PIPE) - p2 = Popen(cmd2, universal_newlines=True, stdin=p1.stdout, stdout=PIPE) - p1.stdout.close() - output = p2.communicate()[0] - p1.wait() - if p1.returncode != 0 or p2.returncode != 0: - if p1.returncode != 0: - raise CalledProcessError(returncode=p1.returncode, cmd=cmd1, output=p1.stdout) - else: - raise CalledProcessError(returncode=p2.returncode, cmd=cmd, output=output) + if len(args) < 2: + raise ValueError("Needs at least 2 processes") + # Set up more arguments in every processes + for arg in args: + arg["stdout"] = PIPE + arg["universal_newlines"] = True + arg["shell"] = False + + # Runs all subprocesses connecting stdins and previous arguments + # to create the pipeline. Closes stdouts to avoid deadlocks. + # Ref: https://docs.python.org/2/library/subprocess.html#replacing-shell-pipeline + popens = [Popen(**args[0])] + for i in range(1,len(args)): + args[i]["stdin"] = popens[i-1].stdout + popens.append(Popen(**args[i])) + popens[i-1].stdout.close() + output = popens[-1].communicate()[0] + + # Wait for the processes to terminate and raise an exeption if exitcode is non zero + i = 0 + while popens: + current = popens.pop(0) + exitcode = current.wait() + if exitcode != 0: + raise CalledProcessError(returncode=exitcode, cmd=args[i], output=current.stdout) + i += 1 + return output diff --git a/src/sonic-py-common/tests/test_general.py b/src/sonic-py-common/tests/test_general.py index a395cf9aeb6b..c09513c72d61 100644 --- a/src/sonic-py-common/tests/test_general.py +++ b/src/sonic-py-common/tests/test_general.py @@ -1,7 +1,7 @@ import sys import pytest import subprocess -from sonic_py_common.general import getstatusoutput_noshell, getstatusoutput_noshell_pipe, check_output_pipe +from sonic_py_common.general import getstatusoutput_noshell, getstatusoutput_noshell_pipes, check_output_pipes def test_getstatusoutput_noshell(tmp_path): @@ -11,21 +11,21 @@ def test_getstatusoutput_noshell(tmp_path): exitcode, output = getstatusoutput_noshell([sys.executable, "-c", "import sys; sys.exit(6)"]) assert exitcode != 0 -def test_getstatusoutput_noshell_pipe(): - exitcode, output = getstatusoutput_noshell_pipe(['echo', 'sonic'], ['awk', '{print $1}']) +def test_getstatusoutput_noshell_pipes(): + exitcode, output = getstatusoutput_noshell_pipes(dict(args=['echo', 'sonic']), dict(args=['awk', '{print $1}'])) assert (exitcode, output) == ([0, 0], 'sonic') - exitcode, output = getstatusoutput_noshell_pipe([sys.executable, "-c", "import sys; sys.exit(6)"], [sys.executable, "-c", "import sys; sys.exit(8)"]) + exitcode, output = getstatusoutput_noshell_pipes(dict(args=[sys.executable, "-c", "import sys; sys.exit(6)"]), dict(args=[sys.executable, "-c", "import sys; sys.exit(8)"])) assert exitcode == [6, 8] -def test_check_output_pipe(): - output = check_output_pipe(['echo', 'sonic'], ['awk', '{print $1}']) +def test_check_output_pipes(): + output = check_output_pipes(dict(args=['echo', 'sonic']), dict(args=['awk', '{print $1}'])) assert output == 'sonic\n' with pytest.raises(subprocess.CalledProcessError) as e: - check_output_pipe([sys.executable, "-c", "import sys; sys.exit(6)"], [sys.executable, "-c", "import sys; sys.exit(0)"]) + check_output_pipes(dict(args=[sys.executable, "-c", "import sys; sys.exit(6)"]), dict(args=[sys.executable, "-c", "import sys; sys.exit(0)"])) assert e.returncode == [6, 0] with pytest.raises(subprocess.CalledProcessError) as e: - check_output_pipe([sys.executable, "-c", "import sys; sys.exit(0)"], [sys.executable, "-c", "import sys; sys.exit(6)"]) + check_output_pipes(dict(args=[sys.executable, "-c", "import sys; sys.exit(0)"]), dict(args=[sys.executable, "-c", "import sys; sys.exit(6)"])) assert e.returncode == [0, 6] From 99ccf28a1a4171e65fd3ab920b032ccacfed02b7 Mon Sep 17 00:00:00 2001 From: maipbui Date: Sun, 18 Sep 2022 01:04:17 +0000 Subject: [PATCH 09/10] Fix PR comments Signed-off-by: maipbui --- .../sonic_py_common/general.py | 58 ++++++------------- src/sonic-py-common/tests/test_general.py | 12 ++-- 2 files changed, 25 insertions(+), 45 deletions(-) diff --git a/src/sonic-py-common/sonic_py_common/general.py b/src/sonic-py-common/sonic_py_common/general.py index 98529f6ef55e..4fea165de9fb 100644 --- a/src/sonic-py-common/sonic_py_common/general.py +++ b/src/sonic-py-common/sonic_py_common/general.py @@ -43,70 +43,50 @@ def getstatusoutput_noshell(cmd): return exitcode, output -def getstatusoutput_noshell_pipes(*args): +def getstatusoutput_noshell_pipe(cmd0, *args): """ This function implements getstatusoutput API from subprocess module but using shell=False to prevent shell injection. Input command - includes two or more pipe commands. + includes two or more commands connected by shell pipe(s). """ - if len(args) < 2: - raise ValueError("Need at least 2 processes") - # Set up more arguments in every processes - for arg in args: - arg["stdout"] = PIPE - arg["universal_newlines"] = True - arg["shell"] = False - - # Runs all subprocesses connecting stdins and previous arguments - # to create the pipeline. Closes stdouts to avoid deadlocks. - # Ref: https://docs.python.org/2/library/subprocess.html#replacing-shell-pipeline - popens = [Popen(**args[0])] - for i in range(1,len(args)): - args[i]["stdin"] = popens[i-1].stdout - popens.append(Popen(**args[i])) - popens[i-1].stdout.close() + popens = [Popen(cmd0, stdout=PIPE, universal_newlines=True)] + i = 0 + while i < len(args): + popens.append(Popen(args[i], stdin=popens[i].stdout, stdout=PIPE, universal_newlines=True)) + popens[i].stdout.close() + i += 1 output = popens[-1].communicate()[0] if output[-1:] == '\n': output = output[:-1] - # Wait for the processes to terminate and return the exitcodes exitcodes = [0] * len(popens) while popens: last = popens.pop(-1) exitcodes[len(popens)] = last.wait() + return (exitcodes, output) -def check_output_pipes(*args): +def check_output_pipe(cmd0, *args): """ This function implements check_output API from subprocess module. - Input command includes two or more pipe command. + Input command includes two or more commands connected by shell pipe(s) """ - if len(args) < 2: - raise ValueError("Needs at least 2 processes") - # Set up more arguments in every processes - for arg in args: - arg["stdout"] = PIPE - arg["universal_newlines"] = True - arg["shell"] = False - - # Runs all subprocesses connecting stdins and previous arguments - # to create the pipeline. Closes stdouts to avoid deadlocks. - # Ref: https://docs.python.org/2/library/subprocess.html#replacing-shell-pipeline - popens = [Popen(**args[0])] - for i in range(1,len(args)): - args[i]["stdin"] = popens[i-1].stdout - popens.append(Popen(**args[i])) - popens[i-1].stdout.close() + popens = [Popen(cmd0, stdout=PIPE, universal_newlines=True)] + i = 0 + while i < len(args): + popens.append(Popen(args[i], stdin=popens[i].stdout, stdout=PIPE, universal_newlines=True)) + popens[i].stdout.close() + i += 1 output = popens[-1].communicate()[0] - # Wait for the processes to terminate and raise an exeption if exitcode is non zero i = 0 + args_list = [cmd0] + list(args) while popens: current = popens.pop(0) exitcode = current.wait() if exitcode != 0: - raise CalledProcessError(returncode=exitcode, cmd=args[i], output=current.stdout) + raise CalledProcessError(returncode=exitcode, cmd=args_list[i], output=current.stdout) i += 1 return output diff --git a/src/sonic-py-common/tests/test_general.py b/src/sonic-py-common/tests/test_general.py index c09513c72d61..27744d3e77c8 100644 --- a/src/sonic-py-common/tests/test_general.py +++ b/src/sonic-py-common/tests/test_general.py @@ -1,7 +1,7 @@ import sys import pytest import subprocess -from sonic_py_common.general import getstatusoutput_noshell, getstatusoutput_noshell_pipes, check_output_pipes +from sonic_py_common.general import getstatusoutput_noshell, getstatusoutput_noshell_pipe, check_output_pipe def test_getstatusoutput_noshell(tmp_path): @@ -12,20 +12,20 @@ def test_getstatusoutput_noshell(tmp_path): assert exitcode != 0 def test_getstatusoutput_noshell_pipes(): - exitcode, output = getstatusoutput_noshell_pipes(dict(args=['echo', 'sonic']), dict(args=['awk', '{print $1}'])) + exitcode, output = getstatusoutput_noshell_pipe(['echo', 'sonic'], ['awk', '{print $1}']) assert (exitcode, output) == ([0, 0], 'sonic') - exitcode, output = getstatusoutput_noshell_pipes(dict(args=[sys.executable, "-c", "import sys; sys.exit(6)"]), dict(args=[sys.executable, "-c", "import sys; sys.exit(8)"])) + exitcode, output = getstatusoutput_noshell_pipe([sys.executable, "-c", "import sys; sys.exit(6)"], [sys.executable, "-c", "import sys; sys.exit(8)"]) assert exitcode == [6, 8] def test_check_output_pipes(): - output = check_output_pipes(dict(args=['echo', 'sonic']), dict(args=['awk', '{print $1}'])) + output = check_output_pipe(['echo', 'sonic'], ['awk', '{print $1}']) assert output == 'sonic\n' with pytest.raises(subprocess.CalledProcessError) as e: - check_output_pipes(dict(args=[sys.executable, "-c", "import sys; sys.exit(6)"]), dict(args=[sys.executable, "-c", "import sys; sys.exit(0)"])) + check_output_pipe([sys.executable, "-c", "import sys; sys.exit(6)"], [sys.executable, "-c", "import sys; sys.exit(0)"]) assert e.returncode == [6, 0] with pytest.raises(subprocess.CalledProcessError) as e: - check_output_pipes(dict(args=[sys.executable, "-c", "import sys; sys.exit(0)"]), dict(args=[sys.executable, "-c", "import sys; sys.exit(6)"])) + check_output_pipe([sys.executable, "-c", "import sys; sys.exit(0)"], [sys.executable, "-c", "import sys; sys.exit(6)"]) assert e.returncode == [0, 6] From cd007de797ad336912b5b3c0e89addb78471b027 Mon Sep 17 00:00:00 2001 From: maipbui Date: Sun, 18 Sep 2022 01:09:52 +0000 Subject: [PATCH 10/10] Fix comments Signed-off-by: maipbui --- src/sonic-py-common/tests/test_general.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sonic-py-common/tests/test_general.py b/src/sonic-py-common/tests/test_general.py index 27744d3e77c8..a395cf9aeb6b 100644 --- a/src/sonic-py-common/tests/test_general.py +++ b/src/sonic-py-common/tests/test_general.py @@ -11,14 +11,14 @@ def test_getstatusoutput_noshell(tmp_path): exitcode, output = getstatusoutput_noshell([sys.executable, "-c", "import sys; sys.exit(6)"]) assert exitcode != 0 -def test_getstatusoutput_noshell_pipes(): +def test_getstatusoutput_noshell_pipe(): exitcode, output = getstatusoutput_noshell_pipe(['echo', 'sonic'], ['awk', '{print $1}']) assert (exitcode, output) == ([0, 0], 'sonic') exitcode, output = getstatusoutput_noshell_pipe([sys.executable, "-c", "import sys; sys.exit(6)"], [sys.executable, "-c", "import sys; sys.exit(8)"]) assert exitcode == [6, 8] -def test_check_output_pipes(): +def test_check_output_pipe(): output = check_output_pipe(['echo', 'sonic'], ['awk', '{print $1}']) assert output == 'sonic\n'