We add the missing release dates, the NEWS regarding the latest CVE
fixes, and move the entry for bug #79536 to 7.4.7 because it didn't
make it into 7.4.6.
This time a number of comments have been added to make it easy for new devs to understand
what is going on. Also adjusted error message to use colons rather than dashes.
proc_open can accept stream resources in the descriptorspec, like this:
proc_open("command", array(0 => $resource), $pipes);
Previously, if a resource which was *not* of type "stream" was passed, proc_open would
return without freeing dynamically allocated memory. It's fixed now.
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 #1588
(https://github.com/php/php-src/pull/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.
Likely this is because PHP's fread() was updated to report errors from the underlying
system calls only recently.
One way out of this dilemma: 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. However, that would raise another issue: we would need a way to ensure the FD
will be closed eventually in long-running programs.
Another discovery made while testing this code is that fread() does not always return
all the data written to the slave end of the PTY in a single call, even if the data was
written with a single syscall and it is only a few bytes long.
Specifically, when the child process in the test case writes "foo\n" to the PTY, the parent
sometimes receives "foo" (3 bytes) and sometimes "foo\r\n" (5 bytes). (The "\r" is from the
TTY line discipline converting "\n" to "\r\n".) A second call to fread() does return the
remaining bytes, though sometimes all the data is read in the first call, and by the time
the second call is made, the child process has already exited. It seems that liberal use
of the @ operator is needed when using fread() on pipes.
Thanks to Nikita Popov for suggesting that we should just use openpty() rather than
grantpt(), unlockpt(), etc.
lcov is emitting several errors for generated regex files that have no code
coverage data. The fix is to add the files to the lcov exlusion list.
This is not an issue for CI because it uses gcovr to generate code coverage.
The errors:
Processing ext/date/lib/parse_date.gcda
geninfo: WARNING: could not open /home/code/vendor/php/php-src/parse_date.re
geninfo: WARNING: could not open /home/code/vendor/php/php-src/<stdout>
geninfo: WARNING: some exclusion markers may be ignored
Processing ext/date/lib/parse_tz.gcda
Processing ext/date/lib/tm2unixtime.gcda
Processing ext/date/lib/parse_iso_intervals.gcda
geninfo: WARNING: could not open /home/code/vendor/php/php-src/<stdout>
geninfo: WARNING: could not open /home/code/vendor/php/php-src/parse_iso_intervals.re
geninfo: WARNING: some exclusion markers may be ignored
...
genhtml: ERROR: cannot read /home/code/vendor/php/php-src/parse_date.re
Processing file /home/code/vendor/php/php-src/parse_date.re
make: *** [Makefile:443: lcov-html] Error 2
Closes GH-5568.
To solve bug #70886, the test uses random keys to prevent collisions;
however, this is not guaranteed, and as such it may even collide with
other tests in the shmop test suite. The proper solution would be to
use a single key (which could be randomly generated), but to actually
`shmop_close()` after each `shmop_delete()`. This would, however, not
work on Windows due to bug #65987. Therefore we use three different
keys for now.
We have a bunch of APIs for getting type names and it's sometimes
hard to keep them apart ... make it clear that this is the one
you definitely do not want to use.
A time limit can be set on PHP script execution via `set_time_limit` (or .ini file).
When the time limit is reached, the OS will notify PHP and `timed_out` and `vm_interrupt`
flags are set. While these flags are regularly checked when executing PHP code, once the
end of the script is reached, they are not checked while invoking shutdown functions
(registered via `register_shutdown_function`).
Of course, if the shutdown functions are implemented *in* PHP, then the interrupt flag
will be checked while the VM is running PHP bytecode and the timeout will take effect.
But if the shutdown functions are built-in (implemented in C), it will not.
Since the shutdown functions are invoked through `zend_call_function`, add a check of the
`vm_interrupt` flag there. Then, the script time limit will be respected when *entering*
each shutdown function. The fact still remains that if a shutdown function is built-in and
runs for a long time, script execution will not time out until it finishes and the
interpreter tries to invoke the next one.
Still, the behavior of scripts with execution time limits will be more consistent after
this patch. To make the execution time-out feature work even more precisely, it would
be necessary to scrutinize all the built-in functions and add checks of the `vm_interrupt`
flag in any which can run for a long time. That might not be worth the effort, though.
It should be mentioned that this patch does not solely affect shutdown functions, neither
does it solely allow for interruption of running code due to script execution timeout.
Anything else which causes `vm_interrupt` to be set, such as the PHP interpreter receiving
a signal, will take effect when exiting from an internal function. And not just internal
functions which are called because they were registered to run at shutdown; there are
other cases where a series of internal functions might run in the midst of a script. In
all such cases, it will be possible to interrupt the interpreter now.
Closes GH-5543.
I used php-cs-fixer to do the cs fixes. The configuration I used is
posted below. The reason I disabled some of the rules is because they
create too much noise and would make it difficult to review. But please
feel free to close this PR and run the php-cs-fixer yourself.
<?php
$config = PhpCsFixer\Config::create();
$config->setRiskyAllowed(false);
$config->setRules([
'@PSR2' => true,
'@Symfony' => true,
'array_syntax' => false,
'binary_operator_spaces' => false,
'blank_line_before_statement' => false,
'concat_space' => false,
'increment_style' => false,
'phpdoc_align' => false,
'single_quote' => false,
'trailing_comma_in_multiline_array' => false,
'unary_operator_spaces' => false,
'yoda_style' => false,
]);
$finder = PhpCsFixer\Finder::create();
$finder->in(getcwd());
$finder->exclude('Zend');
$finder->exclude('build');
$finder->exclude('ext');
$finder->exclude('pear');
$finder->exclude('sapi');
$finder->exclude('scripts');
$finder->exclude('win32');
$config->setFinder($finder);
return $config;
Closes GH-5557.
