From: Yuri Astrakhan <yurik@wikimedia.org>
Date: Thu, 9 Oct 2014 01:47:35 +0000 (-0400)
Subject: Remove duplicate param escaping code
X-Git-Tag: 1.31.0-rc.0~12175
X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/banques/ajouter.php?a=commitdiff_plain;h=7c75ee39766c9a521e369356b9e69c835a421703;p=lhc%2Fweb%2Fwiklou.git

Remove duplicate param escaping code

wfEscapeShellArg() can handle multiple params, escaping each.
This patch changes wfShellExec() to call wfEscapeShellArg() directly
instead of doing the gluing itself.

This patch also extends wfEscapeShellArg() to accept an array parameter
optionally instead of as separate args, which is often useful.

Added also unit test cases for single, multiple args, and single array args.

Change-Id: I7a0761cc2ba98c210a9eacadd12da407d933e42a
---

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index c0b8913f4a..dfced1c0bb 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -2660,13 +2660,19 @@ function wfIniGetBool( $setting ) {
  * Also fixes the locale problems on Linux in PHP 5.2.6+ (bug backported to
  * earlier distro releases of PHP)
  *
- * @param string $args,...
+ * @param string ... strings to escape and glue together, or a single array of strings parameter
  * @return string
  */
 function wfEscapeShellArg( /*...*/ ) {
 	wfInitShellLocale();
 
 	$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 ) {
@@ -2799,12 +2805,7 @@ function wfShellExec( $cmd, &$retval = null, $environ = array(),
 		}
 	}
 	if ( is_array( $cmd ) ) {
-		// Command line may be given as an array, escape each value and glue them together with a space
-		$cmdVals = array();
-		foreach ( $cmd as $val ) {
-			$cmdVals[] = wfEscapeShellArg( $val );
-		}
-		$cmd = implode( ' ', $cmdVals );
+		$cmd = wfEscapeShellArg( $cmd );
 	}
 
 	$cmd = $envcmd . $cmd;
@@ -3047,7 +3048,7 @@ function wfShellWikiCmd( $script, array $parameters = array(), array $options =
 	}
 	$cmd[] = $script;
 	// Escape each parameter for shell
-	return implode( " ", array_map( 'wfEscapeShellArg', array_merge( $cmd, $parameters ) ) );
+	return wfEscapeShellArg( array_merge( $cmd, $parameters ) );
 }
 
 /**
@@ -3092,10 +3093,7 @@ function wfMerge( $old, $mine, $yours, &$result ) {
 	fclose( $yourtextFile );
 
 	# Check for a conflict
-	$cmd = wfEscapeShellArg( $wgDiff3 ) . ' -a --overlap-only ' .
-		wfEscapeShellArg( $mytextName ) . ' ' .
-		wfEscapeShellArg( $oldtextName ) . ' ' .
-		wfEscapeShellArg( $yourtextName );
+	$cmd = wfEscapeShellArg( $wgDiff3, '-a', '--overlap-only', $mytextName, $oldtextName, $yourtextName );
 	$handle = popen( $cmd, 'r' );
 
 	if ( fgets( $handle, 1024 ) ) {
@@ -3106,8 +3104,7 @@ function wfMerge( $old, $mine, $yours, &$result ) {
 	pclose( $handle );
 
 	# Merge differences
-	$cmd = wfEscapeShellArg( $wgDiff3 ) . ' -a -e --merge ' .
-		wfEscapeShellArg( $mytextName, $oldtextName, $yourtextName );
+	$cmd = wfEscapeShellArg( $wgDiff3, '-a', '-e', '--merge', $mytextName, $oldtextName, $yourtextName );
 	$handle = popen( $cmd, 'r' );
 	$result = '';
 	do {
diff --git a/tests/phpunit/includes/GlobalFunctions/wfEscapeShellArgTest.php b/tests/phpunit/includes/GlobalFunctions/wfEscapeShellArgTest.php
new file mode 100644
index 0000000000..cb334d2fd2
--- /dev/null
+++ b/tests/phpunit/includes/GlobalFunctions/wfEscapeShellArgTest.php
@@ -0,0 +1,43 @@
+<?php
+
+/**
+ * @group GlobalFunctions
+ * @covers ::wfEscapeShellArg
+ */
+class wfEscapeShellArgTest extends MediaWikiTestCase {
+	public function testSingleInput() {
+		if ( wfIsWindows() ) {
+			$expected = '"blah"';
+		} else {
+			$expected = "'blah'";
+		}
+
+		$actual = wfEscapeShellArg( 'blah' );
+
+		$this->assertEquals( $expected, $actual );
+	}
+
+	public function testMultipleArgs() {
+		if ( wfIsWindows() ) {
+			$expected = '"foo" "bar" "baz"';
+		} else {
+			$expected = "'foo' 'bar' 'baz'";
+		}
+
+		$actual = wfEscapeShellArg( 'foo', 'bar', 'baz' );
+
+		$this->assertEquals( $expected, $actual );
+	}
+
+	public function testMultipleArgsAsArray() {
+		if ( wfIsWindows() ) {
+			$expected = '"foo" "bar" "baz"';
+		} else {
+			$expected = "'foo' 'bar' 'baz'";
+		}
+
+		$actual = wfEscapeShellArg( array( 'foo', 'bar', 'baz' ) );
+
+		$this->assertEquals( $expected, $actual );
+	}
+}