Improve running of external commands

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

4 files changed, 156 insertions, 27 deletions

diff --git a/application/controllers/file.php b/application/controllers/file.php
index 9e920b9fa..83204c318 100644
--- a/application/controllers/file.php
+++ b/application/controllers/file.php
@@ -130,7 +130,7 @@ class File extends MY_Controller {
 			handle_etag($etag);
 			header("Content-disposition: inline; filename=\"".$id."_qr.png\"\n");
 			header("Content-Type: image/png\n");
-			passthru('qrencode -s 10 -o - '.escapeshellarg(site_url($id).'/'));
+			echo (new \libraries\ProcRunner(array('qrencode', '-s', '10', '-o', '-', site_url($id).'/')))->execSafe()['stdout'];
 			exit();
 
 		case "info":
@@ -235,28 +235,31 @@ class File extends MY_Controller {
 
 	private function _colorify($file, $lexer, $anchor_id = false)
 	{
-		$return_value = 0;
 		$output = "";
 		$lines_to_remove = 0;
 
 		$output .= '<div class="code content table">'."\n";
 		$output .= '<div class="highlight"><pre>'."\n";
 
-		ob_start();
 		if ($lexer == "ascii") {
-			passthru('ansi2html -p < '.escapeshellarg($file), $return_value);
+			// TODO: use exec safe and catch exception
+			$ret = (new \libraries\ProcRunner(array('ansi2html', '-p')))
+				->input(file_get_contents($file))
+				->forbid_stderr()
+				->exec();
 			// Last line is empty
 			$lines_to_remove = 1;
 		} else {
-			passthru('pygmentize -F codetagify -O encoding=guess,outencoding=utf8,stripnl=False -l '.escapeshellarg($lexer).' -f html '.escapeshellarg($file), $return_value);
+			// TODO: use exec safe and catch exception
+			$ret = (new \libraries\ProcRunner(array('pygmentize', '-F', 'codetagify', '-O', 'encoding=guess,outencoding=utf8,stripnl=False', '-l', $lexer, '-f', 'html', $file)))
+				->forbid_stderr()
+				->exec();
 			// Last 2 items are "</pre></div>" and ""
 			$lines_to_remove = 2;
 		}
-		$buf = ob_get_contents();
-		ob_end_clean();
 
 
-		$buf = explode("\n", $buf);
+		$buf = explode("\n", $ret["stdout"]);
 		$line_count = count($buf);
 
 		for ($i = 1; $i <= $lines_to_remove; $i++) {
@@ -287,7 +290,7 @@ class File extends MY_Controller {
 		$output .= "</div>";
 
 		return array(
-			"return_value" => $return_value,
+			"return_value" => $ret["return_code"],
 			"output" => $output
 		);
 	}
@@ -305,12 +308,14 @@ class File extends MY_Controller {
 					echo '<div class="code content table markdownrender">'."\n";
 					echo '<div class="table-row">'."\n";
 					echo '<div class="table-cell">'."\n";
-					passthru(escapeshellarg(FCPATH.'scripts/Markdown.pl').' '.escapeshellarg($file), $return_value);
+					// TODO: use exec safe and catch exception
+					$r = (new \libraries\ProcRunner(array(FCPATH.'scripts/Markdown.pl', $file)))->forbid_stderr()->exec();
+					echo $r['stdout'];
 					echo '</div></div></div>';
 
 					return array(
 						"output" => ob_get_clean(),
-						"return_value" => $return_value,
+						"return_value" => $r["return_code"],
 					);
 				} else {
 					return get_instance()->_colorify($file, $lexer, $is_multipaste ? $filedata["id"] : false);
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;
 		});
 	}