classification
Title: subprocess.getoutput fails on win32
Type: behavior Stage: resolved
Components: Windows Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: tim.golden Nosy List: Arfrever, bpoaugust, brian.curtin, eric.araujo, flox, gregory.p.smith, haypo, jldm, ncoghlan, ned.deily, python-dev, r.david.murray, terry.reedy, tim.golden
Priority: normal Keywords: needs review, patch

Created on 2010-10-26 12:18 by jldm, last changed 2014-03-08 01:26 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
subprocess_getoutput.patch haypo, 2011-03-03 13:18 review
subprocess.patch bpoaugust, 2011-10-17 22:30 Patch to implement ( ) wrapping for Windows
issue10197.diff tim.golden, 2013-11-01 14:45
Messages (31)
msg119605 - (view) Author: (jldm) Date: 2010-10-26 12:18
Hi, first of all sorry for my English.

On windows XP SP3, the following code:

import subprocess
subprocess.getoutput("dir")

returns
    '"{" is not recognized as an internal or external command,\noperable program or batch file.'

I made a workaround by changing in the file Lib/subprocess.py the line 574 (I thin in 3.2a3 is 584) (in the getstatusoutput(cmd) function definition) from:
pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r')

to:
pipe = os.popen('( ' + cmd + '; ) 2>&1', 'r')

I have tested it with:

ActivePython 3.1.2.3 (ActiveState Software Inc.) based on
Python 3.1.2 (r312:79147, Mar 22 2010, 12:20:29) [MSC v.1500 32 bit (Intel)] on win32

and

Python 3.2a3 (r32a3:85355, Oct 10 2010, 17:11:45) [MSC v.1500 32 bit (Intel)] on win32


Regards from Spain.
José Luis Domenech
msg119607 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-10-26 13:29
Oddly, the test suite skips getoutput and getstatusoutput on windows with the comment that the source says it is relevant only for posix, but the documentation does not have 'availability: unix' tags.  (It is also odd that getoutput isn't documented, but that's a different issue.)

Your workaround can't be used as a fix, since the semantics of {}s in the shell are different from those of ()s.

It's not clear to me what the point of the {}s is, but I have a fear that eliminating them would introduce subtle changes in the behavior of getoutput calls.  Perhaps not, though.

It looks like this issue amounts to an RFE for support of getoutput/getstatusoutput on Windows, though the fact that it is not documented as unix-only may make it a bug instead :)

The appropriate fix is probably to conditionalize the code based on platform.  A complete patch will require unit test changes and documentation changes (since the docs currently mention the braces).

All of that said, it also appears that the new check_output should be preferred to either getoutput or getstatusoutput.  Perhaps those functions could be re-implemented in terms of check_output.
msg120558 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-05 23:21
() is used to launch a command in a sub-shell and {} is used to group commands, for example to set up a stream redirection for all commands in brackets.
msg123130 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-12-02 22:05
I think we should implement getstatusoutput and getoutput with Popen objects to gain portability and avoid spawning subshells.
msg123141 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-02 22:47
Do you have in implementation in mind?  I'm not clear how this would work.
msg123150 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-12-03 00:49
My idea is simply using Popen with appropriate args for stdout and stderr instead of using a shell command with redirections:

--- Lib/subprocess.py   (révision 86943)
+++ Lib/subprocess.py   (copie de travail)
@@ -560,11 +560,7 @@
     return ''.join(result)


-# Various tools for executing commands and looking at their output and status.
-#
-# NB This only works (and is only relevant) for UNIX.
-
-def getstatusoutput(cmd):
+def getstatusoutput(cmd, shell=True):
     """Return (status, output) of executing cmd in a shell.

     Execute the string 'cmd' in a shell with os.popen() and return a 2-tuple
@@ -581,14 +577,21 @@
     >>> subprocess.getstatusoutput('/bin/junk')
     (256, 'sh: /bin/junk: not found')
     """
-    pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r')
-    text = pipe.read()
-    sts = pipe.close()
-    if sts is None: sts = 0
-    if text[-1:] == '\n': text = text[:-1]
+    # Wish I could use with...
+    popen = Popen(cmd, shell=shell, stdout=PIPE, stderr=STDOUT)
+    sts = popen.communicate() #or wait() or whatever is the right thing
+    try:
+        text = process.stdout.read()
+    finally:
+        process.stdout.close()
+    if sts is None:
+        sts = 0
+    if text.endswith('\n'):
+        text = text[:-1]
     return sts, text

