[Python-Dev] Re: Two patches for the new subprocess module

[Nick Craig-Wood]

On Tue, Nov 23, 2004 at 12:47:37PM -0800, Russell E. Owen wrote:
> In article <20041123150846.GA22827 at craig-wood.com>,
>  Nick Craig-Wood <nick at craig-wood.com> wrote:
> 
> > I have submitted two patches for the subprocess module against cvs
> > HEAD.  The first notifies the user of a common mistake...
> >
> > subprocess has a very easy to mistake error in it - forgetting to pass
> > the command as a list.... with the patch...
> > 
> > >>> subprocess.call("ls", "-l")
> > Traceback (most recent call last):
> >   File "<stdin>", line 1, in ?
> >   File "subprocess.py", line 428, in call
> >     return Popen(*args, **kwargs).wait()
> >   File "subprocess.py", line 508, in __init__
> >     raise TypeError("bufsize must be an integer - "
> > TypeError: bufsize must be an integer - did you forget to pass your arguments 
> > in a list?
> 
> I hope you didn't totally eliminate the ability for Popen and call to 
> accept args as a string? It is a useful feature and one I've been using.

No, you can still pass *ONE* string as an arg, and that works fine.
If you attempt to pass two (not in a list) it raises an exception as
above.

If you pass two arguments to Popen() or call() the second actually
sets the buffer size...

> If you have broken this, please consider some alternative to your patch.
> 
> Perhaps changing the default for shell to True if args is a string, 
> False if a list would suffice? I'm not sure this is really the right 
> thing to do on all platforms, but it'd be an easy fix (use shell=None in 
> the argument list and then convert it to True or False as needed).

That is a really bad idea IMHO!  Its exactly what perl does in its
system() built in function, and it is a huge source of security holes
in perl scripts.

Eg programmer writes system("prog").  Then realises that prog needs an
argument so writes system("prog $arg").  This is now a potential
security problem if $arg was supplied by a user, because this will be
interpreted by the shell.  Say the user makes $arg="blah ; rm -rf *"
then you are in trouble.  If you are thinking about security you'd
write system("prog", $arg), but the only way this error is caught in
perl is with code review.

Back to python - the subprocess module doesn't have this problem.
subprocess.call("prog "+arg) won't actually work because it won't find
the binary called "prog "+arg.  It will just throw an exception at
that point - a very definite clue to the programmer that there is a
mistake.

Eg

>>> subprocess.call("ls -l")
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "subprocess.py", line 447, in call
    return Popen(*args, **kwargs).wait()
  File "subprocess.py", line 592, in __init__
    errread, errwrite)
  File "subprocess.py", line 1024, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory
>>> 

Having to write

  subprocess.call("prog "+arg, shell=True)

at least should give the programmer pause for thought, and hopefully
the shorter and better

  subprocess.call(["prog", arg])

would come to mind.

-- 
Nick Craig-Wood <nick at craig-wood.com> -- http://www.craig-wood.com/nick