Replace wfShellExec() with a class
authorMax Semenik <maxsem.wiki@gmail.com>
Thu, 3 Nov 2016 00:27:15 +0000 (17:27 -0700)
committerMax Semenik <maxsem.wiki@gmail.com>
Sat, 9 Sep 2017 04:49:49 +0000 (21:49 -0700)
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
<legoktm> 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

RELEASE-NOTES-1.30
autoload.php
includes/GlobalFunctions.php
includes/exception/ProcOpenError.php [new file with mode: 0644]
includes/exception/ShellDisabledError.php [new file with mode: 0644]
includes/shell/Command.php [new file with mode: 0644]
includes/shell/Result.php [new file with mode: 0644]
includes/shell/Shell.php [new file with mode: 0644]
tests/phpunit/includes/shell/CommandTest.php [new file with mode: 0644]
tests/phpunit/includes/shell/ShellTest.php [new file with mode: 0644]

index a221f53..67a449a 100644 (file)
@@ -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
index 5eba00b..4448204 100644 (file)
@@ -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',
index 3bd73e6..fa97515 100644 (file)
@@ -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 (file)
index 0000000..f00bcd4
--- /dev/null
@@ -0,0 +1,29 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki;
+
+use Exception;
+
+class ProcOpenError extends Exception {
+       public function __construct() {
+               parent::__construct( 'proc_open() returned error!' );
+       }
+}
diff --git a/includes/exception/ShellDisabledError.php b/includes/exception/ShellDisabledError.php
new file mode 100644 (file)
index 0000000..203b58d
--- /dev/null
@@ -0,0 +1,32 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki;
+
+use Exception;
+
+/**
+ * @since 1.30
+ */
+class ShellDisabledError extends Exception {
+       public function __construct() {
+               parent::__construct( 'Unable to run external programs, proc_open() is disabled' );
+       }
+}
diff --git a/includes/shell/Command.php b/includes/shell/Command.php
new file mode 100644 (file)
index 0000000..864e69a
--- /dev/null
@@ -0,0 +1,377 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Shell;
+
+use Exception;
+use MediaWiki\ProcOpenError;
+use MediaWiki\ShellDisabledError;
+use Profiler;
+
+/**
+ * Class used for executing shell commands
+ *
+ * @since 1.30
+ */
+class Command {
+       /** @var string */
+       private $command = '';
+
+       /** @var array */
+       private $limits = [];
+
+       /** @var string[] */
+       private $env = [];
+
+       /** @var string */
+       private $method;
+
+       /** @var bool */
+       private $useStderr = false;
+
+       /** @var bool */
+       private $everExecuted = false;
+
+       /**
+        * Constructor. Don't call directly, instead use Shell::command()
+        */
+       public function __construct() {
+               if ( Shell::isDisabled() ) {
+                       throw new ShellDisabledError();
+               }
+       }
+
+       /**
+        * Destructor. Makes sure programmer didn't forget to execute the command after all
+        */
+       public function __destruct() {
+               if ( !$this->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 (file)
index 0000000..c1429df
--- /dev/null
@@ -0,0 +1,61 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Shell;
+
+/**
+ * Returned by MediaWiki\Shell\Command::execute()
+ *
+ * @since 1.30
+ */
+class Result {
+       /** @var int */
+       private $exitCode;
+
+       /** @var string */
+       private $stdout;
+
+       /**
+        * @param int $exitCode
+        * @param string $stdout
+        */
+       public function __construct( $exitCode, $stdout ) {
+               $this->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 (file)
index 0000000..c293ff2
--- /dev/null
@@ -0,0 +1,149 @@
+<?php
+/**
+ * Class used for executing shell commands
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Shell;
+
+/**
+ * Executes shell commands
+ *
+ * @since 1.30
+ *
+ * Use call chaining with this class for expressiveness:
+ *  $result = Shell::command( 'some command' )
+ *       ->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 (file)
index 0000000..33a7f44
--- /dev/null
@@ -0,0 +1,94 @@
+<?php
+
+use MediaWiki\Shell\Command;
+
+/**
+ * @group Shell
+ */
+class CommandTest extends PHPUnit_Framework_TestCase {
+       /**
+        * @expectedException PHPUnit_Framework_Error_Notice
+        */
+       public function testDestruct() {
+               if ( defined( 'HHVM_VERSION' ) ) {
+                       $this->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 (file)
index 0000000..1e91074
--- /dev/null
@@ -0,0 +1,30 @@
+<?php
+
+use MediaWiki\Shell\Shell;
+
+/**
+ * @group Shell
+ */
+class ShellTest extends PHPUnit_Framework_TestCase {
+       public function testIsDisabled() {
+               $this->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'" ],
+               ];
+       }
+}