Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 92b46e6

Browse filesBrowse files
committed
Add PTY support to proc_open (again after 16 long years)
Back in 2004, a feature was added to proc_open which allowed it to open a PTY, connecting specific FDs in the child process to the slave end of the PTY and returning the master end of the PTY (wrapped as a PHP stream) in the `$pipes` array. However, this feature was disabled just about a month later. Little information is available about why this was done, but from talking to the original implementer, it seems there were portability problems with some rare flavors of Unix. Re-enable this feature with a simplified implementation which uses openpty(). No attempt is made to support PTYs if the platform does not have openpty(). The configure script checks if linking with -lutil is necessary to use openpty(), but if anything else is required, like including some special header or linking with some other library, PTY support will be disabled. The original PTY support for proc_open automatically daemonized the child process (disassociating it from the TTY session and process group of the parent). However, I don't think this is a good idea. Just because a user opens a child process in a PTY, it doesn't mean they want it to continue running even when the parent process is killed. Of course, if the child process is some kind of server, it will likely daemonize itself; but we have no reason to preempt that decision. It turns out that since 2015, there has been one test case for PTY support in proc_open() in the test suite. This test was added in GitHub PR php#1588 (php#1588). That PR mentioned that the PHP binary in the Debian/Ubuntu repositories is patched to *enable* PTY support. Checking the Debian PHP repository (https://salsa.debian.org/php-team/php.git) shows that this is still true. Debian's patch does not modify the implementation from 2004 in any way; it just removes the #if 0 line which disables it. Naturally, the test case is skipped if PTY support is not enabled. This means that ever since it was added, every test run against the 'vanilla' PHP codebase has skipped it. Interestingly, the test case which was added in 2015 fails on my Linux Mint PC... both with this simplified implementation *and* when enabling the original implementation. Investigation reveals the reason: when the child process using the slave end of the PTY exits and its FDs are all closed, and all buffered data is read from the master end of the PTY, any further attempt to read from the master end fails with EIO. The test case seems to expect that reading from the master end will always return an empty string if no data is available. Could this mean that on the platform where this test case was originally developed, the PHP parent process ran *faster* than the child process, so that all the calls to fread() complete before the child process exits and closes its FDs? Or could something else about the platform have been different, such that read() in the parent does not fail with EIO even after the child process exits? Another question: Are the Debian packagers even running this test case every time they build PHP binaries? Intriguing questions, these! This voluminous commit message would be incomplete without one more point: IF at least one FD referring to the slave end of the PTY is kept open *in the parent process*, the failure with EIO will not occur even after the child process exits. This might be a good approach, though we need a way to ensure the FD will be closed eventually in long-running programs. Perhaps associate it with the proc resource and close it when the proc is freed or explicitly closed with proc_close?
1 parent 0c74bd2 commit 92b46e6
Copy full SHA for 92b46e6

File tree

3 files changed

+60
-82
lines changed
Filter options

3 files changed

+60
-82
lines changed

‎configure.ac

Copy file name to clipboardExpand all lines: configure.ac
+7-3Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,6 @@ getgrnam_r \
566566
getpwuid_r \
567567
getwd \
568568
glob \
569-
grantpt \
570569
inet_ntoa \
571570
inet_ntop \
572571
inet_pton \
@@ -578,7 +577,6 @@ mmap \
578577
nice \
579578
nl_langinfo \
580579
poll \
581-
ptsname \
582580
putenv \
583581
scandir \
584582
setitimer \
@@ -594,7 +592,6 @@ strptime \
594592
strtok_r \
595593
symlink \
596594
tzset \
597-
unlockpt \
598595
unsetenv \
599596
usleep \
600597
utime \
@@ -713,6 +710,13 @@ if test "$PHP_VALGRIND" != "no"; then
713710
fi
714711
fi
715712

713+
dnl Check for openpty. It may require linking against libutil.
714+
AC_CHECK_FUNCS(openpty, openpty=1, openpty=0)
715+
if test $openpty = 0; then
716+
AC_CHECK_LIB(util, openpty, [AC_DEFINE(HAVE_OPENPTY)
717+
LIBS="$LIBS -lutil"])
718+
fi
719+
716720
dnl General settings.
717721
dnl ----------------------------------------------------------------------------
718722
PHP_CONFIGURE_PART(General settings)

‎ext/standard/proc_open.c

Copy file name to clipboardExpand all lines: ext/standard/proc_open.c
+40-68Lines changed: 40 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,8 @@
4343
* */
4444
#ifdef PHP_CAN_SUPPORT_PROC_OPEN
4545

