Req #81493 [Com]: shell_exec() should get a `int &$result_code = null` argument

Edit report at https://bugs.php.net/bug.php?id=81493&edit=1 ID: 81493 Comment by: divinity76 at gmail dot com Reported by: divinity76 at gmail dot com Summary: shell_exec() should get a int &$result_code = null argument Status: Open Type: Feature/Change Request Package: Unknown/Other Function PHP Version: Next Minor Version Block user comment: N Private report: N New Comment: >It's funny how we have so many of these functions and they all kinda suck. yeah... that reminds me, back in 2019 i made my own function to do this stuff, cus i didn't particularly like any of them (well, maybe except proc_open, but that one is kind-of too difficult to use, most of the time) i know it's not particularly relevant to this feature request, but i'd love for someone to tell me what sucks about this function <?php /** * better version of shell_exec(), * supporting both stdin and stdout and stderr and os-level return code * * @param string $cmd * command to execute * @param string $stdin * (optional) data to send to stdin, binary data is supported. * @param string $stdout * (optional) stdout data generated by cmd * @param string $stderr * (optional) stderr data generated by cmd * @param bool $print_std * (optional, default false) if you want stdout+stderr to be printed while it's running, * set this to true. (useful for long-running commands) * @return int */ function hhb_exec(string $cmd, string $stdin = "", string &$stdout = null, string &$stderr = null, bool $print_std = false): int { $stdouth = tmpfile(); $stderrh = tmpfile(); $descriptorspec = array( 0 => array( "pipe", "rb" ), // stdin 1 => array( "file", stream_get_meta_data($stdouth)['uri'], 'ab' ), 2 => array( "file", stream_get_meta_data($stderrh)['uri'], 'ab' ) ); $pipes = array(); $proc = proc_open($cmd, $descriptorspec, $pipes); while (strlen($stdin) > 0) { $written_now = fwrite($pipes[0], $stdin); if ($written_now < 1 || $written_now === strlen($stdin)) { // ... can add more error checking here break; } $stdin = substr($stdin, $written_now); } fclose($pipes[0]); unset($stdin, $pipes[0]); if (! $print_std) { $proc_ret = proc_close($proc); // this line will stall until the process has exited. $stdout = stream_get_contents($stdouth); $stderr = stream_get_contents($stderrh); } else { $stdout = ""; $stderr = ""; stream_set_blocking($stdouth, false); stream_set_blocking($stderrh, false); $fetchstd = function () use (&$stdout, &$stderr, &$stdouth, &$stderrh): bool { $ret = false; $tmp = stream_get_contents($stdouth); // fread($stdouth, 1); // if (is_string($tmp) && strlen($tmp) > 0) { $ret = true; $stdout .= $tmp; fwrite(STDOUT, $tmp); } $tmp = stream_get_contents($stderrh);// fread($stderrh, 1); // // var_dump($tmp); if (is_string($tmp) && strlen($tmp) > 0) { $ret = true; $stderr .= $tmp; fwrite(STDERR, $tmp); } return $ret; }; while (($status = proc_get_status($proc))["running"]) { if (! $fetchstd()) { // 100 ms usleep(100 * 1000); } } $proc_ret = $status["exitcode"]; proc_close($proc); $fetchstd(); } fclose($stdouth); fclose($stderrh); return $proc_ret; } ?> Previous Comments: ------------------------------------------------------------------------ [2021-10-01 15:21:32] nikic@php.net The docs for shell_exec() say: > On Windows, the underlying pipe is opened in text mode which can cause the function to fail for > binary output. Consider to use popen() instead for such cases. So it wouldn't be binary safe either... It's funny how we have so many of these functions and they all kinda suck. ------------------------------------------------------------------------ [2021-10-01 15:17:13] divinity76 at gmail dot com actually scratch the exec() workaround, turns out that exec() is non-binary-safe and can't do shell_exec()'s job at all (in cases where binary safety is important) - with exec() it is impossible to differentiate between programs printing "foo" and "foo\n" ------------------------------------------------------------------------ [2021-10-01 15:08:10] divinity76 at gmail dot com actually the implode() workaround is non-binary-safe, take for example: <?php declare(strict_types=1); $cmd= "php -r ".escapeshellarg('echo '.var_export("\n",true).";"); exec($cmd,$output,$ret); $output=implode("\n",$output); var_dump($output); ?> it prints emptystring, not a string containing a newline, sooo data corruption ------------------------------------------------------------------------ [2021-10-01 15:01:05] divinity76 at gmail dot com @requinix to use exec() for the same task as shell_exec(), you'll have to (ab)use implode() like exec($cmd,$output,$ret); $output=implode("\n", $output); and to use system() for the same job, you'd have to (ab)use ob_* like ob_start(); system($cmd,$ret); $output=ob_get_clean(); same for passthru() as it stands now, if you want to do a shell_exec while also getting the os-level return value, you'll have to do a exec()+implode() workaround.. there would be no need for workarounds if shell_exec supported $result_code ------------------------------------------------------------------------ [2021-10-01 14:37:57] requinix@php.net If you want a status code then why not use exec() or system()? Do we really need to make all three functions even *more* similar to each other? ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=81493 -- Edit this bug report at https://bugs.php.net/bug.php?id=81493&edit=1