Two patches for the new subprocess module
I have submitted two patches for the subprocess module against cvs HEAD. The first notifies the user of a common mistake and the second adds an IHMO very important utility function. I'd consider the first to be a bug fix and the second a new feature. However a rather nice new feature ;-) ------------------------------------------------------------ [ 1071755 ] raise error for common mistake with subprocess http://python.org/sf/1071755 ------------------------------------------------------------ subprocess has a very easy to mistake error in it - forgetting to pass the command as a list. Eg
# Note no error, warning or anything, but no "ls -l" either And with the patch...
------------------------------------------------------------ [ 1071764 ] a new subprocess.call which raises an error on non-zero rc http://python.org/sf/1071764 ------------------------------------------------------------ The attached patch introduces a 3rd utility function - xcall() to the subprocess module. This function acts just like call but raises errors if the command had a non-zero return code. This saves writing if call(...): raise OSError(...) It is most useful for shell script replacement programming. Checking the return codes of commands called is often forgotten in shell programming. When you've moved up to python because shell is too limiting (usually about 10 lines of shell in my case ;-) you want to make sure that all your commands work so you write robust code. I consider raising an exception to be much more pythonic than checking a return code (ie "Errors should never pass silently" from Zen of Python) Eg # An easy to miss error
# becomes an impossible to miss exception
See patch for more! Its been tested under python 2.3 on windows and linux. -- Nick Craig-Wood <nick@craig-wood.com> -- http://www.craig-wood.com/nick
In article <20041123150846.GA22827@craig-wood.com>, Nick Craig-Wood <nick@craig-wood.com> wrote:
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. 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). One could imagine other fixes, such as offering one variant each of Popen and call that insists on args=string, leaving the main version to insist on args=list. Regards, -- Russell
On Tue, Nov 23, 2004 at 12:47:37PM -0800, Russell E. Owen wrote:
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...
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
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
In article <20041123150846.GA22827@craig-wood.com>, Nick Craig-Wood <nick@craig-wood.com> wrote:
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. 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). One could imagine other fixes, such as offering one variant each of Popen and call that insists on args=string, leaving the main version to insist on args=list. Regards, -- Russell
On Tue, Nov 23, 2004 at 12:47:37PM -0800, Russell E. Owen wrote:
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...
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
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
participants (2)
-
Nick Craig-Wood -
Russell E. Owen