From 9af8afdca02a3af67bb82e846fca643ad8604c0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Michael=20O=2E=20Hegg=C3=B8?= Date: Sat, 13 Aug 2016 15:39:32 +0200 Subject: [PATCH 1/3] Use escapeshellarg Use escapeshellarg by default, but allow disabling it. --- README.md | 8 +++++++ src/Command.php | 50 ++++++++++++++++--------------------------- tests/CommandTest.php | 30 ++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 41c6c58..b74ae3b 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,14 @@ or } ``` +By default, all arguments are escaped using +[escapeshellarg](https://secure.php.net/manual/en/function.escapeshellarg.php). +If you need to pass unescaped arguments, use `{!name!}`, like so: + +```php +Command::exec('echo {!path!}', ['path' => '$PATH']); +``` + ## Testing ``` bash diff --git a/src/Command.php b/src/Command.php index 6e2515f..96c8b66 100644 --- a/src/Command.php +++ b/src/Command.php @@ -13,22 +13,22 @@ private function __construct() /** * Execute command with params. * - * @param string $commandLine + * @param string $command * @param array $params * * @return bool|string * * @throws \Exception */ - public static function exec($commandLine, array $params = array()) + public static function exec($command, array $params = array()) { - if (empty($commandLine)) { + if (empty($command)) { throw new \Exception('Command line is empty'); } - $commandLine = self::bindParams($commandLine, $params); + $command = self::bindParams($command, $params); - exec($commandLine, $output, $code); + exec($command, $output, $code); if (count($output) === 0) { $output = $code; @@ -37,7 +37,7 @@ public static function exec($commandLine, array $params = array()) } if ($code !== 0) { - throw new \Exception($output . ' Command line: ' . $commandLine); + throw new \Exception($output . ' Command line: ' . $command); } return $output; @@ -46,38 +46,26 @@ public static function exec($commandLine, array $params = array()) /** * Bind params to command. * - * @param string $commandLine + * @param string $command * @param array $params * * @return string */ - public static function bindParams($commandLine, array $params) + public static function bindParams($command, array $params) { - if (count($params) > 0) { - $wrapper = function ($string) { - return '{' . $string . '}'; - }; - $converter = function ($var) { - if (is_array($var)) { - $var = implode(' ', $var); - } - - return $var; - }; + $wrappers = array(); + $converters = array(); + foreach ($params as $key => $value) { - $commandLine = str_replace( - array_map( - $wrapper, - array_keys($params) - ), - array_map( - $converter, - array_values($params) - ), - $commandLine - ); + // Escaped + $wrappers[] = '{' . $key . '}'; + $converters[] = escapeshellarg(is_array($value) ? implode(' ', $value) : $value); + + // Unescaped + $wrappers[] = '{!' . $key . '!}'; + $converters[] = is_array($value) ? implode(' ', $value) : $value; } - return $commandLine; + return str_replace($wrappers, $converters, $command); } } diff --git a/tests/CommandTest.php b/tests/CommandTest.php index 0ca5a1a..8a41139 100644 --- a/tests/CommandTest.php +++ b/tests/CommandTest.php @@ -49,4 +49,34 @@ public function testExecEmptyCommand() '' ); } + + /** + * Test that arguments are escaped by default + */ + public function testArgumentsEscapedByDefault() + { + $output = Command::exec( + 'echo {phrase}', + [ + 'phrase' => 'hello $PATH', + ] + ); + + $this->assertEquals('hello $PATH', $output); + } + + /** + * Test that unescaped arguments can be passed + */ + public function testUnescapedArguments() + { + $output = Command::exec( + 'echo {!phrase!}', + [ + 'phrase' => 'hello $PATH', + ] + ); + + $this->assertRegexp('/\//', $output); + } } From a75638092329667258dfa59231ff3142be96e025 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Michael=20O=2E=20Hegg=C3=B8?= Date: Sat, 13 Aug 2016 16:28:54 +0200 Subject: [PATCH 2/3] Use custom exceptions Use InvalidArgumentException for the invalid argument case, and introduce a new CommandException class for the rest. --- src/Command.php | 4 ++-- src/CommandException.php | 39 +++++++++++++++++++++++++++++++++++++++ tests/CommandTest.php | 4 ++-- 3 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 src/CommandException.php diff --git a/src/Command.php b/src/Command.php index 96c8b66..639db1b 100644 --- a/src/Command.php +++ b/src/Command.php @@ -23,7 +23,7 @@ private function __construct() public static function exec($command, array $params = array()) { if (empty($command)) { - throw new \Exception('Command line is empty'); + throw new \InvalidArgumentException('Command line is empty'); } $command = self::bindParams($command, $params); @@ -37,7 +37,7 @@ public static function exec($command, array $params = array()) } if ($code !== 0) { - throw new \Exception($output . ' Command line: ' . $command); + throw new CommandException($command, $output, $code); } return $output; diff --git a/src/CommandException.php b/src/CommandException.php new file mode 100644 index 0000000..370cbaf --- /dev/null +++ b/src/CommandException.php @@ -0,0 +1,39 @@ +command = $command; + $this->output = $output; + $this->returnCode = $returnCode; + + if ($this->returnCode == 127) { + $message = 'Command not found: "' . $this->getCommand() . '"'; + } else { + $message = 'Command "' . $this->getCommand() . '" exited with code ' . $this->getReturnCode() . ': ' . $this->getOutput(); + } + + parent::__construct($message); + } + + public function getCommand() + { + return $this->command; + } + + public function getOutput() + { + return $this->output; + } + + public function getReturnCode() + { + return $this->returnCode; + } +} diff --git a/tests/CommandTest.php b/tests/CommandTest.php index 8a41139..3ca78da 100644 --- a/tests/CommandTest.php +++ b/tests/CommandTest.php @@ -31,7 +31,7 @@ public function testExec() */ public function testExecException() { - $this->setExpectedException('Exception'); + $this->setExpectedException('pastuhov\Command\CommandException'); $output = Command::exec( 'echo111' @@ -43,7 +43,7 @@ public function testExecException() */ public function testExecEmptyCommand() { - $this->setExpectedException('Exception'); + $this->setExpectedException('InvalidArgumentException'); $output = Command::exec( '' From cee2d60504e53071e6004db87687b51b3d7e1890 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Michael=20O=2E=20Hegg=C3=B8?= Date: Sat, 13 Aug 2016 17:37:57 +0200 Subject: [PATCH 3/3] Include stderr in $output by default Redirect stderr to stdout to have it included in output by default. --- src/Command.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Command.php b/src/Command.php index 639db1b..6c76296 100644 --- a/src/Command.php +++ b/src/Command.php @@ -20,7 +20,7 @@ private function __construct() * * @throws \Exception */ - public static function exec($command, array $params = array()) + public static function exec($command, array $params = array(), $mergeStdErr=true) { if (empty($command)) { throw new \InvalidArgumentException('Command line is empty'); @@ -28,6 +28,11 @@ public static function exec($command, array $params = array()) $command = self::bindParams($command, $params); + if ($mergeStdErr) { + // Redirect stderr to stdout to include it in $output + $command .= ' 2>&1'; + } + exec($command, $output, $code); if (count($output) === 0) {