jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/319505 )
Change subject: Replace wfShellExec() with a class
......................................................................
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
<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
---
M RELEASE-NOTES-1.30
M autoload.php
M includes/GlobalFunctions.php
A includes/exception/ProcOpenError.php
A includes/exception/ShellDisabledError.php
A includes/shell/Command.php
A includes/shell/Result.php
A includes/shell/Shell.php
A tests/phpunit/includes/shell/CommandTest.php
A tests/phpunit/includes/shell/ShellTest.php
10 files changed, 807 insertions(+), 267 deletions(-)
Approvals:
Gergő Tisza: Looks good to me, approved
jenkins-bot: Verified
diff --git a/RELEASE-NOTES-1.30 b/RELEASE-NOTES-1.30
index a221f53..67a449a 100644
--- a/RELEASE-NOTES-1.30
+++ b/RELEASE-NOTES-1.30
@@ -215,6 +215,7 @@
* 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 5eba00b..4448204 100644
--- a/autoload.php
+++ b/autoload.php
@@ -904,6 +904,7 @@
'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 @@
'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 3bd73e6..fa97515 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -26,8 +26,10 @@
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 @@
* @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 @@
*
* @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 @@
* 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.';
+ }
+
+ if ( is_array( $cmd ) ) {
+ $cmd = Shell::escape( $cmd );
}
$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 = $envcmd . $cmd;
-
- $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]";
+ $retval = $result->getExitCode();
- $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 $outBuffer;
+ return $result->getStdout();
}
/**
@@ -2569,6 +2330,7 @@
* @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 @@
}
$cmd[] = $script;
// Escape each parameter for shell
- return wfEscapeShellArg( array_merge( $cmd, $parameters ) );
+ return Shell::escape( array_merge( $cmd, $parameters ) );
}
/**
@@ -2654,7 +2416,7 @@
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 @@
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 @@
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 0000000..f00bcd4
--- /dev/null
+++ b/includes/exception/ProcOpenError.php
@@ -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
index 0000000..203b58d
--- /dev/null
+++ b/includes/exception/ShellDisabledError.php
@@ -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
index 0000000..864e69a
--- /dev/null
+++ b/includes/shell/Command.php
@@ -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
index 0000000..c1429df
--- /dev/null
+++ b/includes/shell/Result.php
@@ -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
index 0000000..c293ff2
--- /dev/null
+++ b/includes/shell/Shell.php
@@ -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
index 0000000..33a7f44
--- /dev/null
+++ b/tests/phpunit/includes/shell/CommandTest.php
@@ -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
index 0000000..1e91074
--- /dev/null
+++ b/tests/phpunit/includes/shell/ShellTest.php
@@ -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'" ],
+ ];
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/319505
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I8ac9858b80d7908cf7e7981d7e19d0fc9c2265c0
Gerrit-PatchSet: 23
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: MaxSem <[email protected]>
Gerrit-Reviewer: Brian Wolff <[email protected]>
Gerrit-Reviewer: BryanDavis <[email protected]>
Gerrit-Reviewer: Gergő Tisza <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: MaxSem <[email protected]>
Gerrit-Reviewer: Nikerabbit <[email protected]>
Gerrit-Reviewer: Tim Starling <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