(The new “shell” argument is icing on the cake, allowing us to later support a list as cmd argument like Popen does.)
msg123232 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-12-03 12:35
-def getstatusoutput(cmd):
+def getstatusoutput(cmd, shell=True):

shell=True is dangerous, it can lead to shell command injection. I would prefer to set its default value to False. The function already exists in Python 3.1, but it is not used in Python source code. Is it too late to fix its API to avoid security vulnerabilities?
msg123237 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-12-03 12:46
> The function already exists in Python 3.1, but it is not used in Python source code

We don’t know what code out there uses.  This would be an incompatible change.
msg123246 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-03 13:53
Ah, I did not realize that getstatusoutput was implemented using os.popen.  I thought it already used Popen.  Now, in python3, os.popen is in turn implemented using subprocess.Popen, so removing that level of indirection seems sensible.

The question that remains is, does removing the {} change the output obtained from a command sequence in any way?

Note that for backward compatibility you will need to re-munge the status code into C format.  Which makes me wonder if getoutput/getstatusoutput should just be documentationally deprecated instead.  (I never use them myself, FWIW)
msg123298 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2010-12-03 22:28
See also Issue9922
msg123410 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-12-05 01:37
> The question that remains is, does removing the {} change the output
> obtained from a command sequence in any way?
{} are used to group output from the commands into one stream.  I believe the stdout and stderr arguments to Popen allow us to get compatible behavior.  Tests need to prove that, of course.

> Which makes me wonder if getoutput/getstatusoutput should just be
> documentationally deprecated instead.  (I never use them myself, FWIW)
They were relocated from the commands module, resulting in slightly overlapping functions that don’t share a naming patter: call, check_call, getoutput, getstatusoutput.
msg129967 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-03-03 13:18
subprocess_getoutput.patch: patch subprocess.getstatusoutput() to use directly Popen, instead of os.popen, with stderr=subprocess.STDOUT instead of "2>&1" shell redirection. It strips also all trailing spaces and newlines, not just the last one. And finally, it removes "Availability: UNIX." from the documentation.

