jenkins-bot has submitted this change and it was merged.
Change subject: Add way of including all stderr output when executing command
......................................................................
Add way of including all stderr output when executing command
This adds an option to wfShellExec (and convenience function
wfShellExecWithStderr), to make sure all stderr is duplicated
to stdout. The previous method of doing this was to include
2>&1 on the command line. However this did not redirect errors
from limit.sh (For example cgroups not set up, or if a command
reached the file size limit set by ulimit).
Not sure if this is the best approach, but it seems to work well,
and compared to most other approaches I considered, actually gets
the ulimit errors redirected too.
Currently some files fail to render with no error whatsoever,
hopefully this patch will make what went wrong more obvious.
Also fix a comment in wfShellExec that was incorrect (trailing \n),
and make the initial value of the return value variable be 200, so
if there's ever a bug in php where its not being set properly, it
would be immediately obvious what is happening.
Bug: 53824
Change-Id: I833aeb3ab9da726ecb97331369ea187daad7e795
---
M includes/GlobalFunctions.php
M includes/media/Bitmap.php
M includes/media/Jpeg.php
M includes/media/SVG.php
M includes/upload/UploadBase.php
5 files changed, 47 insertions(+), 13 deletions(-)
Approvals:
BryanDavis: Looks good to me, but someone else must approve
Aaron Schulz: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index bed2c44..90d78ca 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -2759,9 +2759,11 @@
* added to the executed command environment.
* @param array $limits optional array with limits(filesize, memory, time,
walltime)
* this overwrites the global wgShellMax* limits.
- * @return string collected stdout as a string (trailing newlines stripped)
+ * @param array $options Array of options. Only one is "duplicateStderr" =>
true, which
+ * Which duplicates stderr to stdout, including errors from
limit.sh
+ * @return string collected stdout as a string
*/
-function wfShellExec( $cmd, &$retval = null, $environ = array(), $limits =
array() ) {
+function wfShellExec( $cmd, &$retval = null, $environ = array(), $limits =
array(), $options = array() ) {
global $IP, $wgMaxShellMemory, $wgMaxShellFileSize, $wgMaxShellTime,
$wgMaxShellWallClockTime, $wgShellCgroup;
@@ -2772,6 +2774,8 @@
'Unable to run external programs in safe mode.' :
'Unable to run external programs, passthru() is
disabled.';
}
+
+ $includeStderr = isset( $options['duplicateStderr'] ) &&
$options['duplicateStderr'];
wfInitShellLocale();
@@ -2795,6 +2799,10 @@
$cmd = $envcmd . $cmd;
if ( php_uname( 's' ) == 'Linux' ) {
+ $stderrDuplication = '';
+ if ( $includeStderr ) {
+ $stderrDuplication = 'exec 2>&1; ';
+ }
$time = intval ( isset( $limits['time'] ) ? $limits['time'] :
$wgMaxShellTime );
if ( isset( $limits['walltime'] ) ) {
$wallTime = intval( $limits['walltime'] );
@@ -2810,17 +2818,25 @@
$cmd = '/bin/bash ' . escapeshellarg(
"$IP/includes/limit.sh" ) . ' ' .
escapeshellarg( $cmd ) . ' ' .
escapeshellarg(
+ $stderrDuplication .
"MW_CPU_LIMIT=$time; " .
'MW_CGROUP=' . escapeshellarg(
$wgShellCgroup ) . '; ' .
"MW_MEM_LIMIT=$mem; " .
"MW_FILE_SIZE_LIMIT=$filesize; " .
"MW_WALL_CLOCK_LIMIT=$wallTime"
);
+ } else {
+ $cmd .= ' 2>&1';
}
+ } elseif ( $includeStderr ) {
+ $cmd .= ' 2>&1';
}
wfDebug( "wfShellExec: $cmd\n" );
- $retval = 1; // error by default?
+ // Default to an unusual value that shouldn't happen naturally,
+ // so in the unlikely event of a weird php bug, it would be
+ // more obvious what happened.
+ $retval = 200;
ob_start();
passthru( $cmd, $retval );
$output = ob_get_contents();
@@ -2833,6 +2849,24 @@
}
/**
+ * Execute a shell command, returning both stdout and stderr. Convenience
+ * function, as all the arguments to wfShellExec can become unwieldy.
+ *
+ * @note This also includes errors from limit.sh, e.g. if $wgMaxShellFileSize
is exceeded.
+ * @param string $cmd Command line, properly escaped for shell.
+ * @param &$retval null|Mixed optional, will receive the program's exit code.
+ * (non-zero is usually failure)
+ * @param array $environ optional environment variables which should be
+ * added to the executed command environment.
+ * @param array $limits optional array with limits(filesize, memory, time,
walltime)
+ * this overwrites the global wgShellMax* limits.
+ * @return string collected stdout and stderr as a string
+ */
+function wfShellExecWithStderr( $cmd, &$retval = null, $environ = array(),
$limits = array() ) {
+ return wfShellExec( $cmd, $retval, $environ, $limits, array(
'duplicateStderr' => true ) );
+}
+
+/**
* Workaround for http://bugs.php.net/bug.php?id=45132
* escapeshellarg() destroys non-ASCII characters if LANG is not a UTF-8 locale
*/
diff --git a/includes/media/Bitmap.php b/includes/media/Bitmap.php
index 9f7a09c..4031b01 100644
--- a/includes/media/Bitmap.php
+++ b/includes/media/Bitmap.php
@@ -357,12 +357,12 @@
" -depth 8 $sharpen " .
" -rotate -$rotation " .
" {$animation_post} " .
- wfEscapeShellArg( $this->escapeMagickOutput(
$params['dstPath'] ) ) . " 2>&1";
+ wfEscapeShellArg( $this->escapeMagickOutput(
$params['dstPath'] ) );
wfDebug( __METHOD__ . ": running ImageMagick: $cmd\n" );
wfProfileIn( 'convert' );
$retval = 0;
- $err = wfShellExec( $cmd, $retval, $env );
+ $err = wfShellExecWithStderr( $cmd, $retval, $env );
wfProfileOut( 'convert' );
if ( $retval !== 0 ) {
@@ -472,7 +472,7 @@
wfDebug( __METHOD__ . ": Running custom convert command $cmd\n"
);
wfProfileIn( 'convert' );
$retval = 0;
- $err = wfShellExec( $cmd, $retval );
+ $err = wfShellExecWithStderr( $cmd, $retval );
wfProfileOut( 'convert' );
if ( $retval !== 0 ) {
@@ -774,11 +774,11 @@
$cmd = wfEscapeShellArg(
$wgImageMagickConvertCommand ) . " " .
wfEscapeShellArg(
$this->escapeMagickInput( $params['srcPath'], $scene ) ) .
" -rotate -$rotation " .
- wfEscapeShellArg(
$this->escapeMagickOutput( $params['dstPath'] ) ) . " 2>&1";
+ wfEscapeShellArg(
$this->escapeMagickOutput( $params['dstPath'] ) );
wfDebug( __METHOD__ . ": running ImageMagick:
$cmd\n" );
wfProfileIn( 'convert' );
$retval = 0;
- $err = wfShellExec( $cmd, $retval, $env );
+ $err = wfShellExecWithStderr( $cmd, $retval,
$env );
wfProfileOut( 'convert' );
if ( $retval !== 0 ) {
$this->logErrorForExternalProcess(
$retval, $err, $cmd );
diff --git a/includes/media/Jpeg.php b/includes/media/Jpeg.php
index 1feb378..fa76366 100644
--- a/includes/media/Jpeg.php
+++ b/includes/media/Jpeg.php
@@ -75,11 +75,11 @@
$cmd = wfEscapeShellArg( $wgJpegTran ) .
" -rotate " . wfEscapeShellArg( $rotation ) .
" -outfile " . wfEscapeShellArg(
$params['dstPath'] ) .
- " " . wfEscapeShellArg( $params['srcPath'] ) .
" 2>&1";
+ " " . wfEscapeShellArg( $params['srcPath'] );
wfDebug( __METHOD__ . ": running jpgtran: $cmd\n" );
wfProfileIn( 'jpegtran' );
$retval = 0;
- $err = wfShellExec( $cmd, $retval, $env );
+ $err = wfShellExecWithStderr( $cmd, $retval, $env );
wfProfileOut( 'jpegtran' );
if ( $retval !== 0 ) {
$this->logErrorForExternalProcess( $retval,
$err, $cmd );
diff --git a/includes/media/SVG.php b/includes/media/SVG.php
index 4c055a5..72a9696 100644
--- a/includes/media/SVG.php
+++ b/includes/media/SVG.php
@@ -177,7 +177,7 @@
wfEscapeShellArg( $srcPath ),
wfEscapeShellArg( $dstPath ) ),
$wgSVGConverters[$wgSVGConverter]
- ) . " 2>&1";
+ );
$env = array();
if ( $lang !== false ) {
@@ -186,7 +186,7 @@
wfProfileIn( 'rsvg' );
wfDebug( __METHOD__ . ": $cmd\n" );
- $err = wfShellExec( $cmd, $retval, $env );
+ $err = wfShellExecWithStderr( $cmd, $retval,
$env );
wfProfileOut( 'rsvg' );
}
}
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 37dc7cb..3a05f93 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -1323,7 +1323,7 @@
# NOTE: there's a 50 line workaround to make stderr redirection
work on windows, too.
# that does not seem to be worth the pain.
# Ask me (Duesentrieb) about it if it's ever needed.
- $output = wfShellExec( "$command 2>&1", $exitCode );
+ $output = wfShellExecWithStderr( $command, $exitCode );
# map exit code to AV_xxx constants.
$mappedCode = $exitCode;
--
To view, visit https://gerrit.wikimedia.org/r/83974
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I833aeb3ab9da726ecb97331369ea187daad7e795
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Brian Wolff <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Brian Wolff <[email protected]>
Gerrit-Reviewer: BryanDavis <[email protected]>
Gerrit-Reviewer: Reedy <[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
