Deprecate os.popen() function

[vstinner]

BPO	42641
Nosy	@vstinner, @stevendaprano, @serhiy-storchaka
PRs	bpo-42641: Enhance test_select.test_select() #23782

^{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']

[vstinner]

The os.popen() function uses a shell by default which usually leads to shell injection vulnerability.

It also has a weird API:

• closing the file waits until the process completes.
• close() returns a "wait status" (*) not a "returncode"

(*) 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]

In the past, multiple os.popen() usage have been replaced with subprocess in the stdlib to prevent the risk of shell injection:

• bpo-22636: ctypes modules
• bpo-22637: uuid module

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]

os.popen() doesn't emit a ResourceWarning when close() is not called, leading to weird issues like bpo-15408.

[vstinner]

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():

• test_select: SelectTestCase.test_select()
• test_posix: PosixTester.test_getgroups()
• test_os: EnvironTests._test_underlying_process_env()
• test_os: EnvironTests.test_update2()

And os.popen() tests:

• test_os: EnvironTests.test_os_popen_iter()
• test_popen

[vstinner]

About the pipes module, see bpo-41150: "... unapplicable for processing binary data and text data non-encodable with the locale encoding".

[vstinner]

See also bpo-26124: "shlex.quote and pipes.quote do not quote shell keywords".

[vstinner]

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]

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

    check2 = os.popen('grep "net\.ipv4\.ip_forward" /etc/sysctl.conf /etc/sysctl.d/*').read()

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]

check2 = os.popen('grep "net\.ipv4\.ip_forward" /etc/sysctl.conf /etc/sysctl.d/*').read()

Such code leaks a zombi process when the child process completes, because the parent never reads its exit status :-(

[vstinner]

New changeset 7f14a37 by Victor Stinner in branch 'master':
bpo-42641: Enhance test_select.test_select() (GH-23782)
7f14a37

[vstinner]

I created bpo-42648: "subprocess: add a helper/parameter to catch exec() OSError exception".

[vstinner]

document drawbacks and limitations of os.popen and advertise alternatives.

This sounds like a good idea in any case ;-)

[stevendaprano]

There is also the os.system() function which exposes the libc system() function. Should we deprecate this one as well?

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]

It seems like they are legit use cases for os.popen(), so I abandon my idea of deprecating it.