MaxSem has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/393984 )
Change subject: Shell: skip null parameters
......................................................................
Shell: skip null parameters
Right now they're treated as empty strings, however
this doesn't allow skipping parameters in the middle like
$params = [
'foo',
$x ? '--bar' : null,
'--baz',
];
In some cases this matters, e.g. `ls` works while `ls ''` doesn't.
Change-Id: Icb29d4c48ae7f92fb5635e3865346c98f47abb01
---
M includes/shell/Command.php
M includes/shell/Shell.php
M tests/phpunit/includes/shell/CommandTest.php
M tests/phpunit/includes/shell/ShellTest.php
4 files changed, 32 insertions(+), 1 deletion(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/84/393984/1
diff --git a/includes/shell/Command.php b/includes/shell/Command.php
index 264c3b4..92b1c08 100644
--- a/includes/shell/Command.php
+++ b/includes/shell/Command.php
@@ -106,6 +106,7 @@
/**
* Adds parameters to the command. All parameters are sanitized via
Shell::escape().
+ * Null values are ignored.
*
* @param string|string[] $args,...
* @return $this
@@ -124,6 +125,7 @@
/**
* Adds unsafe parameters to the command. These parameters are NOT
sanitized in any way.
+ * Null values are ignored.
*
* @param string|string[] $args,...
* @return $this
@@ -135,6 +137,11 @@
// treat it as a list of arguments
$args = reset( $args );
}
+ $args = array_filter( $args,
+ function ( $value ) {
+ return $value !== null;
+ }
+ );
$this->command .= implode( ' ', $args );
return $this;
diff --git a/includes/shell/Shell.php b/includes/shell/Shell.php
index 6e4fd02..084e10e 100644
--- a/includes/shell/Shell.php
+++ b/includes/shell/Shell.php
@@ -142,7 +142,7 @@
* PHP 5.2.6+ (bug backported to earlier distro releases of PHP).
*
* @param string $args,... strings to escape and glue together, or a
single array of
- * strings parameter
+ * strings parameter. Null values are ignored.
* @return string
*/
public static function escape( /* ... */ ) {
@@ -156,6 +156,9 @@
$first = true;
$retVal = '';
foreach ( $args as $arg ) {
+ if ( $arg === null ) {
+ continue;
+ }
if ( !$first ) {
$retVal .= ' ';
} else {
diff --git a/tests/phpunit/includes/shell/CommandTest.php
b/tests/phpunit/includes/shell/CommandTest.php
index 81fae33..04b32335 100644
--- a/tests/phpunit/includes/shell/CommandTest.php
+++ b/tests/phpunit/includes/shell/CommandTest.php
@@ -103,6 +103,26 @@
$this->assertRegExp( '/^.+no-such-file.*$/m',
$result->getStderr() );
}
+ /**
+ * Test that null values are skipped by params() and unsafeParams()
+ */
+ public function testNullsAreSkipped() {
+ $params = [ 'echo', 'a', null, 'b' ];
+
+ $command = new Command();
+ $result = $command
+ ->params( $params )
+ ->execute();
+ $this->assertEquals( "a b\n", $result->getStdout() );
+
+ $command = new Command();
+ $result = $command
+ ->unsafeParams( $params )
+ ->execute();
+ $this->assertEquals( "a b\n", $result->getStdout() );
+
+ }
+
public function testT69870() {
$commandLine = wfIsWindows()
// 333 = 331 + CRLF
diff --git a/tests/phpunit/includes/shell/ShellTest.php
b/tests/phpunit/includes/shell/ShellTest.php
index 1e91074..7c96c3c 100644
--- a/tests/phpunit/includes/shell/ShellTest.php
+++ b/tests/phpunit/includes/shell/ShellTest.php
@@ -25,6 +25,7 @@
'simple' => [ [ 'true' ], "'true'" ],
'with args' => [ [ 'convert', '-font', 'font name' ],
"'convert' '-font' 'font name'" ],
'array' => [ [ [ 'convert', '-font', 'font name' ] ],
"'convert' '-font' 'font name'" ],
+ 'skip nulls' => [ [ 'ls', null ], "'ls'" ],
];
}
}
--
To view, visit https://gerrit.wikimedia.org/r/393984
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icb29d4c48ae7f92fb5635e3865346c98f47abb01
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: MaxSem <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
