From: Max Semenik Date: Thu, 3 Nov 2016 00:27:15 +0000 (-0700) Subject: Replace wfShellExec() with a class X-Git-Tag: 1.31.0-rc.0~2163^2 X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/categories/modifier.php?a=commitdiff_plain;h=77ce3b98a0193b674232e972e7c6993a585412d7;p=lhc%2Fweb%2Fwiklou.git Replace wfShellExec() with a class This function has gotten so unwieldy that a helper was introduced. Instead, here's this class that makes shelling out easier and more readable. Example usage: $result = Shell::command( 'shell command' ) ->environment( [ 'ENVIRONMENT_VARIABLE' => 'VALUE' ] ) ->limits( [ 'time' => 300 ] ) ->execute(); $exitCode = $result->getExitCode(); $output = $result->getStdout(); This is a minimal change, so lots of stuff remains unrefactored - I'd rather limit the scope of this commit. A future improvement could be an ability to get stderr separately from stdout. Caveat: execution errors (proc_open is disabled/returned error) now throw errors instead of returning a status code. wfShellExec() still emulates this behavior though. Competing commit: I7dccb2b67a4173a8a89b035e444fbda9102e4d0f MaxSem: so you should continue working on your patch and I'll probably refactor on top of it later after its merged :P Change-Id: I8ac9858b80d7908cf7e7981d7e19d0fc9c2265c0 --- diff --git a/RELEASE-NOTES-1.30 b/RELEASE-NOTES-1.30 index a221f535be..67a449a312 100644 --- a/RELEASE-NOTES-1.30 +++ b/RELEASE-NOTES-1.30 @@ -215,6 +215,7 @@ changes to languages because of Phabricator reports. * DB_SLAVE is deprecated. DB_REPLICA should be used instead. * wfUsePHP() is deprecated. * wfFixSessionID() was removed. +* wfShellExec() and related functions are deprecated, use Shell::command(). == Compatibility == MediaWiki 1.30 requires PHP 5.5.9 or later. There is experimental support for diff --git a/autoload.php b/autoload.php index 5eba00b16f..4448204a5e 100644 --- a/autoload.php +++ b/autoload.php @@ -904,6 +904,7 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Logger\\NullSpi' => __DIR__ . '/includes/debug/logger/NullSpi.php', 'MediaWiki\\Logger\\Spi' => __DIR__ . '/includes/debug/logger/Spi.php', 'MediaWiki\\MediaWikiServices' => __DIR__ . '/includes/MediaWikiServices.php', + 'MediaWiki\\ProcOpenError' => __DIR__ . '/includes/exception/ProcOpenError.php', 'MediaWiki\\Search\\ParserOutputSearchDataExtractor' => __DIR__ . '/includes/search/ParserOutputSearchDataExtractor.php', 'MediaWiki\\Services\\CannotReplaceActiveServiceException' => __DIR__ . '/includes/services/CannotReplaceActiveServiceException.php', 'MediaWiki\\Services\\ContainerDisabledException' => __DIR__ . '/includes/services/ContainerDisabledException.php', @@ -928,6 +929,10 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Session\\SessionProviderInterface' => __DIR__ . '/includes/session/SessionProviderInterface.php', 'MediaWiki\\Session\\Token' => __DIR__ . '/includes/session/Token.php', 'MediaWiki\\Session\\UserInfo' => __DIR__ . '/includes/session/UserInfo.php', + 'MediaWiki\\ShellDisabledError' => __DIR__ . '/includes/exception/ShellDisabledError.php', + 'MediaWiki\\Shell\\Command' => __DIR__ . '/includes/shell/Command.php', + 'MediaWiki\\Shell\\Result' => __DIR__ . '/includes/shell/Result.php', + 'MediaWiki\\Shell\\Shell' => __DIR__ . '/includes/shell/Shell.php', 'MediaWiki\\Site\\MediaWikiPageNameNormalizer' => __DIR__ . '/includes/site/MediaWikiPageNameNormalizer.php', 'MediaWiki\\Tidy\\BalanceActiveFormattingElements' => __DIR__ . '/includes/tidy/Balancer.php', 'MediaWiki\\Tidy\\BalanceElement' => __DIR__ . '/includes/tidy/Balancer.php', diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index 3bd73e6d5c..fa97515297 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -26,8 +26,10 @@ if ( !defined( 'MEDIAWIKI' ) ) { use Liuggio\StatsdClient\Sender\SocketSender; use MediaWiki\Logger\LoggerFactory; +use MediaWiki\ProcOpenError; use MediaWiki\Session\SessionManager; use MediaWiki\MediaWikiServices; +use MediaWiki\Shell\Shell; use Wikimedia\ScopedCallback; use Wikimedia\Rdbms\DBReplicationWaitError; @@ -2237,64 +2239,12 @@ function wfIniGetBool( $setting ) { * @param string $args,... strings to escape and glue together, * or a single array of strings parameter * @return string + * @deprecated since 1.30 use MediaWiki\Shell::escape() */ function wfEscapeShellArg( /*...*/ ) { $args = func_get_args(); - if ( count( $args ) === 1 && is_array( reset( $args ) ) ) { - // If only one argument has been passed, and that argument is an array, - // treat it as a list of arguments - $args = reset( $args ); - } - - $first = true; - $retVal = ''; - foreach ( $args as $arg ) { - if ( !$first ) { - $retVal .= ' '; - } else { - $first = false; - } - - if ( wfIsWindows() ) { - // Escaping for an MSVC-style command line parser and CMD.EXE - // @codingStandardsIgnoreStart For long URLs - // Refs: - // * https://web.archive.org/web/20020708081031/http://mailman.lyra.org/pipermail/scite-interest/2002-March/000436.html - // * https://technet.microsoft.com/en-us/library/cc723564.aspx - // * T15518 - // * CR r63214 - // Double the backslashes before any double quotes. Escape the double quotes. - // @codingStandardsIgnoreEnd - $tokens = preg_split( '/(\\\\*")/', $arg, -1, PREG_SPLIT_DELIM_CAPTURE ); - $arg = ''; - $iteration = 0; - foreach ( $tokens as $token ) { - if ( $iteration % 2 == 1 ) { - // Delimiter, a double quote preceded by zero or more slashes - $arg .= str_replace( '\\', '\\\\', substr( $token, 0, -1 ) ) . '\\"'; - } elseif ( $iteration % 4 == 2 ) { - // ^ in $token will be outside quotes, need to be escaped - $arg .= str_replace( '^', '^^', $token ); - } else { // $iteration % 4 == 0 - // ^ in $token will appear inside double quotes, so leave as is - $arg .= $token; - } - $iteration++; - } - // Double the backslashes before the end of the string, because - // we will soon add a quote - $m = []; - if ( preg_match( '/^(.*?)(\\\\+)$/', $arg, $m ) ) { - $arg = $m[1] . str_replace( '\\', '\\\\', $m[2] ); - } - // Add surrounding quotes - $retVal .= '"' . $arg . '"'; - } else { - $retVal .= escapeshellarg( $arg ); - } - } - return $retVal; + return call_user_func_array( Shell::class . '::escape', $args ); } /** @@ -2302,18 +2252,10 @@ function wfEscapeShellArg( /*...*/ ) { * * @return bool|string False or 'disabled' * @since 1.22 + * @deprecated since 1.30 use MediaWiki\Shell::isDisabled() */ function wfShellExecDisabled() { - static $disabled = null; - if ( is_null( $disabled ) ) { - if ( !function_exists( 'proc_open' ) ) { - wfDebug( "proc_open() is disabled\n" ); - $disabled = 'disabled'; - } else { - $disabled = false; - } - } - return $disabled; + return Shell::isDisabled() ? 'disabled' : false; } /** @@ -2337,221 +2279,40 @@ function wfShellExecDisabled() { * method. Set this to a string for an alternative method to profile from * * @return string Collected stdout as a string + * @deprecated since 1.30 use class MediaWiki\Shell\Shell */ function wfShellExec( $cmd, &$retval = null, $environ = [], $limits = [], $options = [] ) { - global $IP, $wgMaxShellMemory, $wgMaxShellFileSize, $wgMaxShellTime, - $wgMaxShellWallClockTime, $wgShellCgroup; - - $disabled = wfShellExecDisabled(); - if ( $disabled ) { + if ( Shell::isDisabled() ) { $retval = 1; + // Backwards compatibility be upon us... return 'Unable to run external programs, proc_open() is disabled.'; } - $includeStderr = isset( $options['duplicateStderr'] ) && $options['duplicateStderr']; - $profileMethod = isset( $options['profileMethod'] ) ? $options['profileMethod'] : wfGetCaller(); - - $envcmd = ''; - foreach ( $environ as $k => $v ) { - if ( wfIsWindows() ) { - /* Surrounding a set in quotes (method used by wfEscapeShellArg) makes the quotes themselves - * appear in the environment variable, so we must use carat escaping as documented in - * https://technet.microsoft.com/en-us/library/cc723564.aspx - * Note however that the quote isn't listed there, but is needed, and the parentheses - * are listed there but doesn't appear to need it. - */ - $envcmd .= "set $k=" . preg_replace( '/([&|()<>^"])/', '^\\1', $v ) . '&& '; - } else { - /* Assume this is a POSIX shell, thus required to accept variable assignments before the command - * http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_09_01 - */ - $envcmd .= "$k=" . escapeshellarg( $v ) . ' '; - } - } if ( is_array( $cmd ) ) { - $cmd = wfEscapeShellArg( $cmd ); + $cmd = Shell::escape( $cmd ); } - $cmd = $envcmd . $cmd; + $includeStderr = isset( $options['duplicateStderr'] ) && $options['duplicateStderr']; + $profileMethod = isset( $options['profileMethod'] ) ? $options['profileMethod'] : wfGetCaller(); - $useLogPipe = false; - if ( is_executable( '/bin/bash' ) ) { - $time = intval( isset( $limits['time'] ) ? $limits['time'] : $wgMaxShellTime ); - if ( isset( $limits['walltime'] ) ) { - $wallTime = intval( $limits['walltime'] ); - } elseif ( isset( $limits['time'] ) ) { - $wallTime = $time; - } else { - $wallTime = intval( $wgMaxShellWallClockTime ); - } - $mem = intval( isset( $limits['memory'] ) ? $limits['memory'] : $wgMaxShellMemory ); - $filesize = intval( isset( $limits['filesize'] ) ? $limits['filesize'] : $wgMaxShellFileSize ); - - if ( $time > 0 || $mem > 0 || $filesize > 0 || $wallTime > 0 ) { - $cmd = '/bin/bash ' . escapeshellarg( "$IP/includes/limit.sh" ) . ' ' . - escapeshellarg( $cmd ) . ' ' . - escapeshellarg( - "MW_INCLUDE_STDERR=" . ( $includeStderr ? '1' : '' ) . ';' . - "MW_CPU_LIMIT=$time; " . - 'MW_CGROUP=' . escapeshellarg( $wgShellCgroup ) . '; ' . - "MW_MEM_LIMIT=$mem; " . - "MW_FILE_SIZE_LIMIT=$filesize; " . - "MW_WALL_CLOCK_LIMIT=$wallTime; " . - "MW_USE_LOG_PIPE=yes" - ); - $useLogPipe = true; - } elseif ( $includeStderr ) { - $cmd .= ' 2>&1'; - } - } elseif ( $includeStderr ) { - $cmd .= ' 2>&1'; - } - wfDebug( "wfShellExec: $cmd\n" ); - - // Don't try to execute commands that exceed Linux's MAX_ARG_STRLEN. - // Other platforms may be more accomodating, but we don't want to be - // accomodating, because very long commands probably include user - // input. See T129506. - if ( strlen( $cmd ) > SHELL_MAX_ARG_STRLEN ) { - throw new Exception( __METHOD__ . - '(): total length of $cmd must not exceed SHELL_MAX_ARG_STRLEN' ); - } - - $desc = [ - 0 => [ 'file', 'php://stdin', 'r' ], - 1 => [ 'pipe', 'w' ], - 2 => [ 'file', 'php://stderr', 'w' ] ]; - if ( $useLogPipe ) { - $desc[3] = [ 'pipe', 'w' ]; - } - $pipes = null; - $scoped = Profiler::instance()->scopedProfileIn( __FUNCTION__ . '-' . $profileMethod ); - $proc = proc_open( $cmd, $desc, $pipes ); - if ( !$proc ) { - wfDebugLog( 'exec', "proc_open() failed: $cmd" ); + try { + $result = Shell::command( [] ) + ->unsafeParams( (array)$cmd ) + ->environment( $environ ) + ->limits( $limits ) + ->includeStderr( $includeStderr ) + ->profileMethod( $profileMethod ) + ->execute(); + } catch ( ProcOpenError $ex ) { $retval = -1; return ''; } - $outBuffer = $logBuffer = ''; - $emptyArray = []; - $status = false; - $logMsg = false; - - /* According to the documentation, it is possible for stream_select() - * to fail due to EINTR. I haven't managed to induce this in testing - * despite sending various signals. If it did happen, the error - * message would take the form: - * - * stream_select(): unable to select [4]: Interrupted system call (max_fd=5) - * - * where [4] is the value of the macro EINTR and "Interrupted system - * call" is string which according to the Linux manual is "possibly" - * localised according to LC_MESSAGES. - */ - $eintr = defined( 'SOCKET_EINTR' ) ? SOCKET_EINTR : 4; - $eintrMessage = "stream_select(): unable to select [$eintr]"; - - $running = true; - $timeout = null; - $numReadyPipes = 0; - - while ( $running === true || $numReadyPipes !== 0 ) { - if ( $running ) { - $status = proc_get_status( $proc ); - // If the process has terminated, switch to nonblocking selects - // for getting any data still waiting to be read. - if ( !$status['running'] ) { - $running = false; - $timeout = 0; - } - } - - $readyPipes = $pipes; - - // Clear last error - // @codingStandardsIgnoreStart Generic.PHP.NoSilencedErrors.Discouraged - @trigger_error( '' ); - $numReadyPipes = @stream_select( $readyPipes, $emptyArray, $emptyArray, $timeout ); - if ( $numReadyPipes === false ) { - // @codingStandardsIgnoreEnd - $error = error_get_last(); - if ( strncmp( $error['message'], $eintrMessage, strlen( $eintrMessage ) ) == 0 ) { - continue; - } else { - trigger_error( $error['message'], E_USER_WARNING ); - $logMsg = $error['message']; - break; - } - } - foreach ( $readyPipes as $fd => $pipe ) { - $block = fread( $pipe, 65536 ); - if ( $block === '' ) { - // End of file - fclose( $pipes[$fd] ); - unset( $pipes[$fd] ); - if ( !$pipes ) { - break 2; - } - } elseif ( $block === false ) { - // Read error - $logMsg = "Error reading from pipe"; - break 2; - } elseif ( $fd == 1 ) { - // From stdout - $outBuffer .= $block; - } elseif ( $fd == 3 ) { - // From log FD - $logBuffer .= $block; - if ( strpos( $block, "\n" ) !== false ) { - $lines = explode( "\n", $logBuffer ); - $logBuffer = array_pop( $lines ); - foreach ( $lines as $line ) { - wfDebugLog( 'exec', $line ); - } - } - } - } - } - - foreach ( $pipes as $pipe ) { - fclose( $pipe ); - } - - // Use the status previously collected if possible, since proc_get_status() - // just calls waitpid() which will not return anything useful the second time. - if ( $running ) { - $status = proc_get_status( $proc ); - } - - if ( $logMsg !== false ) { - // Read/select error - $retval = -1; - proc_close( $proc ); - } elseif ( $status['signaled'] ) { - $logMsg = "Exited with signal {$status['termsig']}"; - $retval = 128 + $status['termsig']; - proc_close( $proc ); - } else { - if ( $status['running'] ) { - $retval = proc_close( $proc ); - } else { - $retval = $status['exitcode']; - proc_close( $proc ); - } - if ( $retval == 127 ) { - $logMsg = "Possibly missing executable file"; - } elseif ( $retval >= 129 && $retval <= 192 ) { - $logMsg = "Probably exited with signal " . ( $retval - 128 ); - } - } - if ( $logMsg !== false ) { - wfDebugLog( 'exec', "$logMsg: $cmd" ); - } + $retval = $result->getExitCode(); - return $outBuffer; + return $result->getStdout(); } /** @@ -2569,6 +2330,7 @@ function wfShellExec( $cmd, &$retval = null, $environ = [], * @param array $limits Optional array with limits(filesize, memory, time, walltime) * this overwrites the global wgMaxShell* limits. * @return string Collected stdout and stderr as a string + * @deprecated since 1.30 use class MediaWiki\Shell\Shell */ function wfShellExecWithStderr( $cmd, &$retval = null, $environ = [], $limits = [] ) { return wfShellExec( $cmd, $retval, $environ, $limits, @@ -2609,7 +2371,7 @@ function wfShellWikiCmd( $script, array $parameters = [], array $options = [] ) } $cmd[] = $script; // Escape each parameter for shell - return wfEscapeShellArg( array_merge( $cmd, $parameters ) ); + return Shell::escape( array_merge( $cmd, $parameters ) ); } /** @@ -2654,7 +2416,7 @@ function wfMerge( $old, $mine, $yours, &$result ) { fclose( $yourtextFile ); # Check for a conflict - $cmd = wfEscapeShellArg( $wgDiff3, '-a', '--overlap-only', $mytextName, + $cmd = Shell::escape( $wgDiff3, '-a', '--overlap-only', $mytextName, $oldtextName, $yourtextName ); $handle = popen( $cmd, 'r' ); @@ -2666,7 +2428,7 @@ function wfMerge( $old, $mine, $yours, &$result ) { pclose( $handle ); # Merge differences - $cmd = wfEscapeShellArg( $wgDiff3, '-a', '-e', '--merge', $mytextName, + $cmd = Shell::escape( $wgDiff3, '-a', '-e', '--merge', $mytextName, $oldtextName, $yourtextName ); $handle = popen( $cmd, 'r' ); $result = ''; @@ -2730,7 +2492,7 @@ function wfDiff( $before, $after, $params = '-u' ) { fclose( $newtextFile ); // Get the diff of the two files - $cmd = "$wgDiff " . $params . ' ' . wfEscapeShellArg( $oldtextName, $newtextName ); + $cmd = "$wgDiff " . $params . ' ' . Shell::escape( $oldtextName, $newtextName ); $h = popen( $cmd, 'r' ); if ( !$h ) { diff --git a/includes/exception/ProcOpenError.php b/includes/exception/ProcOpenError.php new file mode 100644 index 0000000000..f00bcd4bd4 --- /dev/null +++ b/includes/exception/ProcOpenError.php @@ -0,0 +1,29 @@ +everExecuted ) { + $message = __CLASS__ . " was instantiated, but execute() was never called."; + if ( $this->method ) { + $message .= " Calling method: {$this->method}."; + } + $message .= " Command: {$this->command}"; + trigger_error( $message, E_USER_NOTICE ); + } + } + + /** + * Adds parameters to the command. All parameters are sanitized via Shell::escape(). + * + * @param string|string[] $args,... + * @return $this + */ + public function params( /* ... */ ) { + $args = func_get_args(); + if ( count( $args ) === 1 && is_array( reset( $args ) ) ) { + // If only one argument has been passed, and that argument is an array, + // treat it as a list of arguments + $args = reset( $args ); + } + $this->command .= ' ' . Shell::escape( $args ); + + return $this; + } + + /** + * Adds unsafe parameters to the command. These parameters are NOT sanitized in any way. + * + * @param string|string[] $args,... + * @return $this + */ + public function unsafeParams( /* ... */ ) { + $args = func_get_args(); + if ( count( $args ) === 1 && is_array( reset( $args ) ) ) { + // If only one argument has been passed, and that argument is an array, + // treat it as a list of arguments + $args = reset( $args ); + } + $this->command .= implode( ' ', $args ); + + return $this; + } + + /** + * Sets execution limits + * + * @param array $limits Optional array with limits(filesize, memory, time, walltime). + * This overrides the global wgMaxShell* limits. + * @return $this + */ + public function limits( array $limits ) { + $this->limits = $limits; + + return $this; + } + + /** + * Sets environment variables which should be added to the executed command environment + * + * @param string[] $env array of variable name => value + * @return $this + */ + public function environment( array $env ) { + $this->env = $env; + + return $this; + } + + /** + * Sets calling function for profiler. By default, the caller for execute() will be used. + * + * @param string $method + * @return $this + */ + public function profileMethod( $method ) { + $this->method = $method; + + return $this; + } + + /** + * Controls whether stderr should be included in stdout, including errors from limit.sh. + * Default: don't include. + * + * @param bool $yesno + * @return $this + */ + public function includeStderr( $yesno = true ) { + $this->useStderr = $yesno; + + return $this; + } + + /** + * Executes command. Afterwards, getExitCode() and getOutput() can be used to access execution + * results. + * + * @return Result + * @throws Exception + * @throws ProcOpenError + * @throws ShellDisabledError + */ + public function execute() { + global $IP, $wgMaxShellMemory, $wgMaxShellFileSize, $wgMaxShellTime, + $wgMaxShellWallClockTime, $wgShellCgroup; + + $this->everExecuted = true; + + $profileMethod = $this->method ?: wfGetCaller(); + + $envcmd = ''; + foreach ( $this->env as $k => $v ) { + if ( wfIsWindows() ) { + /* Surrounding a set in quotes (method used by wfEscapeShellArg) makes the quotes themselves + * appear in the environment variable, so we must use carat escaping as documented in + * https://technet.microsoft.com/en-us/library/cc723564.aspx + * Note however that the quote isn't listed there, but is needed, and the parentheses + * are listed there but doesn't appear to need it. + */ + $envcmd .= "set $k=" . preg_replace( '/([&|()<>^"])/', '^\\1', $v ) . '&& '; + } else { + /* Assume this is a POSIX shell, thus required to accept variable assignments before the command + * http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_09_01 + */ + $envcmd .= "$k=" . escapeshellarg( $v ) . ' '; + } + } + + $cmd = $envcmd . trim( $this->command ); + + $useLogPipe = false; + if ( is_executable( '/bin/bash' ) ) { + $time = intval( isset( $this->limits['time'] ) ? $this->limits['time'] : $wgMaxShellTime ); + if ( isset( $this->limits['walltime'] ) ) { + $wallTime = intval( $this->limits['walltime'] ); + } elseif ( isset( $this->limits['time'] ) ) { + $wallTime = $time; + } else { + $wallTime = intval( $wgMaxShellWallClockTime ); + } + $mem = intval( isset( $this->limits['memory'] ) ? $this->limits['memory'] : $wgMaxShellMemory ); + $filesize = intval( isset( $this->limits['filesize'] ) + ? $this->limits['filesize'] + : $wgMaxShellFileSize ); + + if ( $time > 0 || $mem > 0 || $filesize > 0 || $wallTime > 0 ) { + $cmd = '/bin/bash ' . escapeshellarg( "$IP/includes/limit.sh" ) . ' ' . + escapeshellarg( $cmd ) . ' ' . + escapeshellarg( + "MW_INCLUDE_STDERR=" . ( $this->useStderr ? '1' : '' ) . ';' . + "MW_CPU_LIMIT=$time; " . + 'MW_CGROUP=' . escapeshellarg( $wgShellCgroup ) . '; ' . + "MW_MEM_LIMIT=$mem; " . + "MW_FILE_SIZE_LIMIT=$filesize; " . + "MW_WALL_CLOCK_LIMIT=$wallTime; " . + "MW_USE_LOG_PIPE=yes" + ); + $useLogPipe = true; + } elseif ( $this->useStderr ) { + $cmd .= ' 2>&1'; + } + } elseif ( $this->useStderr ) { + $cmd .= ' 2>&1'; + } + wfDebug( __METHOD__ . ": $cmd\n" ); + + // Don't try to execute commands that exceed Linux's MAX_ARG_STRLEN. + // Other platforms may be more accomodating, but we don't want to be + // accomodating, because very long commands probably include user + // input. See T129506. + if ( strlen( $cmd ) > SHELL_MAX_ARG_STRLEN ) { + throw new Exception( __METHOD__ . + '(): total length of $cmd must not exceed SHELL_MAX_ARG_STRLEN' ); + } + + $desc = [ + 0 => [ 'file', 'php://stdin', 'r' ], + 1 => [ 'pipe', 'w' ], + 2 => [ 'file', 'php://stderr', 'w' ], + ]; + if ( $useLogPipe ) { + $desc[3] = [ 'pipe', 'w' ]; + } + $pipes = null; + $scoped = Profiler::instance()->scopedProfileIn( __FUNCTION__ . '-' . $profileMethod ); + $proc = proc_open( $cmd, $desc, $pipes ); + if ( !$proc ) { + wfDebugLog( 'exec', "proc_open() failed: $cmd" ); + throw new ProcOpenError(); + } + $outBuffer = $logBuffer = ''; + $emptyArray = []; + $status = false; + $logMsg = false; + + /* According to the documentation, it is possible for stream_select() + * to fail due to EINTR. I haven't managed to induce this in testing + * despite sending various signals. If it did happen, the error + * message would take the form: + * + * stream_select(): unable to select [4]: Interrupted system call (max_fd=5) + * + * where [4] is the value of the macro EINTR and "Interrupted system + * call" is string which according to the Linux manual is "possibly" + * localised according to LC_MESSAGES. + */ + $eintr = defined( 'SOCKET_EINTR' ) ? SOCKET_EINTR : 4; + $eintrMessage = "stream_select(): unable to select [$eintr]"; + + $running = true; + $timeout = null; + $numReadyPipes = 0; + + while ( $running === true || $numReadyPipes !== 0 ) { + if ( $running ) { + $status = proc_get_status( $proc ); + // If the process has terminated, switch to nonblocking selects + // for getting any data still waiting to be read. + if ( !$status['running'] ) { + $running = false; + $timeout = 0; + } + } + + $readyPipes = $pipes; + + // Clear last error + // @codingStandardsIgnoreStart Generic.PHP.NoSilencedErrors.Discouraged + @trigger_error( '' ); + $numReadyPipes = @stream_select( $readyPipes, $emptyArray, $emptyArray, $timeout ); + if ( $numReadyPipes === false ) { + // @codingStandardsIgnoreEnd + $error = error_get_last(); + if ( strncmp( $error['message'], $eintrMessage, strlen( $eintrMessage ) ) == 0 ) { + continue; + } else { + trigger_error( $error['message'], E_USER_WARNING ); + $logMsg = $error['message']; + break; + } + } + foreach ( $readyPipes as $fd => $pipe ) { + $block = fread( $pipe, 65536 ); + if ( $block === '' ) { + // End of file + fclose( $pipes[$fd] ); + unset( $pipes[$fd] ); + if ( !$pipes ) { + break 2; + } + } elseif ( $block === false ) { + // Read error + $logMsg = "Error reading from pipe"; + break 2; + } elseif ( $fd == 1 ) { + // From stdout + $outBuffer .= $block; + } elseif ( $fd == 3 ) { + // From log FD + $logBuffer .= $block; + if ( strpos( $block, "\n" ) !== false ) { + $lines = explode( "\n", $logBuffer ); + $logBuffer = array_pop( $lines ); + foreach ( $lines as $line ) { + wfDebugLog( 'exec', $line ); + } + } + } + } + } + + foreach ( $pipes as $pipe ) { + fclose( $pipe ); + } + + // Use the status previously collected if possible, since proc_get_status() + // just calls waitpid() which will not return anything useful the second time. + if ( $running ) { + $status = proc_get_status( $proc ); + } + + if ( $logMsg !== false ) { + // Read/select error + $retval = -1; + proc_close( $proc ); + } elseif ( $status['signaled'] ) { + $logMsg = "Exited with signal {$status['termsig']}"; + $retval = 128 + $status['termsig']; + proc_close( $proc ); + } else { + if ( $status['running'] ) { + $retval = proc_close( $proc ); + } else { + $retval = $status['exitcode']; + proc_close( $proc ); + } + if ( $retval == 127 ) { + $logMsg = "Possibly missing executable file"; + } elseif ( $retval >= 129 && $retval <= 192 ) { + $logMsg = "Probably exited with signal " . ( $retval - 128 ); + } + } + + if ( $logMsg !== false ) { + wfDebugLog( 'exec', "$logMsg: $cmd" ); + } + + return new Result( $retval, $outBuffer ); + } +} diff --git a/includes/shell/Result.php b/includes/shell/Result.php new file mode 100644 index 0000000000..c1429dfca2 --- /dev/null +++ b/includes/shell/Result.php @@ -0,0 +1,61 @@ +exitCode = $exitCode; + $this->stdout = $stdout; + } + + /** + * Returns exit code of the process + * + * @return int + */ + public function getExitCode() { + return $this->exitCode; + } + + /** + * Returns stdout of the process + * + * @return string + */ + public function getStdout() { + return $this->stdout; + } +} diff --git a/includes/shell/Shell.php b/includes/shell/Shell.php new file mode 100644 index 0000000000..c293ff2110 --- /dev/null +++ b/includes/shell/Shell.php @@ -0,0 +1,149 @@ +environment( [ 'ENVIRONMENT_VARIABLE' => 'VALUE' ] ) + * ->limits( [ 'time' => 300 ] ) + * ->execute(); + * + * ... = $result->getExitCode(); + * ... = $result->getStdout(); + */ +class Shell { + + /** + * Returns a new instance of this class + * + * @param string|string[] $command If string, a properly shell-escaped command line, + * or an array of unescaped arguments, in which case each value will be escaped + * Example: [ 'convert', '-font', 'font name' ] would produce "'convert' '-font' 'font name'" + * @return Command + */ + public static function command( $command ) { + $args = func_get_args(); + if ( count( $args ) === 1 && is_array( reset( $args ) ) ) { + // If only one argument has been passed, and that argument is an array, + // treat it as a list of arguments + $args = reset( $args ); + } + $command = new Command(); + return $command->params( $args ); + } + + /** + * Check if this class is effectively disabled via php.ini config + * + * @return bool + */ + public static function isDisabled() { + static $disabled = null; + + if ( is_null( $disabled ) ) { + if ( !function_exists( 'proc_open' ) ) { + wfDebug( "proc_open() is disabled\n" ); + $disabled = true; + } else { + $disabled = false; + } + } + + return $disabled; + } + + /** + * Version of escapeshellarg() that works better on Windows. + * + * Originally, this fixed the incorrect use of single quotes on Windows + * (https://bugs.php.net/bug.php?id=26285) and the locale problems on Linux in + * PHP 5.2.6+ (bug backported to earlier distro releases of PHP). + * + * @param string $args,... strings to escape and glue together, or a single array of + * strings parameter + * @return string + */ + public static function escape( /* ... */ ) { + $args = func_get_args(); + if ( count( $args ) === 1 && is_array( reset( $args ) ) ) { + // If only one argument has been passed, and that argument is an array, + // treat it as a list of arguments + $args = reset( $args ); + } + + $first = true; + $retVal = ''; + foreach ( $args as $arg ) { + if ( !$first ) { + $retVal .= ' '; + } else { + $first = false; + } + + if ( wfIsWindows() ) { + // Escaping for an MSVC-style command line parser and CMD.EXE + // @codingStandardsIgnoreStart For long URLs + // Refs: + // * https://web.archive.org/web/20020708081031/http://mailman.lyra.org/pipermail/scite-interest/2002-March/000436.html + // * https://technet.microsoft.com/en-us/library/cc723564.aspx + // * T15518 + // * CR r63214 + // Double the backslashes before any double quotes. Escape the double quotes. + // @codingStandardsIgnoreEnd + $tokens = preg_split( '/(\\\\*")/', $arg, -1, PREG_SPLIT_DELIM_CAPTURE ); + $arg = ''; + $iteration = 0; + foreach ( $tokens as $token ) { + if ( $iteration % 2 == 1 ) { + // Delimiter, a double quote preceded by zero or more slashes + $arg .= str_replace( '\\', '\\\\', substr( $token, 0, -1 ) ) . '\\"'; + } elseif ( $iteration % 4 == 2 ) { + // ^ in $token will be outside quotes, need to be escaped + $arg .= str_replace( '^', '^^', $token ); + } else { // $iteration % 4 == 0 + // ^ in $token will appear inside double quotes, so leave as is + $arg .= $token; + } + $iteration++; + } + // Double the backslashes before the end of the string, because + // we will soon add a quote + $m = []; + if ( preg_match( '/^(.*?)(\\\\+)$/', $arg, $m ) ) { + $arg = $m[1] . str_replace( '\\', '\\\\', $m[2] ); + } + + // Add surrounding quotes + $retVal .= '"' . $arg . '"'; + } else { + $retVal .= escapeshellarg( $arg ); + } + } + return $retVal; + } +} diff --git a/tests/phpunit/includes/shell/CommandTest.php b/tests/phpunit/includes/shell/CommandTest.php new file mode 100644 index 0000000000..33a7f44704 --- /dev/null +++ b/tests/phpunit/includes/shell/CommandTest.php @@ -0,0 +1,94 @@ +markTestSkipped( 'destructors are unreliable in HHVM' ); + } + $command = new Command(); + $command->params( 'true' ); + } + + private function requirePosix() { + if ( wfIsWindows() ) { + $this->markTestSkipped( 'This test requires a POSIX environment.' ); + } + } + + /** + * @dataProvider provideExecute + */ + public function testExecute( $commandInput, $expectedExitCode, $expectedOutput ) { + $this->requirePosix(); + + $command = new Command(); + $result = $command + ->params( $commandInput ) + ->execute(); + + $this->assertSame( $expectedExitCode, $result->getExitCode() ); + $this->assertSame( $expectedOutput, $result->getStdout() ); + } + + public function provideExecute() { + return [ + 'success status' => [ 'true', 0, '' ], + 'failure status' => [ 'false', 1, '' ], + 'output' => [ [ 'echo', '-n', 'x', '>', 'y' ], 0, 'x > y' ], + ]; + } + + public function testEnvironment() { + $this->requirePosix(); + + $command = new Command(); + $result = $command + ->params( [ 'printenv', 'FOO' ] ) + ->environment( [ 'FOO' => 'bar' ] ) + ->execute(); + $this->assertSame( "bar\n", $result->getStdout() ); + } + + public function testOutput() { + global $IP; + + $this->requirePosix(); + + $command = new Command(); + $result = $command + ->params( [ 'ls', "$IP/index.php" ] ) + ->execute(); + $this->assertSame( "$IP/index.php", trim( $result->getStdout() ) ); + + $command = new Command(); + $result = $command + ->params( [ 'ls', 'index.php', 'no-such-file' ] ) + ->includeStderr() + ->execute(); + $this->assertRegExp( '/^.+no-such-file.*$/m', $result->getStdout() ); + } + + public function testT69870() { + $commandLine = wfIsWindows() + // 333 = 331 + CRLF + ? ( 'for /l %i in (1, 1, 1001) do @echo ' . str_repeat( '*', 331 ) ) + : 'printf "%-333333s" "*"'; + + // Test several times because it involves a race condition that may randomly succeed or fail + for ( $i = 0; $i < 10; $i++ ) { + $command = new Command(); + $output = $command->unsafeParams( $commandLine ) + ->execute() + ->getStdout(); + $this->assertEquals( 333333, strlen( $output ) ); + } + } +} diff --git a/tests/phpunit/includes/shell/ShellTest.php b/tests/phpunit/includes/shell/ShellTest.php new file mode 100644 index 0000000000..1e91074fd4 --- /dev/null +++ b/tests/phpunit/includes/shell/ShellTest.php @@ -0,0 +1,30 @@ +assertInternalType( 'bool', Shell::isDisabled() ); // sanity + } + + /** + * @dataProvider provideEscape + */ + public function testEscape( $args, $expected ) { + if ( wfIsWindows() ) { + $this->markTestSkipped( 'This test requires a POSIX environment.' ); + } + $this->assertSame( $expected, call_user_func_array( [ Shell::class, 'escape' ], $args ) ); + } + + public function provideEscape() { + return [ + 'simple' => [ [ 'true' ], "'true'" ], + 'with args' => [ [ 'convert', '-font', 'font name' ], "'convert' '-font' 'font name'" ], + 'array' => [ [ [ 'convert', '-font', 'font name' ] ], "'convert' '-font' 'font name'" ], + ]; + } +}