jenkins-bot has submitted this change and it was merged. (
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.
Also, fix spacing problems the new tests uncovered:
* Extra space when using params()
* Missing space when combining params() and unsafeParams()
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, 25 insertions(+), 3 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/shell/Command.php b/includes/shell/Command.php
index 264c3b4..998b3ed 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
@@ -117,13 +118,14 @@
// treat it as a list of arguments
$args = reset( $args );
}
- $this->command .= ' ' . Shell::escape( $args );
+ $this->command = trim( $this->command . ' ' . Shell::escape(
$args ) );
return $this;
}
/**
* 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,7 +137,12 @@
// treat it as a list of arguments
$args = reset( $args );
}
- $this->command .= implode( ' ', $args );
+ $args = array_filter( $args,
+ function ( $value ) {
+ return $value !== null;
+ }
+ );
+ $this->command = trim( $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..f7275e1 100644
--- a/tests/phpunit/includes/shell/CommandTest.php
+++ b/tests/phpunit/includes/shell/CommandTest.php
@@ -1,6 +1,7 @@
<?php
use MediaWiki\Shell\Command;
+use Wikimedia\TestingAccessWrapper;
/**
* @group Shell
@@ -103,6 +104,16 @@
$this->assertRegExp( '/^.+no-such-file.*$/m',
$result->getStderr() );
}
+ /**
+ * Test that null values are skipped by params() and unsafeParams()
+ */
+ public function testNullsAreSkipped() {
+ $command = TestingAccessWrapper::newFromObject( new Command );
+ $command->params( 'echo', 'a', null, 'b' );
+ $command->unsafeParams( 'c', null, 'd' );
+ $this->assertEquals( "'echo' 'a' 'b' c d", $command->command );
+ }
+
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: merged
Gerrit-Change-Id: Icb29d4c48ae7f92fb5635e3865346c98f47abb01
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: MaxSem <[email protected]>
Gerrit-Reviewer: Gergő Tisza <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: MaxSem <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
