Not guarding env value in subprocess.py cause python crash when certain env variable is set to None

[tianrui-wei]

Bug report

A minimal bug can be found using the following file

from subprocess import Popen, PIPE

env = {}
env["a"] = None
process = Popen(['echo', '"hello"'], stdout=PIPE, stderr=PIPE, env = env)
stdout, stderr = process.communicate()

Your environment

M1 MacOS Monterey, python 3.10
x86_64 Ubuntu 20.04 LTS, python 3.11 (installed via pyenv)

• PR: gh-99238: Fix crashes caused by lack of checking in subprocess module #99239

• PR: gh-99238: clarify the type of the env dict. #99253

• PR: [3.11] gh-99238: clarify the type of the env dict. (GH-99253) #99263

• PR: [3.10] gh-99238: clarify the type of the env dict. (GH-99253) #99264

[gpshead]

Environment dictionaries are string to string maps. - https://docs.python.org/3/library/os.html#os.environ None is not a valid value. If code wants to define an environment variable with no value it should set it to an empty string. env["a"] = "".

While the subprocess module docs do not explicitly say this, the intent was always that this dict matched the type you'd find in os.environ. We could improve the subprocess docs to make this clear.

[tianrui-wei]

Do you think there's a stricter type checking approach we could take rather than the PR I proposed? I ran into a library that does this and unfortunately took quite a while to debug. I'd love your feedback on this matter, thanks.

[gpshead]

We shouldn't be adding assertions into the standard library because of uncommon bugs in calling code. However if you use a static analysis tool like pytype or mypy it should catch this one as the subprocess API definition in https://github.com/python/typeshed/blob/main/stdlib/subprocess.pyi#L73 that python type checkers is correct for env. None is not allowed.