[issue13238] Add shell command helpers to shutil module
Éric Araujo mer...@netwok.org added the comment: I’m not sure my question was well phrased. If I have these files: spam.py ham.py foo bar.py will a pattern of '*.py' match all of them with your functions, even the one with an embedded space? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Antoine Pitrou pit...@free.fr added the comment: With the default whitespace escaping (which allows spaces in filenames), wildcard matching still works (thus the list of directories matching the ../py* pattern), but with full quoting it breaks (thus the nothing named '../py*' result). My question is why it would be a good idea to make a difference between whitespace and other characters. If you use a wildcard pattern, generally it won't contain spaces at all, so you don't have to quote it. If you are injecting a normal filename, noticing that whitespace gets quoted may get you a false sense of security until somebody injects a wildcard character that won't get quoted. So what I'm saying is that a middleground between quoting and no quoting is dangerous and not very useful. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment: Yeah, I was thinking about this a bit more and realised that I'd rejected the quote everything by default approach before I had the idea of providing a custom conversion specifier to disable the implicit string conversion and quoting. So perhaps a better alternative would be: default - str + shlex.quote !u - unquoted (i.e. normal str.format default behaviour) When you have a concise way to explicitly bypass it, making the default behaviour as safe as possible seems like a good way to go. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment: I realised I could use the convert_field() option in the custom formatter to choose between several interpolation quoting options: default - str + shutil.quote_ascii_whitespace !q - str + shlex.quote !u - unquoted (i.e. no conversion, str.format default behaviour) !s - str (as usual) !r - repr (as usual) The most recent commit also exposes public APIs for the formatting aspects (shutil.quote_ascii_whitespace, shutil.shell_format, shutil.shell_format_map) -- keywords: +patch Added file: http://bugs.python.org/file23542/issue13238_shell_helpers.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment:
Some examples:
import shutil
shutil.shell_call(du -hs {}, ../py*)
594M../py3k
579M../py3k_pristine
480M../python27
301M../python31
382M../python32
288K../python_swallowed_whole
0
shutil.shell_call(du -hs {!q}, ../py*)
du: cannot access `../py*': No such file or directory
1
shutil.shell_call(ls {}, no file)
ls: cannot access no file: No such file or directory
2
shutil.shell_call(ls {!u}, no file)
ls: cannot access no: No such file or directory
ls: cannot access file: No such file or directory
2
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13238
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Éric Araujo mer...@netwok.org added the comment: The custom formatter idea sounds brilliant. Can you test that auto-escaping of spaces works well with glob patterns? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Antoine Pitrou pit...@free.fr added the comment: default - str + shutil.quote_ascii_whitespace !q - str + shlex.quote !u - unquoted (i.e. no conversion, str.format default behaviour) The default doesn't look very understandable to me. Why would you quote only some characters and not all of them? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment: The first version I wrote *did* automatically invoke shlex.quote on all interpolated values, but that breaks wildcard handling. You can see that in the examples I posted above. With the default whitespace escaping (which allows spaces in filenames), wildcard matching still works (thus the list of directories matching the ../py* pattern), but with full quoting it breaks (thus the nothing named '../py*' result). So I figured the simplest default was you can have spaces in your filenames, but otherwise you can do what you want. Now that I have the 'unquoted' conversion specifier, I'm open to tweaking that scheme to allow the same chars that shlex.quote does *plus* specifically the wildcard matching chars (i.e. '*', '?'). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment:
Considering this further, I've realised that the idea of implicit quoting for
this style of helper function is misguided on another level - the parameters to
be interpolated may not even be strings yet, so attempting to quote them would
fail:
subprocess.call(exit {}.format(1), shell=True)
1
shlex.quote(1)
Traceback (most recent call last):
File stdin, line 1, in module
File /home/ncoghlan/devel/py3k/Lib/shlex.py, line 285, in quote
if _find_unsafe(s) is None:
TypeError: expected string or buffer
I'll note that none of these problems will be unique to the new convenience API
- they're all a general reflection of the impedance mismatch between typical
shell interfaces and filenames that can contain spaces.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13238
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment: I discovered a couple of APIs that were moved from the commands module to the subprocess module in 3.0: http://docs.python.org/dev/library/subprocess#subprocess.getstatusoutput However, they have issues, especially on Windows: http://bugs.python.org/issue10197 So part of this patch would include deprecating those two interfaces in favour of the new shutil ones - the existing APIs break the subprocess promise of never invoking the shell implicitly. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment: After a bit of thought, I realised I could use the string.Formatter API to implement a custom formatter for the shell command helpers that auto-escapes whitespace while leaving the other shell metacharacters alone (so you can still interpolate paths containing wildcards, etc). People that want to bypass the autoescaping of whitespace can do the interpolation prior to the shell call, those that also want to escape shell metacharacters can use shlex.quote explicitly. Diff can be seen here: https://bitbucket.org/ncoghlan/cpython_sandbox/changeset/f19accc9a91b -- hgrepos: +85 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment:
Unfortunately, I don't think including implicit shlex.quote() calls is going to
have the effect I was originally looking for:
subprocess.call(du -hs ../py*, shell=True)
593M../py3k
577M../py3k_pristine
479M../python27
300M../python31
381M../python32
288K../python_swallowed_whole
0
subprocess.call(du -hs {}.format(shlex.quote(../py*)), shell=True)
du: cannot access `../py*': No such file or directory
1
However, tinkering with this did provide some other feels like using the
shell examples:
subprocess.call(du -hs ~/devel, shell=True)
4.5G/home/ncoghlan/devel
0
subprocess.call([du,-hs,~/devel])
du: cannot access `~/devel': No such file or directory
1
(I'm using the existing subprocess calls rather than the proposed API, since
I'm playing with this on the current hg tip without making any changes)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13238
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Éric Araujo mer...@netwok.org added the comment: [snip rationale about why shutil and not subprocess] I’m convinced (with one nit: sh in the shutil name does not ring a security alarm for me, as I understand it as “shell-like conveniences in nice, dont-do-nasty-things-with-stings Python” :) but the shell in check_shell_call does warn). Automatic call of shlex.quote is an argument in favor of the new helpers. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Éric Araujo mer...@netwok.org added the comment: s/stings/strings/ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
New submission from Nick Coghlan ncogh...@gmail.com: I've been doing a few systems administration tasks with Python recently, and shell command invocation directly via the subprocess module is annoyingly clunky (even with the new convenience APIs). Since subprocess needs to avoid the shell by default for security reasons, I suggest we add the following simple helper functions to shutil: def call(cmd, *args, **kwds): if args or kwds: cmd = cmd.format(*args, **kwds) return subprocess.call(cmd, shell=True) def check_call(cmd, *args, **kwds): if args or kwds: cmd = cmd.format(*args, **kwds) return subprocess.check_call(cmd, shell=True) def check_output(cmd, *args, **kwds): if args or kwds: cmd = cmd.format(*args, **kwds) return subprocess.check_output(cmd, shell=True) Initially posted at: http://code.activestate.com/recipes/577891-simple-invocation-of-shell-commands/ Of course, even if others agree in principle, documentation and tests are still needed before this change can go anywhere. -- components: Library (Lib) messages: 146061 nosy: ncoghlan priority: normal severity: normal status: open title: Add shell command helpers to shutil module versions: Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Alex Gaynor alex.gay...@gmail.com added the comment: These feel like a shell injection waiting to happen to me. -- nosy: +alex ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment: And that's exactly the problem - a web developer's or security auditor's shell injection is a system administrator's this language sucks. These wrappers are the kind of thing you want for shell invocations when using Python as a replacement for a shell script or rewriting something that was originally written in Perl, but they're a terrible idea if anything you're interpolating came from an untrusted data source. Currently, requiring shell=True in the arguments to the subprocess calls is considered a sufficient deterrent against people doing the wrong thing. I'm suggesting that requiring import shutil instead of import subprocess may be a similarly acceptable compromise that better serves the system administrators that choose to use Python for system automation tasks. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment: Perhaps a better idea would be to use different names, so it's clearer at the point of invocation that the shell is being invoked (and hence shell injection attacks are a potential concern). For example: shell_call check_shell_call check_shell_output That would make large applications easier to audit (just search for 'shell_') while still making life easier for sysadmins. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Chris Rebert pyb...@rebertia.com added the comment:
Is format() really the best choice here, considering that {}s already have a
meaning in the shell?
--
nosy: +cvrebert
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13238
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment: Of the 3 available options (mod style, string.Template and str.format), yes, str.format is the best choice. If people want the shell meaning of the braces, they can escape them by doubling them up in the command string. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Éric Araujo mer...@netwok.org added the comment: Why not keeping these helpers in subprocess? -- nosy: +eric.araujo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment: Initially, because I was suggesting the names shadow the subprocess convenience functions so they *had* to live in a different namespace. However, even after changing the names to explicitly include shell, I'd like to keep them away from the general subprocess functionality - these wrappers are more convenient for shell operations than the subprocess ones, but it's that very convenience that makes them potentially dangerous in larger applications that may be interpolating data that untrusted users can manipulate. Since the intended audience is system administrators working on shell-like operations, the shell utility module seems like an appropriate place for them. Both the import shutil and the shell in the names would then serve as red flags on a code review or security audit. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Antoine Pitrou pit...@free.fr added the comment:
Hum, in:
return_code = shellcmd.shell_call('ls -l {}', dirname)
listing = shellcmd.check_shell_output('ls -l {}', dirname)
...how do you know that dirname doesn't need some kind of escaping?
This is not only a security issue, but a bug. Even if security doesn't matter
on your system, your script will still break and/or do unexpected things.
Also, I don't really understand how your recipe improves things. You're just
saving one call to .format(). You would probably have the same saving by using
the % operator.
--
nosy: +pitrou
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13238
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment:
It's a flow thing. This idea was kicked off by the process of translating a
large Perl script to Python and paying attention to what the translation made
*worse*.
One of the big things it made worse was the translation of qx (quoted
executable) strings. In Perl, those are almost as neat and tidy as if you were
writing directly in the shell:
qx|ls -l $dirname|
The thought process isn't build this command and then execute it, it's run
this shell command.
Yes, you have to be careful that dirname is legal in the shell, but that
usually isn't a big problem in practice, because dirname came from a previous
listdir call, or you otherwise know that it's valid to interpolate it into the
command (and if it isn't, then the bug lies in the way 'dirname' was populated,
not in this operation).
Now, Python's never going to have implicit string interpolation, and that's
fine - we have explicit interpolation instead. A direct translation of the
above to idiomatic Python 2.7 code looks like the following:
subprocess.check_output(ls -l {}.format(dirname), shell=True)
Now, if I'm doing system administration tasks (like the script I was
translating), then I'm going to be doing that kind of thing a *lot*. I'm also
likely to be a sysadmin rather than a programmer, so rather than going Oh, I
can write a helper function for this, my natural reaction is going to be I'm
going to use a language that doesn't get in my way so much.
This proposal is aimed directly at making Python a better language for systems
administration by making shell invocation almost as easy as it is in Perl:
shutil.check_shell_output(ls -l {}, dirname)
Heck, if someone really wanted to, they could even do:
qx = shutil.check_shell_output
qx(ls -l {}, dirname)
However, this is also why I *don't* want these methods in subprocess - they put
the burden on the user to think about their data as if they were writing shell
scripts, because that data is going to get passed straight to the shell for
execution without any additional escaping. That's a reasonable idea for a shell
utility in shutil, it's not reasonable for a general purpose subprocess
manipulation utility in subprocess.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13238
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Antoine Pitrou pit...@free.fr added the comment: Yes, you have to be careful that dirname is legal in the shell, but that usually isn't a big problem in practice, because dirname came from a previous listdir call, or you otherwise know that it's valid to interpolate it into the command I don't understand. os.listdir() doesn't escape filenames for you. Having been bitten several times by almost-working shell commands which would crash when one of the 1 files being processed had a space in it (ironically, I think that was in the Python source tree with some of the old Mac directories), I think we really don't want to publish a function which encourages people to pass unescaped arguments to the shell. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Nick Coghlan ncogh...@gmail.com added the comment:
That's a fair point, but I think it actually *improves* the argument for better
helper functions, since we can have them automatically invoke shlex.quote() on
all of the arguments:
def _shell_format(cmd, args, kwds):
args = map(shlex.quote, args)
kwds = {k:shlex.quote(v) for k, v in kwds}
return cmd.format(*args, **kwds)
def _invoke_shell(func, cmd, args, kwds):
return func(_shell_format(cmd, args, kwds), shell=True)
def format_shell_call(cmd, *args, kwds):
return _shell_format(cmd, args, kwds)
def shell_call(cmd, *args, **kwds):
return _invoke_shell(subprocess.call, cmds, args, kwds)
def check_shell_call(cmd, *args, **kwds):
return _invoke_shell(subprocess.check_call, cmds, args, kwds)
def check_shell_output(cmd, *args, **kwds):
return _invoke_shell(subprocess.check_output, cmds, args, kwds)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13238
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue13238] Add shell command helpers to shutil module
Antoine Pitrou pit...@free.fr added the comment: That's a fair point, but I think it actually *improves* the argument for better helper functions Agreed :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13238 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
