On Tue, Nov 23, 2004 at 12:47:37PM -0800, Russell E. Owen wrote:
In article <20041123150846.GA22827@craig-wood.com>, Nick Craig-Wood <nick@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@craig-wood.com> -- http://www.craig-wood.com/nick