From 1bb9a223d268518cedc16cfe4d67a293989d7aa5 Mon Sep 17 00:00:00 2001 From: Max Semenik Date: Fri, 8 Sep 2017 23:10:09 -0700 Subject: [PATCH] Inject dependencies into Shell\Command This slightly changes how execution time limits fall back on each other. Change-Id: I7754a9e6be9638eebe90cb953adb8e2a6ee97cef --- RELEASE-NOTES-1.30 | 5 ++- includes/shell/Command.php | 66 ++++++++++++++++++++++++++------------ includes/shell/Shell.php | 21 ++++++++++-- 3 files changed, 67 insertions(+), 25 deletions(-) diff --git a/RELEASE-NOTES-1.30 b/RELEASE-NOTES-1.30 index 2090ce9255..bec7b86b34 100644 --- a/RELEASE-NOTES-1.30 +++ b/RELEASE-NOTES-1.30 @@ -233,7 +233,10 @@ 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(). +* wfShellExec() and related functions are deprecated, use Shell::command(). This also + slightly changes the behavior of how execution time limits are calculated when only + some of defaults are overridden per-call. When in doubt, always override both wall + clock and CPU time. * (T138166) SpecialEmailUser::getTarget() now requires a second argument, the sending user object. Using the method without the second argument is deprecated. * (T67297) Browsers that don't support Unicode will have their edits rejected. diff --git a/includes/shell/Command.php b/includes/shell/Command.php index 864e69a115..fd8f6a0ca7 100644 --- a/includes/shell/Command.php +++ b/includes/shell/Command.php @@ -24,6 +24,8 @@ use Exception; use MediaWiki\ProcOpenError; use MediaWiki\ShellDisabledError; use Profiler; +use Psr\Log\LoggerAwareTrait; +use Psr\Log\NullLogger; /** * Class used for executing shell commands @@ -31,11 +33,22 @@ use Profiler; * @since 1.30 */ class Command { + use LoggerAwareTrait; + /** @var string */ private $command = ''; /** @var array */ - private $limits = []; + private $limits = [ + // seconds + 'time' => 180, + // seconds + 'walltime' => 180, + // KB + 'memory' => 307200, + // KB + 'filesize' => 102400, + ]; /** @var string[] */ private $env = []; @@ -49,13 +62,20 @@ class Command { /** @var bool */ private $everExecuted = false; + /** @var string|false */ + private $cGroup = false; + /** * Constructor. Don't call directly, instead use Shell::command() + * + * @throws ShellDisabledError */ public function __construct() { if ( Shell::isDisabled() ) { throw new ShellDisabledError(); } + + $this->setLogger( new NullLogger() ); } /** @@ -112,11 +132,10 @@ class Command { * 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; + $this->limits = $limits + $this->limits; return $this; } @@ -158,6 +177,18 @@ class Command { return $this; } + /** + * Sets cgroup for this command + * + * @param string|false $cgroup + * @return $this + */ + public function cgroup( $cgroup ) { + $this->cGroup = $cgroup; + + return $this; + } + /** * Executes command. Afterwards, getExitCode() and getOutput() can be used to access execution * results. @@ -168,8 +199,7 @@ class Command { * @throws ShellDisabledError */ public function execute() { - global $IP, $wgMaxShellMemory, $wgMaxShellFileSize, $wgMaxShellTime, - $wgMaxShellWallClockTime, $wgShellCgroup; + global $IP; $this->everExecuted = true; @@ -197,18 +227,12 @@ class 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 ); + $time = intval( $this->limits['time'] ); + $wallTime = intval( $this->limits['walltime'] ); + // for b/c, wall time falls back to time + $wallTime = min( $time, $wallTime ); + $mem = intval( $this->limits['memory'] ); + $filesize = intval( $this->limits['filesize'] ); if ( $time > 0 || $mem > 0 || $filesize > 0 || $wallTime > 0 ) { $cmd = '/bin/bash ' . escapeshellarg( "$IP/includes/limit.sh" ) . ' ' . @@ -216,7 +240,7 @@ class Command { escapeshellarg( "MW_INCLUDE_STDERR=" . ( $this->useStderr ? '1' : '' ) . ';' . "MW_CPU_LIMIT=$time; " . - 'MW_CGROUP=' . escapeshellarg( $wgShellCgroup ) . '; ' . + 'MW_CGROUP=' . escapeshellarg( $this->cGroup ) . '; ' . "MW_MEM_LIMIT=$mem; " . "MW_FILE_SIZE_LIMIT=$filesize; " . "MW_WALL_CLOCK_LIMIT=$wallTime; " . @@ -252,7 +276,7 @@ class Command { $scoped = Profiler::instance()->scopedProfileIn( __FUNCTION__ . '-' . $profileMethod ); $proc = proc_open( $cmd, $desc, $pipes ); if ( !$proc ) { - wfDebugLog( 'exec', "proc_open() failed: $cmd" ); + $this->logger->error( "proc_open() failed: {command}", [ 'command' => $cmd ] ); throw new ProcOpenError(); } $outBuffer = $logBuffer = ''; @@ -329,7 +353,7 @@ class Command { $lines = explode( "\n", $logBuffer ); $logBuffer = array_pop( $lines ); foreach ( $lines as $line ) { - wfDebugLog( 'exec', $line ); + $this->logger->info( $line ); } } } @@ -369,7 +393,7 @@ class Command { } if ( $logMsg !== false ) { - wfDebugLog( 'exec', "$logMsg: $cmd" ); + $this->logger->warning( "$logMsg: {command}", [ 'command' => $cmd ] ); } return new Result( $retval, $outBuffer ); diff --git a/includes/shell/Shell.php b/includes/shell/Shell.php index c293ff2110..f2c96aeb99 100644 --- a/includes/shell/Shell.php +++ b/includes/shell/Shell.php @@ -22,6 +22,9 @@ namespace MediaWiki\Shell; +use MediaWiki\Logger\LoggerFactory; +use MediaWiki\MediaWikiServices; + /** * Executes shell commands * @@ -39,10 +42,10 @@ namespace MediaWiki\Shell; class Shell { /** - * Returns a new instance of this class + * Returns a new instance of Command 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 + * @param string|string[] $command String or array of strings representing the command to + * be executed, each value will be escaped. * Example: [ 'convert', '-font', 'font name' ] would produce "'convert' '-font' 'font name'" * @return Command */ @@ -54,6 +57,18 @@ class Shell { $args = reset( $args ); } $command = new Command(); + $config = MediaWikiServices::getInstance()->getMainConfig(); + + $limits = [ + 'time' => $config->get( 'MaxShellTime' ), + 'walltime' => $config->get( 'MaxShellWallClockTime' ), + 'memory' => $config->get( 'MaxShellMemory' ), + 'filesize' => $config->get( 'MaxShellFileSize' ), + ]; + $command->limits( $limits ); + $command->cgroup( $config->get( 'ShellCgroup' ) ); + $command->setLogger( LoggerFactory::getInstance( 'exec' ) ); + return $command->params( $args ); } -- 2.20.1