From 926c97c69fb0a07d1e6dc72626de2fe73648c206 Mon Sep 17 00:00:00 2001
From: Max Semenik <maxsem.wiki@gmail.com>
Date: Fri, 2 Dec 2016 16:12:59 -0800
Subject: [PATCH] Return stderr from Shell\Command

Change-Id: I5551ae4bbe7b539b528a734aa82198b11f103871
---
 includes/shell/Command.php                   |  8 +++-
 includes/shell/Result.php                    | 16 +++++++-
 includes/shell/Shell.php                     |  1 +
 tests/phpunit/includes/shell/CommandTest.php | 42 +++++++++++++++++++-
 4 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/includes/shell/Command.php b/includes/shell/Command.php
index 4e0c0ec309..fb2d787a41 100644
--- a/includes/shell/Command.php
+++ b/includes/shell/Command.php
@@ -265,7 +265,7 @@ class Command {
 		$desc = [
 			0 => [ 'file', 'php://stdin', 'r' ],
 			1 => [ 'pipe', 'w' ],
-			2 => [ 'file', 'php://stderr', 'w' ],
+			2 => [ 'pipe', 'w' ],
 		];
 		if ( $useLogPipe ) {
 			$desc[3] = [ 'pipe', 'w' ];
@@ -278,6 +278,7 @@ class Command {
 			throw new ProcOpenError();
 		}
 		$outBuffer = $logBuffer = '';
+		$errBuffer = null;
 		$emptyArray = [];
 		$status = false;
 		$logMsg = false;
@@ -352,6 +353,9 @@ class Command {
 				} elseif ( $fd == 1 ) {
 					// From stdout
 					$outBuffer .= $block;
+				} elseif ( $fd == 2 ) {
+					// From stderr
+					$errBuffer .= $block;
 				} elseif ( $fd == 3 ) {
 					// From log FD
 					$logBuffer .= $block;
@@ -402,6 +406,6 @@ class Command {
 			$this->logger->warning( "$logMsg: {command}", [ 'command' => $cmd ] );
 		}
 
-		return new Result( $retval, $outBuffer );
+		return new Result( $retval, $outBuffer, $errBuffer );
 	}
 }
diff --git a/includes/shell/Result.php b/includes/shell/Result.php
index c1429dfca2..1e18210f2b 100644
--- a/includes/shell/Result.php
+++ b/includes/shell/Result.php
@@ -32,13 +32,17 @@ class Result {
 	/** @var string */
 	private $stdout;
 
+	/** @var string|null */
+	private $stderr;
+
 	/**
 	 * @param int $exitCode
 	 * @param string $stdout
 	 */
-	public function __construct( $exitCode, $stdout ) {
+	public function __construct( $exitCode, $stdout, $stderr = null ) {
 		$this->exitCode = $exitCode;
 		$this->stdout = $stdout;
+		$this->stderr = $stderr;
 	}
 
 	/**
@@ -58,4 +62,14 @@ class Result {
 	public function getStdout() {
 		return $this->stdout;
 	}
+
+	/**
+	 * Returns stderr of the process or null if the Command was configured to add stderr to stdout
+	 * with includeStderr( true )
+	 *
+	 * @return string|null
+	 */
+	public function getStderr() {
+		return $this->stderr;
+	}
 }
diff --git a/includes/shell/Shell.php b/includes/shell/Shell.php
index f2c96aeb99..e21d762d20 100644
--- a/includes/shell/Shell.php
+++ b/includes/shell/Shell.php
@@ -38,6 +38,7 @@ use MediaWiki\MediaWikiServices;
  *
  *  ... = $result->getExitCode();
  *  ... = $result->getStdout();
+ *  ... = $result->getStderr();
  */
 class Shell {
 
diff --git a/tests/phpunit/includes/shell/CommandTest.php b/tests/phpunit/includes/shell/CommandTest.php
index 33a7f44704..34434b9aaf 100644
--- a/tests/phpunit/includes/shell/CommandTest.php
+++ b/tests/phpunit/includes/shell/CommandTest.php
@@ -57,23 +57,61 @@ class CommandTest extends PHPUnit_Framework_TestCase {
 		$this->assertSame( "bar\n", $result->getStdout() );
 	}
 
+	public function testStdout() {
+		$this->requirePosix();
+
+		$command = new Command();
+
+		$result = $command
+			->params( 'bash', '-c', 'echo ThisIsStderr 1>&2' )
+			->execute();
+
+		$this->assertNotContains( 'ThisIsStderr', $result->getStdout() );
+		$this->assertEquals( "ThisIsStderr\n", $result->getStderr() );
+	}
+
+	public function testStdoutRedirection() {
+		$this->requirePosix();
+
+		$command = new Command();
+
+		$result = $command
+			->params( 'bash', '-c', 'echo ThisIsStderr 1>&2' )
+			->includeStderr( true )
+			->execute();
+
+		$this->assertEquals( "ThisIsStderr\n", $result->getStdout() );
+		$this->assertNull( $result->getStderr() );
+	}
+
 	public function testOutput() {
 		global $IP;
 
 		$this->requirePosix();
+		chdir( $IP );
 
 		$command = new Command();
 		$result = $command
-			->params( [ 'ls', "$IP/index.php" ] )
+			->params( [ 'ls', 'index.php' ] )
 			->execute();
-		$this->assertSame( "$IP/index.php", trim( $result->getStdout() ) );
+		$this->assertRegExp( '/^index.php$/m', $result->getStdout() );
+		$this->assertSame( null, $result->getStderr() );
 
 		$command = new Command();
 		$result = $command
 			->params( [ 'ls', 'index.php', 'no-such-file' ] )
 			->includeStderr()
 			->execute();
+		$this->assertRegExp( '/^index.php$/m', $result->getStdout() );
 		$this->assertRegExp( '/^.+no-such-file.*$/m', $result->getStdout() );
+		$this->assertSame( null, $result->getStderr() );
+
+		$command = new Command();
+		$result = $command
+			->params( [ 'ls', 'index.php', 'no-such-file' ] )
+			->execute();
+		$this->assertRegExp( '/^index.php$/m', $result->getStdout() );
+		$this->assertRegExp( '/^.+no-such-file.*$/m', $result->getStderr() );
 	}
 
 	public function testT69870() {
-- 
2.20.1