[issue42641] Deprecate os.popen() function

STINNER Victor added the comment: It seems like they are legit use cases for os.popen(), so I abandon my idea of deprecating it. -- resolution: -> rejected stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue42641] Deprecate os.popen() function

Steven D'Aprano added the comment: > 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,

[issue42641] Deprecate os.popen() function

STINNER Victor added the comment: > document drawbacks and limitations of os.popen and advertise alternatives. This sounds like a good idea in any case ;-) -- ___ Python tracker

[issue42641] Deprecate os.popen() function

STINNER Victor added the comment: I created bpo-42648: "subprocess: add a helper/parameter to catch exec() OSError exception". -- ___ Python tracker ___

[issue42641] Deprecate os.popen() function

STINNER Victor added the comment: New changeset 7f14a3756b61272cc15f24302589874b125c2f04 by Victor Stinner in branch 'master': bpo-42641: Enhance test_select.test_select() (GH-23782) https://github.com/python/cpython/commit/7f14a3756b61272cc15f24302589874b125c2f04 --

[issue42641] Deprecate os.popen() function

Change by STINNER Victor : -- keywords: +patch pull_requests: +22640 stage: -> patch review pull_request: https://github.com/python/cpython/pull/23782 ___ Python tracker ___

[issue42641] Deprecate os.popen() function

STINNER Victor added the comment: > 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 :-( --

[issue42641] Deprecate os.popen() function

Serhiy Storchaka added the comment: 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

[issue42641] Deprecate os.popen() function

STINNER Victor added the comment: 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()".

[issue42641] Deprecate os.popen() function

STINNER Victor added the comment: See also bpo-26124: "shlex.quote and pipes.quote do not quote shell keywords". -- ___ Python tracker ___

[issue42641] Deprecate os.popen() function

STINNER Victor added the comment: About the pipes module, see bpo-41150: "... unapplicable for processing binary data and text data non-encodable with the locale encoding". -- ___ Python tracker

[issue42641] Deprecate os.popen() function

STINNER Victor added the comment: 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:

[issue42641] Deprecate os.popen() function

STINNER Victor added the comment: os.popen() doesn't emit a ResourceWarning when close() is not called, leading to weird issues like bpo-15408. -- ___ Python tracker ___

[issue42641] Deprecate os.popen() function

STINNER Victor added the comment: 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

[issue42641] Deprecate os.popen() function

New submission from STINNER Victor : 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