assertion failure in subprocess.Popen() in case the env arg has a bad keys() method

[orenmn]

BPO	31471
Nosy	@vstinner, @serhiy-storchaka, @orenmn
PRs	bpo-31471: fix assertion failure in subprocess.Popen() on Windows, in case env has a bad keys() method #3580 [3.6] bpo-31471: Fix assertion failure in subprocess.Popen() on Windows, in case env has a bad keys() method. (GH-3580) #3584 [2.7] bpo-31471: Fix assertion failure in subprocess.Popen() on Windows, in case env has a bad keys() method. (GH-3580) #3595

^{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 2017-09-15.07:20:59.988>
created_at = <Date 2017-09-14.16:42:10.886>
labels = ['extension-modules', '3.7', 'type-crash']
title = 'assertion failure in subprocess.Popen() in case the env arg has a bad keys() method'
updated_at = <Date 2017-09-15.07:20:59.986>
user = 'https://github.com/orenmn'

bugs.python.org fields:

activity = <Date 2017-09-15.07:20:59.986>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2017-09-15.07:20:59.988>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2017-09-14.16:42:10.886>
creator = 'Oren Milman'
dependencies = []
files = []
hgrepos = []
issue_num = 31471
keywords = ['patch']
message_count = 10.0
messages = ['302181', '302199', '302200', '302203', '302231', '302235', '302236', '302237', '302238', '302239']
nosy_count = 3.0
nosy_names = ['vstinner', 'serhiy.storchaka', 'Oren Milman']
pr_nums = ['3580', '3584', '3595']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue31471'
versions = ['Python 2.7', 'Python 3.6', 'Python 3.7']

[orenmn]

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.

[serhiy-storchaka]

New changeset 0b3a87e 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. (bpo-3580)
0b3a87e

[serhiy-storchaka]

Is this bug reproducible in 2.7?

[serhiy-storchaka]

New changeset f135f62 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) (bpo-3584)
f135f62

[orenmn]

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.

[serhiy-storchaka]

Then I think it is worth to backport the fix to 2.7.

[orenmn]

OK.
but there isn't an assertion failure to test in 2.7, so is adding a test
still relevant?

[serhiy-storchaka]

It's on you.

[serhiy-storchaka]

New changeset c7f165f 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) (bpo-3595)
c7f165f

[serhiy-storchaka]

Thanks Oren!