Skip to content

Commit

Permalink
twisted#12068: Fix incorrect env handling in spawnProcess on Posix wh…
Browse files Browse the repository at this point in the history
…en posix_spawnp is used. (twisted#12069)
  • Loading branch information
glyph committed Jan 24, 2024
2 parents a4ab53c + 90c56ab commit 11c9f58
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/twisted/internet/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,7 @@ def _trySpawnInsteadOfFork(
else:
fdState.append((eachFD, isCloseOnExec))
if environment is None:
environment = {}
environment = os.environ

setSigDef = [
everySignal
Expand Down
127 changes: 127 additions & 0 deletions src/twisted/internet/test/test_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from twisted.python.filepath import FilePath, _asFilesystemBytes
from twisted.python.log import err, msg
from twisted.python.runtime import platform
from twisted.test.test_process import Accumulator
from twisted.trial.unittest import SynchronousTestCase, TestCase

# Get the current Python executable as a bytestring.
Expand Down Expand Up @@ -1001,6 +1002,132 @@ def launchProcessAndWait(reactor):
hamcrest.equal_to(["process already removed as desired"]),
)

def checkSpawnProcessEnvironment(self, spawnKwargs, expectedEnv, usePosixSpawnp):
"""
Shared code for testing the environment variables
present in the spawned process.
The spawned process serializes its environ to stderr or stdout (depending on usePTY)
which is checked against os.environ of the calling process.
"""
p = Accumulator()
d = p.endedDeferred = Deferred()

reactor = self.buildReactor()
reactor._neverUseSpawn = not usePosixSpawnp

reactor.callWhenRunning(
reactor.spawnProcess,
p,
pyExe,
[
pyExe,
b"-c",
networkString(
"import os, sys; "
"env = dict(os.environ); "
# LC_CTYPE is set by python, see https://peps.python.org/pep-0538/
'env.pop("LC_CTYPE", None); '
'env.pop("__CF_USER_TEXT_ENCODING", None); '
"sys.stderr.write(str(sorted(env.items())))"
),
],
usePTY=self.usePTY,
**spawnKwargs,
)

def shutdown(ign):
reactor.stop()

d.addBoth(shutdown)

self.runReactor(reactor)

expectedEnv.pop("LC_CTYPE", None)
expectedEnv.pop("__CF_USER_TEXT_ENCODING", None)
self.assertEqual(
bytes(str(sorted(expectedEnv.items())), "utf-8"),
p.outF.getvalue() if self.usePTY else p.errF.getvalue(),
)

def checkSpawnProcessEnvironmentWithPosixSpawnp(self, spawnKwargs, expectedEnv):
return self.checkSpawnProcessEnvironment(
spawnKwargs, expectedEnv, usePosixSpawnp=True
)

def checkSpawnProcessEnvironmentWithFork(self, spawnKwargs, expectedEnv):
return self.checkSpawnProcessEnvironment(
spawnKwargs, expectedEnv, usePosixSpawnp=False
)

@onlyOnPOSIX
def test_environmentPosixSpawnpEnvNotSet(self):
"""
An empty environment is passed to the spawned process, when the default value of the C{env}
is used. That is, when the C{env} argument is not explicitly set.
In this case posix_spawnp is used as the backend for spawning processes.
"""
return self.checkSpawnProcessEnvironmentWithPosixSpawnp({}, {})

@onlyOnPOSIX
def test_environmentForkEnvNotSet(self):
"""
An empty environment is passed to the spawned process, when the default value of the C{env}
is used. That is, when the C{env} argument is not explicitly set.
In this case fork+execvpe is used as the backend for spawning processes.
"""
return self.checkSpawnProcessEnvironmentWithFork({}, {})

@onlyOnPOSIX
def test_environmentPosixSpawnpEnvNone(self):
"""
The parent process environment is passed to the spawned process, when C{env} is set to
C{None}.
In this case posix_spawnp is used as the backend for spawning processes.
"""
return self.checkSpawnProcessEnvironmentWithPosixSpawnp(
{"env": None}, os.environ
)

@onlyOnPOSIX
def test_environmentForkEnvNone(self):
"""
The parent process environment is passed to the spawned process, when C{env} is set to
C{None}.
In this case fork+execvpe is used as the backend for spawning processes.
"""
return self.checkSpawnProcessEnvironmentWithFork({"env": None}, os.environ)

@onlyOnPOSIX
def test_environmentPosixSpawnpEnvCustom(self):
"""
The user-specified environment without extra variables from parent process is passed to the
spawned process, when C{env} is set to a dictionary.
In this case posix_spawnp is used as the backend for spawning processes.
"""
return self.checkSpawnProcessEnvironmentWithPosixSpawnp(
{"env": {"MYENV1": "myvalue1"}},
{"MYENV1": "myvalue1"},
)

@onlyOnPOSIX
def test_environmentForkEnvCustom(self):
"""
The user-specified environment without extra variables from parent process is passed to the
spawned process, when C{env} is set to a dictionary.
In this case fork+execvpe is used as the backend for spawning processes.
"""
return self.checkSpawnProcessEnvironmentWithFork(
{"env": {"MYENV1": "myvalue1"}},
{"MYENV1": "myvalue1"},
)


globals().update(ProcessTestsBuilder.makeTestCaseClasses())

Expand Down
1 change: 1 addition & 0 deletions src/twisted/newsfragments/12068.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
twisted.internet.process.Process, used by ``reactor.spawnProcess``, now copies the parent environment when the `env=None` argument is passed on Posix systems and ``os.posix_spawnp`` is used internally.

0 comments on commit 11c9f58

Please sign in to comment.