From 8b9020458a8576459040fce985ab140f0876e2f1 Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Tue, 29 Sep 2009 19:10:15 +0000 Subject: #5329: fix os.popen* regression from 2.5: don't execute commands as a sequence through the shell. also document the correct subprocess replacement for this case patch from Jean-Paul Calderone and Jani Hakala --- Doc/library/subprocess.rst | 50 ++++++++++++++++++++++++----------------- Lib/os.py | 15 +++++++------ Lib/subprocess.py | 56 +++++++++++++++++++++++++++++++++------------- Lib/test/test_popen2.py | 26 +++++++++++++++++++++ Misc/NEWS | 4 ++++ 5 files changed, 109 insertions(+), 42 deletions(-) diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index 09ae62b..10747e6 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -457,21 +457,21 @@ Replacing :func:`os.popen`, :func:`os.popen2`, :func:`os.popen3` :: - pipe = os.popen(cmd, 'r', bufsize) + pipe = os.popen("cmd", 'r', bufsize) ==> - pipe = Popen(cmd, shell=True, bufsize=bufsize, stdout=PIPE).stdout + pipe = Popen("cmd", shell=True, bufsize=bufsize, stdout=PIPE).stdout :: - pipe = os.popen(cmd, 'w', bufsize) + pipe = os.popen("cmd", 'w', bufsize) ==> - pipe = Popen(cmd, shell=True, bufsize=bufsize, stdin=PIPE).stdin + pipe = Popen("cmd", shell=True, bufsize=bufsize, stdin=PIPE).stdin :: - (child_stdin, child_stdout) = os.popen2(cmd, mode, bufsize) + (child_stdin, child_stdout) = os.popen2("cmd", mode, bufsize) ==> - p = Popen(cmd, shell=True, bufsize=bufsize, + p = Popen("cmd", shell=True, bufsize=bufsize, stdin=PIPE, stdout=PIPE, close_fds=True) (child_stdin, child_stdout) = (p.stdin, p.stdout) @@ -479,9 +479,9 @@ Replacing :func:`os.popen`, :func:`os.popen2`, :func:`os.popen3` (child_stdin, child_stdout, - child_stderr) = os.popen3(cmd, mode, bufsize) + child_stderr) = os.popen3("cmd", mode, bufsize) ==> - p = Popen(cmd, shell=True, bufsize=bufsize, + p = Popen("cmd", shell=True, bufsize=bufsize, stdin=PIPE, stdout=PIPE, stderr=PIPE, close_fds=True) (child_stdin, child_stdout, @@ -489,21 +489,33 @@ Replacing :func:`os.popen`, :func:`os.popen2`, :func:`os.popen3` :: - (child_stdin, child_stdout_and_stderr) = os.popen4(cmd, mode, bufsize) + (child_stdin, child_stdout_and_stderr) = os.popen4("cmd", mode, + bufsize) ==> - p = Popen(cmd, shell=True, bufsize=bufsize, + p = Popen("cmd", shell=True, bufsize=bufsize, stdin=PIPE, stdout=PIPE, stderr=STDOUT, close_fds=True) (child_stdin, child_stdout_and_stderr) = (p.stdin, p.stdout) +On Unix, os.popen2, os.popen3 and os.popen4 also accept a sequence as +the command to execute, in which case arguments will be passed +directly to the program without shell intervention. This usage can be +replaced as follows:: + + (child_stdin, child_stdout) = os.popen2(["/bin/ls", "-l"], mode, + bufsize) + ==> + p = Popen(["/bin/ls", "-l"], bufsize=bufsize, stdin=PIPE, stdout=PIPE) + (child_stdin, child_stdout) = (p.stdin, p.stdout) + Return code handling translates as follows:: - pipe = os.popen(cmd, 'w') + pipe = os.popen("cmd", 'w') ... rc = pipe.close() - if rc != None and rc % 256: + if rc != None and rc % 256: print "There were some errors" ==> - process = Popen(cmd, 'w', stdin=PIPE) + process = Popen("cmd", 'w', shell=True, stdin=PIPE) ... process.stdin.close() if process.wait() != 0: @@ -513,11 +525,6 @@ Return code handling translates as follows:: Replacing functions from the :mod:`popen2` module ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -.. note:: - - If the cmd argument to popen2 functions is a string, the command is executed - through /bin/sh. If it is a list, the command is directly executed. - :: (child_stdout, child_stdin) = popen2.popen2("somestring", bufsize, mode) @@ -526,9 +533,12 @@ Replacing functions from the :mod:`popen2` module stdin=PIPE, stdout=PIPE, close_fds=True) (child_stdout, child_stdin) = (p.stdout, p.stdin) -:: +On Unix, popen2 also accepts a sequence as the command to execute, in +which case arguments will be passed directly to the program without +shell intervention. This usage can be replaced as follows:: - (child_stdout, child_stdin) = popen2.popen2(["mycmd", "myarg"], bufsize, mode) + (child_stdout, child_stdin) = popen2.popen2(["mycmd", "myarg"], bufsize, + mode) ==> p = Popen(["mycmd", "myarg"], bufsize=bufsize, stdin=PIPE, stdout=PIPE, close_fds=True) diff --git a/Lib/os.py b/Lib/os.py index 40d117e..c41af1a 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -666,8 +666,9 @@ if _exists("fork"): import subprocess PIPE = subprocess.PIPE - p = subprocess.Popen(cmd, shell=True, bufsize=bufsize, - stdin=PIPE, stdout=PIPE, close_fds=True) + p = subprocess.Popen(cmd, shell=isinstance(cmd, basestring), + bufsize=bufsize, stdin=PIPE, stdout=PIPE, + close_fds=True) return p.stdin, p.stdout __all__.append("popen2") @@ -685,9 +686,9 @@ if _exists("fork"): import subprocess PIPE = subprocess.PIPE - p = subprocess.Popen(cmd, shell=True, bufsize=bufsize, - stdin=PIPE, stdout=PIPE, stderr=PIPE, - close_fds=True) + p = subprocess.Popen(cmd, shell=isinstance(cmd, basestring), + bufsize=bufsize, stdin=PIPE, stdout=PIPE, + stderr=PIPE, close_fds=True) return p.stdin, p.stdout, p.stderr __all__.append("popen3") @@ -705,8 +706,8 @@ if _exists("fork"): import subprocess PIPE = subprocess.PIPE - p = subprocess.Popen(cmd, shell=True, bufsize=bufsize, - stdin=PIPE, stdout=PIPE, + p = subprocess.Popen(cmd, shell=isinstance(cmd, basestring), + bufsize=bufsize, stdin=PIPE, stdout=PIPE, stderr=subprocess.STDOUT, close_fds=True) return p.stdin, p.stdout __all__.append("popen4") diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 6b91f69..45e49a1 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -298,54 +298,80 @@ Popen(["/bin/mycmd", "myarg"], env={"PATH": "/usr/bin"}) Replacing os.popen* ------------------- -pipe = os.popen(cmd, mode='r', bufsize) +pipe = os.popen("cmd", mode='r', bufsize) ==> -pipe = Popen(cmd, shell=True, bufsize=bufsize, stdout=PIPE).stdout +pipe = Popen("cmd", shell=True, bufsize=bufsize, stdout=PIPE).stdout -pipe = os.popen(cmd, mode='w', bufsize) +pipe = os.popen("cmd", mode='w', bufsize) ==> -pipe = Popen(cmd, shell=True, bufsize=bufsize, stdin=PIPE).stdin +pipe = Popen("cmd", shell=True, bufsize=bufsize, stdin=PIPE).stdin -(child_stdin, child_stdout) = os.popen2(cmd, mode, bufsize) +(child_stdin, child_stdout) = os.popen2("cmd", mode, bufsize) ==> -p = Popen(cmd, shell=True, bufsize=bufsize, +p = Popen("cmd", shell=True, bufsize=bufsize, stdin=PIPE, stdout=PIPE, close_fds=True) (child_stdin, child_stdout) = (p.stdin, p.stdout) (child_stdin, child_stdout, - child_stderr) = os.popen3(cmd, mode, bufsize) + child_stderr) = os.popen3("cmd", mode, bufsize) ==> -p = Popen(cmd, shell=True, bufsize=bufsize, +p = Popen("cmd", shell=True, bufsize=bufsize, stdin=PIPE, stdout=PIPE, stderr=PIPE, close_fds=True) (child_stdin, child_stdout, child_stderr) = (p.stdin, p.stdout, p.stderr) -(child_stdin, child_stdout_and_stderr) = os.popen4(cmd, mode, bufsize) +(child_stdin, child_stdout_and_stderr) = os.popen4("cmd", mode, + bufsize) ==> -p = Popen(cmd, shell=True, bufsize=bufsize, +p = Popen("cmd", shell=True, bufsize=bufsize, stdin=PIPE, stdout=PIPE, stderr=STDOUT, close_fds=True) (child_stdin, child_stdout_and_stderr) = (p.stdin, p.stdout) +On Unix, os.popen2, os.popen3 and os.popen4 also accept a sequence as +the command to execute, in which case arguments will be passed +directly to the program without shell intervention. This usage can be +replaced as follows: + +(child_stdin, child_stdout) = os.popen2(["/bin/ls", "-l"], mode, + bufsize) +==> +p = Popen(["/bin/ls", "-l"], bufsize=bufsize, stdin=PIPE, stdout=PIPE) +(child_stdin, child_stdout) = (p.stdin, p.stdout) + +Return code handling translates as follows: + +pipe = os.popen("cmd", 'w') +... +rc = pipe.close() +if rc != None and rc % 256: + print "There were some errors" +==> +process = Popen("cmd", 'w', shell=True, stdin=PIPE) +... +process.stdin.close() +if process.wait() != 0: + print "There were some errors" + Replacing popen2.* ------------------ -Note: If the cmd argument to popen2 functions is a string, the command -is executed through /bin/sh. If it is a list, the command is directly -executed. - (child_stdout, child_stdin) = popen2.popen2("somestring", bufsize, mode) ==> p = Popen(["somestring"], shell=True, bufsize=bufsize stdin=PIPE, stdout=PIPE, close_fds=True) (child_stdout, child_stdin) = (p.stdout, p.stdin) +On Unix, popen2 also accepts a sequence as the command to execute, in +which case arguments will be passed directly to the program without +shell intervention. This usage can be replaced as follows: -(child_stdout, child_stdin) = popen2.popen2(["mycmd", "myarg"], bufsize, mode) +(child_stdout, child_stdin) = popen2.popen2(["mycmd", "myarg"], bufsize, + mode) ==> p = Popen(["mycmd", "myarg"], bufsize=bufsize, stdin=PIPE, stdout=PIPE, close_fds=True) diff --git a/Lib/test/test_popen2.py b/Lib/test/test_popen2.py index 743b873..1383c62 100644 --- a/Lib/test/test_popen2.py +++ b/Lib/test/test_popen2.py @@ -78,6 +78,14 @@ class Popen2Test(unittest.TestCase): def test_os_popen2(self): # same test as test_popen2(), but using the os.popen*() API + if os.name == 'posix': + w, r = os.popen2([self.cmd]) + self.validate_output(self.teststr, self.expected, r, w) + + w, r = os.popen2(["echo", self.teststr]) + got = r.read() + self.assertEquals(got, self.teststr + "\n") + w, r = os.popen2(self.cmd) self.validate_output(self.teststr, self.expected, r, w) @@ -87,9 +95,27 @@ class Popen2Test(unittest.TestCase): w, r, e = os.popen3([self.cmd]) self.validate_output(self.teststr, self.expected, r, w, e) + w, r, e = os.popen3(["echo", self.teststr]) + got = r.read() + self.assertEquals(got, self.teststr + "\n") + got = e.read() + self.assertFalse(got, "unexpected %r on stderr" % got) + w, r, e = os.popen3(self.cmd) self.validate_output(self.teststr, self.expected, r, w, e) + def test_os_popen4(self): + if os.name == 'posix': + w, r = os.popen4([self.cmd]) + self.validate_output(self.teststr, self.expected, r, w) + + w, r = os.popen4(["echo", self.teststr]) + got = r.read() + self.assertEquals(got, self.teststr + "\n") + + w, r = os.popen4(self.cmd) + self.validate_output(self.teststr, self.expected, r, w) + def test_main(): run_unittest(Popen2Test) diff --git a/Misc/NEWS b/Misc/NEWS index 9cd6638..13912e9 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -12,6 +12,10 @@ What's New in Python 2.7 alpha 1 Core and Builtins ----------------- +- Issue #5329: Fix os.popen* regression from 2.5 with commands as a + sequence running through the shell. Patch by Jean-Paul Calderone + and Jani Hakala. + - Issue #7019: Raise ValueError when unmarshalling bad long data, instead of producing internally inconsistent Python longs. -- cgit v0.12