Do you have any data about the “folks hate the list input format so much” claim?
Is it really hate, or is it just that people don’t understand why it’s preferrable?
8 Likes
For the record, is this talking about the CommandLineToArgvW function ? Can it be considered “standard” as far as Windows command line parsing is concerned?
It’s not even used by the builtin echo command.
1 Like
I can’ see this being significant number of people. I see shell=True all the time and it’s always out of nativity (often fuelled by experience in other languages that don’t support anything better). The subprocess == shell script misconception is one that we’re apparently just born with (former me included). (And what sane developer would knowingly risk shell injection just to save a few characters.)
2 Likes
Please also note that in most if not all cases where people use shell=True (see my next point), there is already a safer alternative - the list format. You should probably also include somewhere a discussion on why adding t-string support is a better solution than simply directing people towards list format. It feels to me that the argument here is “because t-strings are a shiny new thing, they might get the attention of some people who won’t listen to arguments that the list format is safer”. Whch is a pretty weak argument 
In the rework (or for that matter, as a reply in this discussion), please provide evidence for this claim. My experience is that nearly everyone uses the list format without shell=True - enough that I have essentially no evidence that shell=True is even used, much less why people use it.
Please include a note that this is a workaround for a flaw in the t-string approach, and the list approach avoids the problem in the first place.
This is a straw man. The echo command isn’t a standalone command on Windows, it’s a shell builtin. As such, it has non-standard argument evaluation rules. The problem here is that the t-string proposal still passes a string to the shell, rather than running a command directly, and it’s therefore liable to issues when shell builtins (which have the ability to use non-standard syntax on every shell and operating system) override normal executables. The echo command is simply a particular case of this (and a fairly pointless one, as why wouldn’t you just use print()?)
As a demonstration, here’s subprocess.run in list format correctly handling your case, using the 3rd party echo command provided by the uutils suite of tools.
>>> subprocess.run(["uutils", "echo", "'", '"', "'single quoted arg'", '"double quoted arg"'])
' " 'single quoted arg' "double quoted arg"
The level of portability we should be aiming for here is the same as the list format - not some designed-to-fail extreme case using a command that won’t be relevant in real world code.
Finally, can you add a section to the “How to Teach This” part of the PEP which explains how we will ensure that people understand that t-string support is still less safe than using the list format, and when developers have a choice they should prefer list format. This will clearly be difficult to achieve, because the PEP itself currently fails to be clear that what it’s proposing is only intended for cases where users cannot[1] use list format - if the PEP itself can’t explain this clearly, how do we hope to teach it properly?
or will not, in spite of the security risks ↩︎
8 Likes
Looking here I see lots of shell=True:
There are many cases where it is just a constant string and could use .split() rather than shell=True but it probably doesn’t matter much either way.
Common cases are also shell commands with redirection like |, <<<, > and $ENV variables. I guess shell syntax is just convenient for those things in a way that the subprocess module has never been.
4 Likes
Demonstrating the existing lack of portability even for the list format was the point of the echo example - if that kind of misbehaviour is considered acceptable on Windows for list inputs, then it probably isn’t reasonable to expect we’re going to be able to avoid similar problems for t-strings.
The situation on POSIX systems is quite different: the POSIX process spawning APIs accept the command arguments as an array rather than as a string that needs splitting, and the shell quoting rules are part of the spec for /bin/sh rather than being something each shell makes up for itself.
The end result is that this PEP is starting to feel like the locale coercion & encoding override PEPs to me: the proposal itself should still end up being relatively simple, but the background needed to fully explain the quirks of the status quo, and the lengths people go to to try and emulate native shell invocation support, is far more involved than what we included in this initial version.
I’ve also realised I was remiss in not including any xonsh references in the PEP. xonsh approaches the same problem space from the other direction by aiming to provide a shell environment that feels more like Python than shell, without losing the simple command invocation features of a native shell.
2 Likes
This one should use shutil.copyfileobj or directly assign file descriptors
Curiously i took note that shutil does not expose fast copying in that helper
After I posted I realized a better approach for pipelining is already documented: subprocess — Subprocess management — Python 3.13.3 documentation
(My actual use case involves subclassing “BinaryPiper” witha TeeBinaryPiper that records the data passed from one subprocess to another.)
1 Like
I have been thinking this for days but wasn’t sure how to best frame it politely. This seems like a solution in search of a problem instead of a problem instead of a solution.
1 Like
The current subprocess module has a “run()” method with 14 named arguments, at least two of which are redundant (universal_newlines and capture_output) and passes additional arguments onto Popen which has 25 named arguments.
The args argument is functionally overloaded, in that the user can pass:
- a string with shell=True. Superfically convenient but just a bad idea. I
- a sequence. (I usually use tuples, but lists seem more popular for some reason). Much better.
Unless I’m missing something, this proposal suggests adding a third overload of *args" that provides no advantage of just passing the sequence. Instead of simple to understand datatypes (tuple & list) that have existed since Python 1.0 it suggests additional a data type (t-strings) that does not exist yet in current Python. The subprocess docs would then have to be updated to thoroughly explain all this.
There should be one-- and preferably only one --obvious way to do it.
1 Like
For certain types of scripting, the advantage is that reading the code that invokes it becomes easier. Here are some before/afters from a script I’m actively using:[1]
run([sys.executable, "-m", "pymsbuild", "wheel"])
run([sys.executable, "scripts/generate-nuget-index.py", LAYOUT / "bundled" / "index.json"])
run(["py-manager.exe", "install", "-v", "-f", "--download", TEMP / "bundle", "default"])
run(t"{sys.executable} -m pymsbuild wheel")
run(rt"{sys.executable} scripts/generate-nuget-index.py {LAYOUT}\bundled\index.json")
run(rt"py-manager.exe install -v -f --download {TEMP}\bundle default")
Arguing “there should be one … way” here is the same as the arguments against f-strings, saying that percent formatting is good enough. We’ve already rejected it for cases where inline substitutions are an improvement in readability.
Now, the fact that POSIX developers are already allergic to passing what seems to be a single string is clearly a vote against, at least within groups where POSIX devs are the majority. But we also already have a “standard” way to split a single string into arguments for passing as an array, so it’s actually not hard to handle that case (whether it works reliably or not is separate, since that question already exists for our current API).
Unfortunately not, since UCRT uses its own algorithm (which is the one that turns into main(int argc, char **argv) arguments). We can at least see the implementation of that algorithm (it’s included in the Windows SDK).
The main problem is of course for shell=True, where we should be combining the arguments and quoting them following cmd.exe’s own algorithm, which isn’t based on the UCRT algorithm, so they are passed to the shell as a single command. The problem then is knowing what the caller meant (did they use & to run two commands or should it be escaped?), and since we can’t know that, we don’t try. t-strings could help, by distinguishing between argument (& gets escaped) and command (& doesn’t get escaped), without putting pressure on the developer to handle it all themselves.
But ultimately, shell=True just needs to be avoided most of the time. Virtually everything you might need the shell commands for can be done with pathlib or os, typically faster and certainly more portable.
The cases where t-strings help without shell=True do exist - intentional quotes vs. unintentional quotes - but those cases are reliable enough with a list and (as I’ve learned) are only portable if you use a list. So the main thing t-strings could bring here are portable string commands with substitutions, hence my examples above where I would argue it’s an improvement (incremental, perhaps, though typing is much nicer without doing ', ' over and over).
Some additional
env=andcwd=arguments omitted. ↩︎
5 Likes
Instead of adding overloads to run
Perhaps start by just adding support to shlex.split
Once established subprocess can used it to provide convenience
1 Like
Because you can append/extend a list.
I use that a lot for complex command line building where args are oprional. Think compilers with or without debug etc.
1 Like
Sure, I use a list when dynamically building arguments and a tuple when they’re fixed. The examples of fixed arguments seem to use lists for some reason.
I always use a list for things like this because of the case of a one-element tuple requiring a comma, and I’ve forgotten those by mistake, especially when I go back and edit existing code. I don’t see any harm in using a list.
2 Likes
In general during the discussion of PEP 750 I had imagined that the intended future was one in which it would be a type error to use f-strings in situations where a t-string was needed so e.g. you have functions like:
def sh(cmd: Template):
"""Run command with safe interpolation."""
...
def sh_unsafe(cmd: str):
"""Run command without safe interpolation."""
...
Then the idea is that the type checker and the function name etc clearly show what is the safe approach. Someone who uses an f-string when they should be using a t-string gets a type error both statically and at runtime:
sh(t"echo {name}") # fine
sh(f"echo {name}") # type error
sh_unsafe(f"echo {name}") # ok, but visibly unsafe
The simple way to miss this benefit is by making a function that accepts both:
def sh(cmd: Template | str):
"""Possibly safe or possibly not."""
...
I can see the desire to do it this way because subprocess.run etc effectively already have this by allowing list | str and so allowing t-strings makes it possible to say “replace f-strings with t-strings” as an easy transition but it also misses the opportunity to aim for what might be a better API in the long term.
Maybe given backwards compatibility this is the best approach for the subprocess module but I think it maybe sets a bad precedent for API design as a first application of t-strings.
9 Likes
Especially as
- it’s probably easy to mistype a t-string as a f-string; such a simple typo inducing a security issue silently would be very bad
- beginners may not grasp the fundamental difference between both notations
The fact t-strings look very much like normal strings (or f-strings) may not be a good thing after all.
6 Likes
It’s worth noting that the subprocess.run API is actually more complex than that - there’s no clear definition of what will happen if a list is passed with shell=True, and with shell=False, a string argument must be a bare program name with no arguments.
So the logic isn’t simply “replace t-strings with f-strings”, it’s actually a lot more complex than that:
- With
shell=True, replacing f-strings with t-strings breaks any cross-platform portability the original had[1]. This might be fine, but it might also be a silent breakage - With
shell=False, the f-string can only be constructing an executable name anyway, and not only do t-strings not add any value there, but they actually have incorrect semantics, because~/my apps/"is not the same executable as~/"my apps"/'"'…[2] - And with
shell=False, t-strings are actually likely to be seen as a “more readable” version of the list format. Which they definitely aren’t, because on POSIX systems a raw list is passed to the OS directly, so the t-string translation is at best unnecessary, and at worst gives different results. And on non-POSIX systems (Windows) the list is processed back to a command string using different rules, so the t-string needs to round-trip without change, which is far from guaranteed.
I’m sure we can define some precise rules that explain what will happen in every potential situation - but do we really want something that has the potential to be that unintuitive? It just feels like a huge footgun, and presenting it as “safer” is optimistic at best, and actually flat-out misleading in reality.
8 Likes
I don’t know enough about all the different combinations and possibilities here (especially Windows) but how about this as a suggestion:
- Deprecate (in the soft sense) any use of
runexcept for with a list of strings and withshell=Falseand also deprecate the shell parameter. - Add a new function
shrunwhose argument must be a t-string.
The intended usage is then:
def run(cmd: list[str], ...):
"""Launch subprocess"""
...
def shrun(cmd: Template, ...):
"""Run command in shell with interpolation"""
...
Then the suggestion is that anyone doing
run(f'echo {name}', shell=True)
should do
shrun(t'echo {name}')
2 Likes