classification
Title: assertion failure in subprocess.Popen() in case the env arg has a bad keys() method
Type: crash Stage: resolved
Components: Extension Modules Versions: Python 3.7, Python 3.6, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Oren Milman, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2017-09-14 16:42 by Oren Milman, last changed 2017-09-15 07:20 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 3580 merged Oren Milman, 2017-09-14 17:40
PR 3584 merged python-dev, 2017-09-14 19:30
PR 3595 merged Oren Milman, 2017-09-15 07:09
Messages (10)
msg302181 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-09-14 16:42
The following code causes an assertion failure on Windows:
class BadEnv(dict):
    keys = None

import subprocess

import sys

subprocess.Popen([sys.executable, "-c", "pass"], env=BadEnv())


this is because getenvironment() (in Modules/_winapi.c) calls PyMapping_Values()
immediately after calling PyMapping_Keys().
calling PyMapping_Values() ultimately leads to calling _PyObject_FastCallDict(),
which does 'assert(!PyErr_Occurred());'.
thus, in case of an error in PyMapping_Keys(), the assertion fails.
msg302199 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-14 19:30
New changeset 0b3a87ef54a0112b74e8a1d8c6f87d10db4239ab by Serhiy Storchaka (Oren Milman) in branch 'master':
bpo-31471: Fix assertion failure in subprocess.Popen() on Windows, in case env has a bad keys() method. (#3580)
https://github.com/python/cpython/commit/0b3a87ef54a0112b74e8a1d8c6f87d10db4239ab
msg302200 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-14 19:32
Is this bug reproducible in 2.7?
msg302203 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-14 19:56
New changeset f135f62cfd1529cbb9326e53728a22afd05b6bc3 by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
[3.6] bpo-31471: Fix assertion failure in subprocess.Popen() on Windows, in case env has a bad keys() method. (GH-3580) (#3584)
https://github.com/python/cpython/commit/f135f62cfd1529cbb9326e53728a22afd05b6bc3
msg302231 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-09-15 04:53
in 2.7 getenvironment() is in PC/_subprocess.c, and it also calls PyMapping_Values()
immediately after calling PyMapping_Keys().
however, _PyObject_FastCallDict() doesn't exist here.
in case of an error in both PyMapping_Keys() and PyMapping_Values(), the
error in PyMapping_Values() just overwrites the error in PyMapping_Keys().

but I haven't gone over all of the code that could be run as part of
PyMapping_Values(), so I am not sure whether something could go wrong in case PyMapping_Keys() failed.
msg302235 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-15 06:10
Then I think it is worth to backport the fix to 2.7.
msg302236 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-09-15 06:31
OK.
but there isn't an assertion failure to test in 2.7, so is adding a test
still relevant?
msg302237 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-15 06:49
It's on you.
msg302238 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-15 07:20
New changeset c7f165fe652651c32833245fc902c790a4f173fa by Serhiy Storchaka (Oren Milman) in branch '2.7':
[2.7] bpo-31471: Fix assertion failure in subprocess.Popen() on Windows, in case env has a bad keys() method. (GH-3580) (#3595)
https://github.com/python/cpython/commit/c7f165fe652651c32833245fc902c790a4f173fa
msg302239 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-15 07:20
Thanks Oren!
History
Date User Action Args
2017-09-15 07:20:59serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg302239

stage: patch review -> resolved
2017-09-15 07:20:13serhiy.storchakasetmessages: + msg302238
2017-09-15 07:09:47Oren Milmansetpull_requests: + pull_request3586
2017-09-15 06:49:44serhiy.storchakasetmessages: + msg302237
2017-09-15 06:31:04Oren Milmansetmessages: + msg302236
2017-09-15 06:10:27serhiy.storchakasetmessages: + msg302235
versions: + Python 2.7
2017-09-15 04:53:18Oren Milmansetmessages: + msg302231
2017-09-14 19:56:36serhiy.storchakasetmessages: + msg302203
2017-09-14 19:32:06serhiy.storchakasetmessages: + msg302200
versions: + Python 3.6
2017-09-14 19:30:38python-devsetpull_requests: + pull_request3575
2017-09-14 19:30:29serhiy.storchakasetmessages: + msg302199
2017-09-14 17:40:43Oren Milmansetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request3570
2017-09-14 16:54:52serhiy.storchakasetstage: needs patch
2017-09-14 16:47:41vstinnersetnosy: + vstinner, serhiy.storchaka
2017-09-14 16:42:10Oren Milmancreate