Improve running of external commands

Signed-off-by: Florian Pritz <bluewind@xinu.at>

3 files changed, 140 insertions, 16 deletions

diff --git a/application/libraries/Image/Drivers/imagemagick.php b/application/libraries/Image/Drivers/imagemagick.php
index ed0c70dc6..e37b9c8b3 100644
--- a/application/libraries/Image/Drivers/imagemagick.php
+++ b/application/libraries/Image/Drivers/imagemagick.php
@@ -45,13 +45,6 @@ class imagemagick implements \libraries\Image\ImageDriver {
 		$this->arguments = array();
 	}
 
-	private function passthru($arguments)
-	{
-		$command_string = implode(" ", array_map("escapeshellarg", $arguments));
-		passthru($command_string, $ret);
-		return $ret;
-	}
-
 	public function get($target_type = null)
 	{
 		if ($target_type === null) {
@@ -62,7 +55,6 @@ class imagemagick implements \libraries\Image\ImageDriver {
 		$command = array_merge($command, $this->arguments);
 		$command[] = $this->source_file."[0]";
 
-		ob_start();
 		switch ($target_type) {
 			case IMAGETYPE_GIF:
 				$command[] = "gif:-";
@@ -76,14 +68,14 @@ class imagemagick implements \libraries\Image\ImageDriver {
 			default:
 				assert(0);
 		}
-		$ret = $this->passthru($command);
-		$result = ob_get_clean();
 
-		if ($ret != 0 || $result === false) {
+		try {
+			$ret = (new \libraries\ProcRunner($command))->execSafe();
+		} catch (\exceptions\ApiException $e) {
 			throw new \exceptions\ApiException("libraries/Image/thumbnail-creation-failed", "Failed to create thumbnail");
 		}
 
-		return $result;
+		return $ret["stdout"];
 	}
 
 	public function resize($width, $height)
diff --git a/application/libraries/ProcRunner.php b/application/libraries/ProcRunner.php
new file mode 100644
index 000000000..6aaaa1f20
--- /dev/null
+++ b/application/libraries/ProcRunner.php
@@ -0,0 +1,129 @@
+<?php
+/*
+ * Copyright 2015 Florian "Bluewind" Pritz <bluewind@server-speed.net>
+ *
+ * Licensed under AGPLv3
+ * (see COPYING for full license text)
+ *
+ */
+
+namespace libraries;
+
+class ProcRunner {
+	private $cmd;
+	private $input = NULL;
+	private $forbid_nonzero = false;
+	private $forbid_stderr = false;
+
+	/**
+	 * This function automatically escapes all parameters before executing the command.
+	 *
+	 * @param cmd array with the command and it's arguments
+	 */
+	function __construct($cmd)
+	{
+		assert(is_array($cmd));
+		$this->cmd = implode(" ", array_map('escapeshellarg', $cmd));
+	}
+
+	/**
+	 * Set stdin. You will have to set this to NULL if you call exec() a second
+	 * time and don't want stdin to be sent again
+	 *
+	 * @param input string to send via stdin
+	 */
+	function input($input)
+	{
+		$this->input = $input;
+		return $this;
+	}
+
+	function forbid_nonzero()
+	{
+		$this->forbid_nonzero = true;
+		return $this;
+	}
+
+
+	function forbid_stderr()
+	{
+		$this->forbid_stderr = true;
+		return $this;
+	}
+
+	/**
+	 * Run the command.
+	 *
+	 * @return array with keys return_code, stdout, stderr
+	 */
+	function exec()
+	{
+		$descriptorspec = array(
+			1 => array('pipe', 'w'),
+			2 => array('pipe', 'w'),
+		);
+
+		if ($this->input !== NULL) {
+			$descriptorspec[0] = array('pipe', 'r');
+		}
+
+		$proc = proc_open($this->cmd, $descriptorspec, $pipes);
+
+		if ($proc === false) {
+			throw new \exceptions\ApiException('procrunner/proc_open-failed',
+				'Failed to run process',
+				array($this->cmd, $this->input)
+			);
+		}
+
+		if ($this->input !== NULL) {
+			fwrite($pipes[0], $this->input);
+			fclose($pipes[0]);
+		}
+
+		$stdout = stream_get_contents($pipes[1]);
+		fclose($pipes[1]);
+		assert($stdout !== false);
+
+		$stderr = stream_get_contents($pipes[2]);
+		fclose($pipes[2]);
+		assert($stderr !== false);
+
+		$return_code = proc_close($proc);
+
+		$ret = array(
+			"return_code" => $return_code,
+			"stdout" => $stdout,
+			"stderr" => $stderr,
+		);
+
+		if ($this->forbid_nonzero && $return_code !== 0) {
+			throw new \exceptions\ApiException('procrunner/non-zero-exit',
+				'Process exited with a non-zero status',
+				array($this->cmd, $this->input, $ret)
+			);
+		}
+
+		if ($this->forbid_stderr && $stderr !== "") {
+			throw new \exceptions\ApiException('procrunner/stderr',
+				'Output on stderr not allowed but received',
+				array($this->cmd, $this->input, $ret)
+			);
+		}
+
+		return $ret;
+	}
+
+	/**
+	 * Run the command and enable some sanity checks such as empty stderr and
+	 * zero exit status. Might enable more in the future.
+	 *
+	 * @See exec
+	 */
+	function execSafe()
+	{
+		$this->forbid_stderr();
+		$this->forbid_nonzero();
+		return $this->exec();
+	}
+}
diff --git a/application/libraries/Pygments.php b/application/libraries/Pygments.php
index 588f78842..5d37b69c3 100644
--- a/application/libraries/Pygments.php
+++ b/application/libraries/Pygments.php
@@ -22,11 +22,14 @@ class Pygments {
 
 	private static function get_pygments_info() {
 		return cache_function_full('pygments_info-v2', 1800, function() {
-			ob_start();
-			passthru(escapeshellarg(FCPATH."scripts/get_lexer_list.py"));
-			$output = ob_get_clean();
+			$r = (new \libraries\ProcRunner(array(FCPATH."scripts/get_lexer_list.py")))->execSafe();
 
-			return json_decode($output, true);
+			$ret = json_decode($r["stdout"], true);
+			if ($ret === NULL) {
+				throw new \exceptions\ApiException('pygments/json-failed', "Failed to decode JSON", $r);
+			}
+
+			return $ret;
 		});
 	}