Repository navigation
Deprecate os.popen() function #86807
Description
| BPO | 42641 |
|---|---|
| Nosy | @vstinner, @stevendaprano, @serhiy-storchaka |
| PRs |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2021-09-21.20:03:59.565>
created_at = <Date 2020-12-14.22:52:43.279>
labels = ['library', '3.10']
title = 'Deprecate os.popen() function'
updated_at = <Date 2021-09-21.20:03:59.565>
user = 'https://github.com/vstinner'bugs.python.org fields:
activity = <Date 2021-09-21.20:03:59.565>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-09-21.20:03:59.565>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2020-12-14.22:52:43.279>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 42641
keywords = ['patch']
message_count = 14.0
messages = ['383012', '383013', '383014', '383015', '383016', '383017', '383020', '383037', '383074', '383079', '383080', '383100', '383143', '402353']
nosy_count = 3.0
nosy_names = ['vstinner', 'steven.daprano', 'serhiy.storchaka']
pr_nums = ['23782']
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue42641'
versions = ['Python 3.10']
Activity
vstinner commentedon Dec 14, 2020
The os.popen() function uses a shell by default which usually leads to shell injection vulnerability.
It also has a weird API:
(*) see https://docs.python.org/dev/library/os.html#os.waitstatus_to_exitcode for the meaning of a "wait status".
IMO the subprocess module provides better and safer alternatives with a clean API. The subprocess module already explains how to replace os.popen() with subprocess:
https://docs.python.org/dev/library/subprocess.html#replacing-os-popen-os-popen2-os-popen3
In Python 2, os.popen() was deprecated since Python 2.6, but Python 3.0 removed the deprecation (commit dcf97b9, then commit f5a4292 extended os.popen() documentation again: bpo-6490).
platform.popen() existed until Python 3.8 (bpo-35345). It was deprecated since Python 3.3 (bpo-11377).
--
There is also the os.system() function which exposes the libc system() function. Should we deprecate this one as well?
vstinner commentedon Dec 14, 2020
In the past, multiple os.popen() usage have been replaced with subprocess in the stdlib to prevent the risk of shell injection:
By the way, there is an open issue bpo-21557 "os.popen & os.system lack shell-related security warnings".
See also bpo-6490 "os.popen documentation is probably wrong" (fixed).
vstinner commentedon Dec 14, 2020
os.popen() doesn't emit a ResourceWarning when close() is not called, leading to weird issues like bpo-15408.
vstinner commentedon Dec 14, 2020
The pipes module uses os.popen(): The open_r() and open_w() methods of pipes.Template are implemented with os.popen().
Multiple tests still use os.popen():
And os.popen() tests:
vstinner commentedon Dec 14, 2020
About the pipes module, see bpo-41150: "... unapplicable for processing binary data and text data non-encodable with the locale encoding".
vstinner commentedon Dec 14, 2020
See also bpo-26124: "shlex.quote and pipes.quote do not quote shell keywords".
vstinner commentedon Dec 14, 2020
About shell injection, subprocess.getstatusoutput() uses subprocess.Popen(shell=True).
https://docs.python.org/dev/library/subprocess.html#subprocess.getstatusoutput
It's done on purpose: "Execute the string cmd in a shell with Popen.check_output()".
serhiy-storchaka commentedon Dec 15, 2020
Searching os.popen in code on GitHub gives around 4.5 millions of results. Seems that most of them are with literal strings which are very specific to the program, like
They are not vulnerable to shell injection and other drawbacks of os.popen do not matter in that cases. Most of that code looks like specialized scripts rather than parts of general libraries.
Yes, some examples can be vulnerable to shell injection (although in they use cases, with restricted data and environment, they can be pretty safe). But deprecating os.popen can break millions of scripts and cause more harm than prevent bugs.
It may be better strategy to document drawbacks and limitations of os.popen and advertise alternatives.
vstinner commentedon Dec 15, 2020
Such code leaks a zombi process when the child process completes, because the parent never reads its exit status :-(
vstinner commentedon Dec 15, 2020
New changeset 7f14a37 by Victor Stinner in branch 'master':
bpo-42641: Enhance test_select.test_select() (GH-23782)
7f14a37
vstinner commentedon Dec 15, 2020
I created bpo-42648: "subprocess: add a helper/parameter to catch exec() OSError exception".
vstinner commentedon Dec 15, 2020
This sounds like a good idea in any case ;-)
stevendaprano commentedon Dec 16, 2020
Please don't deprecate os.system. For quick and dirty scripts used in trusted environments with trusted data, it is simple to use, effective and safe enough.
vstinner commentedon Sep 21, 2021
It seems like they are legit use cases for os.popen(), so I abandon my idea of deprecating it.