[Patches] [ python-Patches-466451 ] popen fix for multiple quoted arguments

noreply@sourceforge.net noreply@sourceforge.net
Mon, 01 Oct 2001 19:51:04 -0700


Patches item #466451, was opened at 2001-09-29 17:34
You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=466451&group_id=5470

Category: Core (C code)
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Brian Quinlan (bquinlan)
Assigned to: Mark Hammond (mhammond)
Summary: popen fix for multiple quoted arguments

Initial Comment:
In a Windows version of Python, try the following:

os.popen('"<path-of-exe" "path-of-arg"'>.read()

cmd.exe will misinterpret the command string because 
it strips the first and last quote in the command 
string (cmd.exe /? for details).

This patch does a few things:

1. it encloses the command string sent to cmd.exe in 
quotes and uses the /S switch

2. if the shell is not command.com or cmd.exe, it 
passes the -c switch to the shell (this makes popen 
work if the user's shell is bash, sh, etc.)

3. changes the variables s1, s2 and s3 to cmdPath, 
completeCmd and cmdArgs

----------------------------------------------------------------------

>Comment By: Brian Quinlan (bquinlan)
Date: 2001-10-01 19:51

Message:
Logged In: YES 
user_id=108973

Sorry, I was sloppy about my point (I missed the -c 
switch). Here is dump from my UNIX box:

[93:~] brian% sh -c "ls" "/usr"
Desktop      Documents    Movies       Pictures     
Sites        bash-2.05
Dev          Library      Music        Public       Wolf MP 
Test setiathome
[93:~] brian% sh -c ""ls" "/usr""
bin        lib        local      share
include    libexec    sbin       standalone
[93:~] brian% python
[...]
>>> import os
>>> os.popen("ls /usr").read()
'bin\ninclude\nlib\nlibexec\nlocal\nsbin\nshare\nstandalone\
n'

You will notice that the results from Python are inline 
with the quoted invocation of sh -c. And "man popen" on my 
box says that "This command is passed to /bin/sh using the -
c flag;..." so presumably popen is quoting the arguments 
(Windows _popen doesn't though, I checked).

And I didn't change the COMMAND.COM behavior - I think that 
it is a special case that is too broken to fix and will, 
with any luck, will die soon.

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2001-10-01 19:12

Message:
Logged In: YES 
user_id=31435

Special-casing bash instead (+ other shells known to need 
it) to use -c would be fine.

I *expect* os.system and os.popen to work differently 
across platforms, because both depend crucially on the 
platform shell spawned to crack the cmdline.  Tcl's "exec" 
(which I mentioned before) has the only good solution to 
this I'm aware of, via getting the platform shells out of 
the equation entirely (even on Unix).  Python does nothing 
of the sort, and the more undocumented tricks are stuffed 
into it, the more baffling it gets when things go wrong (as 
they often will, and especially on Windows).

I didn't understand the point to noting that

    bash "ls" "/usr/bin"

won't work on Unix.  Neither will

    bash ls /usr/bin

The quotes are irrelevant here, yes?  Python isn't adding 
quotes to things, or deleting them, anyway.  Note that if 
you write your own copy.bat,

   command /c "copy" "whatever1" "whatever2"

does work fine on Win9x (command.com).  It doesn't work 
with the Windows copy solely because copy is a command.com 
builtin -- and you can't hide arbitrary platform crap like 
that from users either.

----------------------------------------------------------------------

Comment By: Brian Quinlan (bquinlan)
Date: 2001-10-01 17:43

Message:
Logged In: YES 
user_id=108973

Tim, I don't disagree with your first point; without 
specific knowledge of each shell, you really have to guess 
what the correct shell arguments format is. There are two 
ways that we can work around that:

1. remove those two lines of code in my patch that do the 
special case
2. special case bash and sh (at least bash please) and 
use /C for the other cases

I disagree with everything else :-)

The command:
bash "ls" "/usr/bin" 

will NOT work on UNIX. Just like typing: 
cmd /c "copy" "c:\..." "c:\..."

won't work. So the UNIX popen implementation must be doing 
the moral equivalent of quoting the entire command string. 
So the current behavior is suprising and inconsistent 
accross platforms.

I don't think that we can do anything with regards to 
backwards compatibility but I think that the cost/benifit 
favours fixing the problem i.e. accepting my patch :-)

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2001-10-01 15:36

Message:
Logged In: YES 
user_id=31435

Assigned to MarkH for further pondering.

I like giving vrbls useful names  The rest seems highly 
dubious:

+ Catering to non-MS shells ("-c" vs "/C") is a fine idea, 
but what about 4DOS or 4NT, or other non-MS shells that 
also expect /C?  It looks like this patch will kill people 
using them.

- It's not backwards compatible regardless of shell (as 
Brian pointed out).

- It's surprising to anyone who has read the cmd docs and 
expects cmd.exe to work as documented (by MS).

- Since it's unique to Python's popen, a command string 
that works for popen() may break if passed to os.system() 
instead, and vice versa, and maybe for os.startfile() too.

Other things bug me, but I'll stop there <wink>.  The MS 
shells are a godawful mess, and it would really be nice to 
supply a rational x-platform "virtual shell" syntax (as Tcl 
does for its "exec" cmd).  Short of that, I'm not sure a 
collection of inconsistent and undocumented hacks is 
actually progress; as is, Python's behavior is at least 
predictable from reading the MS docs and/or playing with C 
programs under MSVC.

----------------------------------------------------------------------

Comment By: Brian Quinlan (bquinlan)
Date: 2001-09-30 13:11

Message:
Logged In: YES 
user_id=108973

Mark understands the issue; I've already talked to him 
about this. 

Another thing that I should have noted is that this patch 
does NOT ensure backwards compatibility. Any scripts that 
manually quote their command strings, to work around this 
problem, will break.

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2001-09-30 11:46

Message:
Logged In: YES 
user_id=6380

Tim, whaddayathink? Or is this for Mark?

----------------------------------------------------------------------

You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=466451&group_id=5470