46-
#if 0 && HAVE_PTSNAME && HAVE_GRANTPT && HAVE_UNLOCKPT && HAVE_SYS_IOCTL_H && HAVE_TERMIOS_H
47-
# include <sys/ioctl.h>
48-
# include <termios.h>
49-
# define PHP_CAN_DO_PTS 1
46+
#if HAVE_OPENPTY
47+
#include <pty.h>
5048
#endif
5149

5250
#include "proc_open.h"
@@ -598,6 +596,27 @@ static int set_proc_descriptor_to_blackhole(struct php_proc_open_descriptor_item
598596
return SUCCESS;
599597
}
600598

599+
static int set_proc_descriptor_to_pty(struct php_proc_open_descriptor_item *desc, int *master_fd, int *slave_fd)
600+
{
601+
#if HAVE_OPENPTY
602+
if (*master_fd == -1) {
603+
if (openpty(master_fd, slave_fd, NULL, NULL, NULL)) {
604+
php_error_docref(NULL, E_WARNING, "Could not open PTY (pseudoterminal) - %s", strerror(errno));
605+
return FAILURE;
606+
}
607+
}
608+
609+
desc->is_pipe = 1;
610+
desc->childend = dup(*slave_fd);
611+
desc->parentend = dup(*master_fd);
612+
desc->mode_flags = O_RDWR;
613+
return SUCCESS;
614+
#else
615+
php_error_docref(NULL, E_WARNING, "PTY (pseudoterminal) not supported on this system");
616+
return FAILURE;
617+
#endif
618+
}
619+
601620
static int set_proc_descriptor_to_pipe(struct php_proc_open_descriptor_item *desc, zend_string *zmode)
602621
{
603622
php_file_descriptor_t newpipe[2];
@@ -797,6 +816,7 @@ PHP_FUNCTION(proc_open)
797816
#else
798817
char **argv = NULL;
799818
#endif
819+
int master_fd = -1, slave_fd = -1;
800820
php_process_id_t child;
801821
struct php_process_handle *proc;
802822

@@ -806,11 +826,6 @@ PHP_FUNCTION(proc_open)
806826
#define cleanup_zend_string(ptr) do { if (ptr != NULL) { zend_string_release(ptr); ptr = NULL; } } while(0)
807827
#define cleanup_zend_strings() cleanup_zend_string(ztype); cleanup_zend_string(zmode); cleanup_zend_string(zfile)
808828

809-
#if PHP_CAN_DO_PTS
810-
php_file_descriptor_t dev_ptmx = -1; /* master */
811-
php_file_descriptor_t slave_pty = -1;
812-
#endif
813-
814829
ZEND_PARSE_PARAMETERS_START(3, 6)
815830
Z_PARAM_ZVAL(command_zv)
816831
Z_PARAM_ARRAY(descriptorspec)
@@ -935,31 +950,9 @@ PHP_FUNCTION(proc_open)
935950
goto exit_fail;
936951
}
937952
} else if (strcmp(ZSTR_VAL(ztype), "pty") == 0) {
938-
#if PHP_CAN_DO_PTS
939-
if (dev_ptmx == -1) {
940-
/* open things up */
941-
dev_ptmx = open("/dev/ptmx", O_RDWR);
942-
if (dev_ptmx == -1) {
943-
php_error_docref(NULL, E_WARNING, "Failed to open /dev/ptmx, errno %d", errno);
944-
goto exit_fail;
945-
}
946-
grantpt(dev_ptmx);
947-
unlockpt(dev_ptmx);
948-
slave_pty = open(ptsname(dev_ptmx), O_RDWR);
949-
950-
if (slave_pty == -1) {
951-
php_error_docref(NULL, E_WARNING, "Failed to open slave pty, errno %d", errno);
952-
goto exit_fail;
953-
}
953+
if (set_proc_descriptor_to_pty(&descriptors[ndesc], &master_fd, &slave_fd) == FAILURE) {
954+
goto exit_fail;
954955
}
955-
descriptors[ndesc].is_pipe = 1;
956-
descriptors[ndesc].childend = dup(slave_pty);
957-
descriptors[ndesc].parentend = dup(dev_ptmx);
958-
descriptors[ndesc].mode_flags = O_RDWR;
959-
#else
960-
php_error_docref(NULL, E_WARNING, "PTY pseudo terminal not supported on this system");
961-
goto exit_fail;
962-
#endif
963956
} else {
964957
php_error_docref(NULL, E_WARNING, "%s is not a valid descriptor spec/mode", ZSTR_VAL(ztype));
965958
goto exit_fail;
@@ -1046,27 +1039,6 @@ PHP_FUNCTION(proc_open)
10461039
if (child == 0) {
10471040
/* this is the child process */
10481041

1049-
#if PHP_CAN_DO_PTS
1050-
if (dev_ptmx >= 0) {
1051-
int my_pid = getpid();
1052-
1053-
#ifdef TIOCNOTTY
1054-
/* detach from original tty. Might only need this if isatty(0) is true */
1055-
ioctl(0,TIOCNOTTY,NULL);
1056-
#else
1057-
setsid();
1058-
#endif
1059-
/* become process group leader */
1060-
setpgid(my_pid, my_pid);
1061-
tcsetpgrp(0, my_pid);
1062-
}
1063-
1064-
if (dev_ptmx >= 0) {
1065-
close(dev_ptmx);
1066-
close(slave_pty);
1067-
}
1068-
#endif
1069-
10701042
if (close_parent_ends_of_pipes_in_child(descriptors, ndesc) == FAILURE) {
10711043
/* We are already in child process and can't do anything to make
10721044
* proc_open() return an error in the parent
@@ -1123,13 +1095,6 @@ PHP_FUNCTION(proc_open)
11231095
#endif
11241096
proc->env = env;
11251097

1126-
#if PHP_CAN_DO_PTS
1127-
if (dev_ptmx >= 0) {
1128-
close(dev_ptmx);
1129-
close(slave_pty);
1130-
}
1131-
#endif
1132-
11331098
/* clean up all the child ends and then open streams on the parent
11341099
* ends, where appropriate */
11351100
for (i = 0; i < ndesc; i++) {
@@ -1193,7 +1158,14 @@ PHP_FUNCTION(proc_open)
11931158
#else
11941159
efree_argv(argv);
11951160
#endif
1196-
1161+
#if HAVE_OPENPTY
1162+
if (master_fd != -1) {
1163+
close(master_fd);
1164+
}
1165+
if (slave_fd != -1) {
1166+
close(slave_fd);
1167+
}
1168+
#endif
11971169
efree(descriptors);
11981170
ZVAL_RES(return_value, zend_register_resource(proc, le_proc_open));
11991171
return;
@@ -1213,15 +1185,15 @@ PHP_FUNCTION(proc_open)
12131185
#else
12141186
efree_argv(argv);
12151187
#endif
1216-
cleanup_zend_strings();
1217-
#if PHP_CAN_DO_PTS
1218-
if (dev_ptmx >= 0) {
1219-
close(dev_ptmx);
1188+
#if HAVE_OPENPTY
1189+
if (master_fd != -1) {
1190+
close(master_fd);
12201191
}
1221-
if (slave_pty >= 0) {
1222-
close(slave_pty);
1192+
if (slave_fd != -1) {
1193+
close(slave_fd);
12231194
}
12241195
#endif
1196+
cleanup_zend_strings();
12251197
RETURN_FALSE;
12261198
}
12271199
/* }}} */

‎ext/standard/tests/file/bug69442.phpt

Copy file name to clipboardExpand all lines: ext/standard/tests/file/bug69442.phpt
+13-11Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ EOC;
1717
$output = join("\n", $output);
1818
unlink($tmpFile);
1919

20-
if (strstr($output, "PTY pseudo terminal not supported on this system") !== false) {
20+
if (strstr($output, "PTY (pseudoterminal) not supported on this system") !== false) {
2121
die("skip PTY pseudo terminals are not supported");
2222
}
2323
--FILE--
@@ -28,17 +28,19 @@ $pipes = array();
2828

2929
$process = proc_open($cmd, $descriptors, $pipes);
3030

31-
foreach ($pipes as $type => $pipe) {
32-
$data = fread($pipe, 999);
33-
echo 'type ' . $type . ' ';
34-
var_dump($data);
35-
fclose($pipe);
36-
}
31+
$data0 = fread($pipes[0], 100);
32+
echo 'read from pipe 0: ';
33+
var_dump($data0);
34+
fclose($pipes[0]);
35+
36+
$data3 = fread($pipes[3], 100);
37+
echo 'read from pipe 3: ';
38+
var_dump($data3);
39+
fclose($pipes[3]);
40+
3741
proc_close($process);
3842
--EXPECT--
39-
type 0 string(5) "foo
43+
read from pipe 0: string(5) "foo
4044
"
41-
type 1 string(0) ""
42-
type 2 string(0) ""
43-
type 3 string(3) "42
45+
read from pipe 3: string(3) "42
4446
"

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.