From 37e32abb0ecb3f34ba8d6f1d1306d0769598eb51 Mon Sep 17 00:00:00 2001 From: Brian Wolff Date: Wed, 11 Sep 2013 22:40:56 -0300 Subject: [PATCH] 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 --- includes/GlobalFunctions.php | 40 +++++++++++++++++++++++++++++++--- includes/media/Bitmap.php | 10 ++++----- includes/media/Jpeg.php | 4 ++-- includes/media/SVG.php | 4 ++-- includes/upload/UploadBase.php | 2 +- 5 files changed, 47 insertions(+), 13 deletions(-) diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index bed2c44b03..90d78cac5a 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -2759,9 +2759,11 @@ function wfShellExecDisabled() { * 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; @@ -2773,6 +2775,8 @@ function wfShellExec( $cmd, &$retval = null, $environ = array(), $limits = array 'Unable to run external programs, passthru() is disabled.'; } + $includeStderr = isset( $options['duplicateStderr'] ) && $options['duplicateStderr']; + wfInitShellLocale(); $envcmd = ''; @@ -2795,6 +2799,10 @@ function wfShellExec( $cmd, &$retval = null, $environ = array(), $limits = array $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 @@ function wfShellExec( $cmd, &$retval = null, $environ = array(), $limits = array $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(); @@ -2832,6 +2848,24 @@ function wfShellExec( $cmd, &$retval = null, $environ = array(), $limits = array return $output; } +/** + * 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 9f7a09cc85..4031b01c5f 100644 --- a/includes/media/Bitmap.php +++ b/includes/media/Bitmap.php @@ -357,12 +357,12 @@ class BitmapHandler extends ImageHandler { " -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 @@ class BitmapHandler extends ImageHandler { 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 @@ class BitmapHandler extends ImageHandler { $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 1feb3788d4..fa7636682e 100644 --- a/includes/media/Jpeg.php +++ b/includes/media/Jpeg.php @@ -75,11 +75,11 @@ class JpegHandler extends ExifBitmapHandler { $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 4c055a5f07..72a9696c29 100644 --- a/includes/media/SVG.php +++ b/includes/media/SVG.php @@ -177,7 +177,7 @@ class SvgHandler extends ImageHandler { wfEscapeShellArg( $srcPath ), wfEscapeShellArg( $dstPath ) ), $wgSVGConverters[$wgSVGConverter] - ) . " 2>&1"; + ); $env = array(); if ( $lang !== false ) { @@ -186,7 +186,7 @@ class SvgHandler extends ImageHandler { 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 37dc7cba30..3a05f93b13 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -1323,7 +1323,7 @@ abstract class UploadBase { # 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; -- 2.20.1