Skip to content

Commit

Permalink
bpo-36046: posix_spawn() doesn't support uid/gid (GH-16384)
Browse files Browse the repository at this point in the history
* subprocess.Popen now longer uses posix_spawn() if uid, gid or gids are set.
* test_subprocess: add "nobody" and "nfsnobody" group names for test_group().
* test_subprocess: test_user() and test_group() are now also tested with close_fds=False.
  • Loading branch information
vstinner authored and Yhg1s committed Sep 25, 2019
1 parent 1dc1acb commit faca855
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 34 deletions.
5 changes: 4 additions & 1 deletion Lib/subprocess.py
Expand Up @@ -1681,7 +1681,10 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
and (p2cread == -1 or p2cread > 2)
and (c2pwrite == -1 or c2pwrite > 2)
and (errwrite == -1 or errwrite > 2)
and not start_new_session):
and not start_new_session
and gid is None
and gids is None
and uid is None):
self._posix_spawn(args, executable, env, restore_signals,
p2cread, p2cwrite,
c2pread, c2pwrite,
Expand Down
71 changes: 38 additions & 33 deletions Lib/test/test_subprocess.py
Expand Up @@ -1589,7 +1589,7 @@ def test_run_with_shell_timeout_and_capture_output(self):


def _get_test_grp_name():
for name_group in ('staff', 'nogroup', 'grp'):
for name_group in ('staff', 'nogroup', 'grp', 'nobody', 'nfsnobody'):
if grp:
try:
grp.getgrnam(name_group)
Expand Down Expand Up @@ -1768,24 +1768,27 @@ def test_user(self):
test_users.append(name_uid)

for user in test_users:
with self.subTest(user=user):
try:
output = subprocess.check_output(
[sys.executable, "-c",
"import os; print(os.getuid())"],
user=user)
except PermissionError: # errno.EACCES
pass
except OSError as e:
if e.errno not in (errno.EACCES, errno.EPERM):
raise
else:
if isinstance(user, str):
user_uid = pwd.getpwnam(user).pw_uid
# posix_spawn() may be used with close_fds=False
for close_fds in (False, True):
with self.subTest(user=user, close_fds=close_fds):
try:
output = subprocess.check_output(
[sys.executable, "-c",
"import os; print(os.getuid())"],
user=user,
close_fds=close_fds)
except PermissionError: # (EACCES, EPERM)
pass
except OSError as e:
if e.errno not in (errno.EACCES, errno.EPERM):
raise
else:
user_uid = user
child_user = int(output)
self.assertEqual(child_user, user_uid)
if isinstance(user, str):
user_uid = pwd.getpwnam(user).pw_uid
else:
user_uid = user
child_user = int(output)
self.assertEqual(child_user, user_uid)

with self.assertRaises(ValueError):
subprocess.check_call([sys.executable, "-c", "pass"], user=-1)
Expand All @@ -1809,23 +1812,25 @@ def test_group(self):
group_list.append(name_group)

for group in group_list + [gid]:
with self.subTest(group=group):
try:
output = subprocess.check_output(
[sys.executable, "-c",
"import os; print(os.getgid())"],
group=group)
except OSError as e:
if e.errno != errno.EPERM:
raise
else:
if isinstance(group, str):
group_gid = grp.getgrnam(group).gr_gid
# posix_spawn() may be used with close_fds=False
for close_fds in (False, True):
with self.subTest(group=group, close_fds=close_fds):
try:
output = subprocess.check_output(
[sys.executable, "-c",
"import os; print(os.getgid())"],
group=group,
close_fds=close_fds)
except PermissionError: # (EACCES, EPERM)
pass
else:
group_gid = group
if isinstance(group, str):
group_gid = grp.getgrnam(group).gr_gid
else:
group_gid = group

child_group = int(output)
self.assertEqual(child_group, group_gid)
child_group = int(output)
self.assertEqual(child_group, group_gid)

# make sure we bomb on negative values
with self.assertRaises(ValueError):
Expand Down

0 comments on commit faca855

Please sign in to comment.