This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: shlex.quote and pipes.quote do not quote shell keywords
Type: behavior Stage: patch review
Components: Documentation, Library (Lib) Versions: Python 3.6
process
Status: open Resolution: wont fix
Dependencies: Superseder:
Assigned To: docs@python Nosy List: Charles Daffern, docs@python, eric.araujo, fdrake, r.david.murray, serhiy.storchaka
Priority: normal Keywords: easy, patch

Created on 2016-01-15 13:14 by Charles Daffern, last changed 2022-04-11 14:58 by admin.

Pull Requests
URL Status Linked Edit
PR 13333 open Windson Yang, 2019-05-15 01:34
Messages (9)
msg258292 - (view) Author: Charles Daffern (Charles Daffern) Date: 2016-01-15 13:14
The shlex.quote and pipes.quote functions do not quote shell keywords.

Example shell keywords: done, time, coproc, while

Presumably the intent of the quote functions is to prevent the resulting string from altering the syntax of the script it is inserted into, which is why I think these need to be quoted.

We can't possibly know every shell keyword, so the only sensible thing to do here is to quote everything.
msg258298 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-01-15 15:13
Could you please provide an example?
msg258299 - (view) Author: Charles Daffern (Charles Daffern) Date: 2016-01-15 15:37
It's definitely a corner case (in argv[0] position + is keyword), but here's an example:

>>> import subprocess
>>> import shlex
>>> subprocess.call(shlex.quote("done"), shell=True)
/bin/sh: 1: Syntax error: "done" unexpected
2

The expected output of this would be:

/bin/sh: 1: done: not found
127

This would be the output if shlex.quote("done") returned "'done'" or r'\done' or any other combination of escaped/quoted characters where the keyword would otherwise be.
msg258300 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-01-15 15:58
This does not seem to be a serious problem. In any case the command is failed. And usually argv[0] is predefined command name, not arbitrary user input. To be sure that it is existing program, you can use shutil.which().

I would close this issue as "won't fix".
msg258303 - (view) Author: Charles Daffern (Charles Daffern) Date: 2016-01-15 16:15
In that case, should the documentation specify that shlex.quote is unsuitable for quoting command names?
msg258304 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-01-15 16:21
May be. But only if this doesn't make the documentation too verbose.
msg258318 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2016-01-15 18:44
It's not at all obvious that the intention is to ensure such an argument should be treated only as a command external to the shell.

If an application really wants to ensure the command is not handled as a shell built-in, it should use shell=False.

Making this clear in the documentation is reasonable.
msg258322 - (view) Author: Charles Daffern (Charles Daffern) Date: 2016-01-15 19:29
>To be sure that it is existing program, you can use shutil.which()

I'd like to clear this up a little because this is worded as if shutil.which()'s success implies that the shell will not fail.

Here is the setup to demonstrate:

>>> import os, shlex, shutil, subprocess
>>> open("do", "w").write("#!/bin/sh\necho Something is being done...")
__main__:1: ResourceWarning: unclosed file <_io.TextIOWrapper name='do' mode='w' encoding='UTF-8'>
41
>>> os.chmod("do", 0o700)


Here is the behaviour using shlex.quote:

>>> subprocess.call(shlex.quote("do"), shell=True, env={'PATH': '.'})
/bin/sh: 1: Syntax error: "do" unexpected
2


Here is the behaviour when quoting properly:

>>> subprocess.call("'do'", shell=True, env={'PATH': '.'})
Something is being done...
0


Here is the output of shutil.which:

>>> shutil.which("do", path=".")
'./do'


So checking shutil.which()'s success or failure will not guard against this case (though using its output would work around the problem).

>It's not at all obvious that the intention is to ensure such an argument should be treated only as a command external to the shell.
>
>If an application really wants to ensure the command is not handled as a shell built-in, it should use shell=False.

The shell will still search builtins if the argument is quoted, it just won't search for keywords. So, a quoted "bind", "shopt" or "jobs" will still work, but a quoted "case", "fi" or "done" will cause the shell to search for a command of that name rather than treating it as syntax.

Looking at the source, shlex.quote's refusal to quote certain arguments appears to be intentional. I would rather it quote slightly more carefully than necessary, than quote something incorrectly.
msg258550 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-01-18 21:48
Quote is for quoting shell special characters, not command names.  It would be fairly straightforward to add that to the docs ("Return a version of the string s will with all shell special characters (including whitespace) escaped according to shell escaping rules."  Wordier, but more accurate, and not *too* much longer.
History
Date User Action Args
2022-04-11 14:58:26adminsetgithub: 70312
2019-05-15 01:34:47Windson Yangsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request13245
2016-01-18 21:48:42r.david.murraysetnosy: + r.david.murray
messages: + msg258550
2016-01-15 19:29:50Charles Daffernsetmessages: + msg258322
2016-01-15 18:44:29fdrakesetnosy: + fdrake
messages: + msg258318
2016-01-15 16:21:39serhiy.storchakasetassignee: docs@python
components: + Documentation

keywords: + easy
nosy: + docs@python
messages: + msg258304
stage: needs patch
2016-01-15 16:15:51Charles Daffernsetmessages: + msg258303
2016-01-15 15:58:13serhiy.storchakasetresolution: wont fix
messages: + msg258300
2016-01-15 15:37:27Charles Daffernsetmessages: + msg258299
2016-01-15 15:13:26serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg258298
2016-01-15 14:28:42SilentGhostsetnosy: + eric.araujo

components: + Library (Lib), - Extension Modules
versions: - Python 2.7, Python 3.2, Python 3.3, Python 3.4, Python 3.5
2016-01-15 13:14:41Charles Dafferncreate