Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pass_fds parameter to PtyProcess:spawn() #49

Merged
merged 1 commit into from
Jan 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions ptyprocess/ptyprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def __init__(self, pid, fd):
@classmethod
def spawn(
cls, argv, cwd=None, env=None, echo=True, preexec_fn=None,
dimensions=(24, 80)):
dimensions=(24, 80), pass_fds=()):
'''Start the given command in a child process in a pseudo terminal.

This does all the fork/exec type of stuff for a pty, and returns an
Expand All @@ -190,6 +190,10 @@ def spawn(

Dimensions of the psuedoterminal used for the subprocess can be
specified as a tuple (rows, cols), or the default (24, 80) will be used.

By default, all file descriptors except 0, 1 and 2 are closed. This
behavior can be overridden with pass_fds, a list of file descriptors to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: you document it as a list, but it will only work with a tuple, because you concatenate it to a tuple (pass_fds + (exec_err_pipe_write,)). Let's cast it to a set, like subprocess does; that should work with any sequence, and also eliminates any odd behaviour with duplicate entries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right!

keep open between the parent and the child.
'''
# Note that it is difficult for this method to fail.
# You cannot detect if the child process cannot start.
Expand Down Expand Up @@ -255,12 +259,14 @@ def spawn(

# Do not allow child to inherit open file descriptors from parent,
# with the exception of the exec_err_pipe_write of the pipe
# and pass_fds.
# Impose ceiling on max_fd: AIX bugfix for users with unlimited
# nofiles where resource.RLIMIT_NOFILE is 2^63-1 and os.closerange()
# occasionally raises out of range error
max_fd = min(1048576, resource.getrlimit(resource.RLIMIT_NOFILE)[0])
os.closerange(3, exec_err_pipe_write)
os.closerange(exec_err_pipe_write+1, max_fd)
spass_fds = sorted(set(pass_fds) | {exec_err_pipe_write})
for pair in zip([2] + spass_fds, spass_fds + [max_fd]):
os.closerange(pair[0]+1, pair[1])

if cwd is not None:
os.chdir(cwd)
Expand Down
36 changes: 36 additions & 0 deletions tests/test_spawn.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import fcntl
import os
import time
import select
import tempfile
import unittest
from ptyprocess.ptyprocess import which
from ptyprocess import PtyProcess, PtyProcessUnicode
Expand Down Expand Up @@ -111,3 +113,37 @@ def test_interactive_repl_unicode_noecho(self):
@unittest.skipIf(which('bc') is None, "bc(1) not found on this server.")
def test_interactive_repl_unicode_echo(self):
self._interactive_repl_unicode(echo=True)

def test_pass_fds(self):
with tempfile.NamedTemporaryFile() as temp_file:
temp_file_fd = temp_file.fileno()
temp_file_name = temp_file.name

# Temporary files are CLOEXEC by default
fcntl.fcntl(temp_file_fd,
fcntl.F_SETFD,
fcntl.fcntl(temp_file_fd, fcntl.F_GETFD) &
~fcntl.FD_CLOEXEC)

# You can write with pass_fds
p = PtyProcess.spawn(['bash',
'-c',
'printf hello >&{}'.format(temp_file_fd)],
echo=True,
pass_fds=(temp_file_fd,))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth having a test that when you don't specify it in pass_fds, the child process can't access it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the more tests, the better :) I've added that one too.

p.wait()
assert p.status == 0

with open(temp_file_name, 'r') as temp_file_r:
assert temp_file_r.read() == 'hello'

# You can't write without pass_fds
p = PtyProcess.spawn(['bash',
'-c',
'printf bye >&{}'.format(temp_file_fd)],
echo=True)
p.wait()
assert p.status != 0

with open(temp_file_name, 'r') as temp_file_r:
assert temp_file_r.read() == 'hello'