I tried to add a shell argument (to be able to disable the shell) and to accept any Popen keyword, but I don't know how to implement shell=False if the input is a list of arguments. list2cmdline() is unsafe on UNIX (see #8972). And if getstatusoutput() doesn't accept argument list, it becomes useless with shell=False (it doesn't support to call a program with arguments).

Note: the status is still shifted on UNIX to be compatible with the wait() format.
msg129974 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-03-03 15:35
> I tried to add a shell argument (to be able to disable the shell) and
> to accept any Popen keyword, but I don't know how to implement
> shell=False if the input is a list of arguments. list2cmdline() is
> unsafe on UNIX (see #8972).

Example of function to escape a list of arguments on UNIX:

def escapeargs(*args):
   return ' '.join(pipes.quote(arg) for arg in args)

R. David Murray disagree with me to allow getoutput(list) (shell=True) because Popen(list, shell=True) behaves differently.

subprocess.Popen(['echo Hello'], shell=True) writes 'Hello', whereas subprocess.Popen(['echo', 'Hello'], shell=True) writes nothing (because echo has no argument.

I would like to do something like that: getoutput(['echo', 'Hello']) calls Popen('echo Hello', shell=True) using escapeargs() function defined above. So getoutput(list) calls shell -c "arg1 arg2", whereas Popen(list, shell=True) calls shell -c "arg1" arg2 arg3 ...

See also issue #7839 for Popen(str, shell=False) and Popen(list, shell=True) cases.
msg145735 - (view) Author: (bpoaugust) Date: 2011-10-17 17:22
subprocess.getoutput does not currently work at all on Windows.
So it's not necessary to maintain backwards compatibility.

The following fix works for me on WinXP/Python 3.2.2.

Replace

    pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r') # line 613 of subprocess.py

with

    if mswindows:
        pipe = os.popen(cmd + ' 2>&1', 'r') # Windows does not support { }
    else:
        pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r')
msg145764 - (view) Author: (bpoaugust) Date: 2011-10-17 22:30
A better fix, which supports multiple windows commands:

    if mswindows:
        pipe = os.popen('( ' + cmd + ' ) 2>&1', 'r') # Windows uses () rather than { }
    else:
        pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r')

This works with the command

subprocess.getoutput("echo before & python -V & echo after")

Note that python -V writes to stderr, so without the enclosing ( ) the version information is not captured.
msg145824 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-18 16:01
If Windows shell syntax is similar to POSIX one, then () will run in a sub-shell, which would be a different behavior than using {} (which merely group statements and their streams).
msg145835 - (view) Author: (bpoaugust) Date: 2011-10-18 16:22
I got the () syntax from:

http://technet.microsoft.com/en-us/library/cc737438%28WS.10%29.aspx

which refers to grouping, not subshell.
msg146365 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-10-25 12:23
Without knowing this issue existed, I recently started working on adding some convenience APIs for shell invocation to shutil: http://bugs.python.org/issue13238

I think the getstatus and getstatusoutput APIs were copied from the commands module in 3.0 without sufficient thought being given to whether or not they fit with the design principles of the subprocess module. IMO, both should be deprecated:
   - they're not cross-platform
   - they invoke the shell implicitly, which subprocess promises never to do
msg146379 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-25 16:11
> IMO, both should be deprecated:
> - they're not cross-platform
Isn’t the purpose of this report to fix that? :)

> - they invoke the shell implicitly, which subprocess promises never to do
One could argue that it’s not implicit if it’s documented.  Nonetheless, I agree that they don’t fit well with the subprocess promises.

So, +1 on deprecating and +1 on new, safer helpers.
msg157670 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-04-06 16:51
I think that adding safer wrappers and deprecating things are valuable but different bugs.  In the short term, we could apply the proposed small patch to just fix the issue at hand.  Can one of the Windows experts weigh in?

The patch does this:

    if mswindows:
        pipe = os.popen('( ' + cmd + ' ) 2>&1', 'r')
    else:
        pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r')

It was tested manually; a test should be simple to write.
msg201907 - (view) Author: Tim Golden (tim.golden) * (Python committer) Date: 2013-11-01 14:45
Patched according to Nick Coghlan's suggestion in http://bugs.python.org/issue9922#msg150093. Ad hoc tests look ok on Windows. I'll add tests & look at *nix later.
msg202026 - (view) Author: Roundup Robot (python-dev) Date: 2013-11-03 14:23
New changeset c34e163c0086 by Tim Golden in branch '3.3':
Issue #10197 Rework subprocess.get[status]output to use subprocess functionality and thus to work on Windows. Patch by Nick Coghlan.
http://hg.python.org/cpython/rev/c34e163c0086

New changeset 05ce1bd1a4c2 by Tim Golden in branch 'default':
Issue #10197 Rework subprocess.get[status]output to use subprocess functionality and thus to work on Windows. Patch by Nick Coghlan.
http://hg.python.org/cpython/rev/05ce1bd1a4c2

New changeset b6efaa97ee0e by Tim Golden in branch '3.3':
Issue #10197: merge heads
http://hg.python.org/cpython/rev/b6efaa97ee0e

New changeset 28a0ae3dcb16 by Tim Golden in branch 'default':
Issue #10197: merge heads
http://hg.python.org/cpython/rev/28a0ae3dcb16

New changeset fe828884a077 by Tim Golden in branch 'default':
Issue #10197: merge 3.3
http://hg.python.org/cpython/rev/fe828884a077
msg202031 - (view) Author: Tim Golden (tim.golden) * (Python committer) Date: 2013-11-03 16:01
Code & tests now work on Windows. Applied to 3.3 & 3.4.
msg202044 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2013-11-03 17:39
The documentation needs updating to state that these are available on Windows (currently it says UNIX) with a versionchanged annotation.

http://docs.python.org/3.3/library/subprocess.html#legacy-shell-invocation-functions
msg202048 - (view) Author: Roundup Robot (python-dev) Date: 2013-11-03 18:28
New changeset 2924a63aab73 by Tim Golden in branch '3.3':
Issue #10197: Indicate availability of subprocess.get[status]output on Windows and add a note about the effects of universal newlines
http://hg.python.org/cpython/rev/2924a63aab73

New changeset effad2bda4cb by Tim Golden in branch 'default':
Issue #10197: Indicate availability of subprocess.get[status]output on Windows and add a note about the effects of universal newlines
http://hg.python.org/cpython/rev/effad2bda4cb
msg202050 - (view) Author: Tim Golden (tim.golden) * (Python committer) Date: 2013-11-03 18:32
Good point. I've added the versionchanged tag.

The issue with bytes-string encoding goes all the way back to 
Popen.communicate if universal newlines mode is used so I've simply put 
in a reference to the existing notes on the subject higher up in the docs.
msg202208 - (view) Author: Roundup Robot (python-dev) Date: 2013-11-05 12:58
New changeset 0aa2aedc6a21 by Tim Golden in branch 'default':
Issue #10197 Tweak docs for subprocess.getstatusoutput and align the documentation, the module docstring, and the function docstring.
http://hg.python.org/cpython/rev/0aa2aedc6a21
msg202450 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-11-08 23:32
Is this ready to be reclosed?
msg202504 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * Date: 2013-11-10 01:09
Lib/subprocess.py still has outdated comment:

# NB This only works (and is only relevant) for POSIX.
msg202630 - (view) Author: Tim Golden (tim.golden) * (Python committer) Date: 2013-11-11 15:09
Thanks: final outdated comments removed
msg212918 - (view) Author: Roundup Robot (python-dev) Date: 2014-03-08 01:26
New changeset 34df43c9c74a by R David Murray in branch '3.3':
#10197: Update get[status]output versionchanged with actual version.
http://hg.python.org/cpython/rev/34df43c9c74a

New changeset ee277b383d33 by R David Murray in branch 'default':
#10197: Update get[status]output versionchanged with actual version.
http://hg.python.org/cpython/rev/ee277b383d33
History
Date User Action Args
2014-03-08 01:26:13python-devsetmessages: + msg212918
2013-11-11 15:09:35tim.goldensetstatus: open -> closed

messages: + msg202630
2013-11-10 01:09:43Arfreversetnosy: + Arfrever
messages: + msg202504
2013-11-08 23:32:25terry.reedysetnosy: + terry.reedy
messages: + msg202450
2013-11-05 12:58:40python-devsetmessages: + msg202208
2013-11-03 18:32:51tim.goldensetmessages: + msg202050
2013-11-03 18:28:13python-devsetmessages: + msg202048
2013-11-03 17:39:43gregory.p.smithsetstatus: closed -> open
nosy: + gregory.p.smith
messages: + msg202044

2013-11-03 16:01:09tim.goldensetstatus: open -> closed
resolution: fixed
messages: + msg202031

stage: test needed -> resolved
2013-11-03 14:23:35python-devsetnosy: + python-dev
messages: + msg202026
2013-11-01 14:45:39tim.goldensetfiles: + issue10197.diff

messages: + msg201907
2013-11-01 13:31:16tim.goldensetassignee: tim.golden
versions: + Python 3.4, - Python 3.2
2012-08-20 03:57:25r.david.murraylinkissue5808 superseder
2012-06-15 12:38:49r.david.murraylinkissue15073 superseder
2012-04-06 16:51:07eric.araujosetnosy: + tim.golden
messages: + msg157670

keywords: + needs review
stage: test needed
2011-10-25 16:11:00eric.araujosetmessages: + msg146379
2011-10-25 12:23:00ncoghlansetnosy: + ncoghlan
messages: + msg146365
2011-10-18 16:22:39bpoaugustsetmessages: + msg145835
2011-10-18 16:01:08eric.araujosetmessages: + msg145824
versions: - Python 3.1
2011-10-18 09:21:35floxsetnosy: + flox
2011-10-17 23:16:00hayposetversions: + Python 3.3
2011-10-17 22:37:10bpoaugustsetversions: - Python 3.3
2011-10-17 22:30:03bpoaugustsetfiles: + subprocess.patch

messages: + msg145764
2011-10-17 17:22:49bpoaugustsetnosy: + bpoaugust

messages: + msg145735
versions: + Python 3.3
2011-03-03 15:35:15hayposetnosy: haypo, ned.deily, eric.araujo, r.david.murray, brian.curtin, jldm
messages: + msg129974
2011-03-03 13:18:53hayposetfiles: + subprocess_getoutput.patch

messages: + msg129967
keywords: + patch
nosy: haypo, ned.deily, eric.araujo, r.david.murray, brian.curtin, jldm
2010-12-05 01:37:11eric.araujosetmessages: + msg123410
2010-12-03 22:28:48ned.deilysetnosy: + ned.deily
messages: + msg123298
2010-12-03 13:53:44r.david.murraysetmessages: + msg123246
2010-12-03 12:46:18eric.araujosetmessages: + msg123237
2010-12-03 12:35:34hayposetnosy: + haypo
messages: + msg123232
2010-12-03 00:49:27eric.araujosetmessages: + msg123150
2010-12-02 22:47:55r.david.murraysetmessages: + msg123141
2010-12-02 22:05:52eric.araujosetmessages: + msg123130
2010-11-05 23:21:46eric.araujosetnosy: + eric.araujo
messages: + msg120558
2010-10-26 14:06:10brian.curtinsetnosy: + brian.curtin
components: + Windows
2010-10-26 13:29:16r.david.murraysetnosy: + r.david.murray
messages: + msg119607
2010-10-26 12:18:50jldmcreate