Use findBinaryPath for previews

Signed-off-by: J0WI <J0WI@users.noreply.github.com>
nextcloud · Oct 23, 2021 · 047cab8 · 047cab8
1 parent 623ac8c
commit 047cab8
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 80 deletions.
diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
@@ -4319,16 +4319,6 @@
       <code>$second</code>
     </InvalidScalarArgument>
   </file>
-  <file src="lib/private/Preview/Office.php">
-    <ForbiddenCode occurrences="3">
-      <code>shell_exec($exec)</code>
-      <code>shell_exec('command -v libreoffice')</code>
-      <code>shell_exec('command -v openoffice')</code>
-    </ForbiddenCode>
-    <ImplicitToStringCast occurrences="1">
-      <code>$png</code>
-    </ImplicitToStringCast>
-  </file>
   <file src="lib/private/Preview/ProviderV1Adapter.php">
     <InvalidReturnStatement occurrences="1">
       <code>$thumbnail === false ? null: $thumbnail</code>
@@ -4350,12 +4340,6 @@
       <code>$svg</code>
     </ImplicitToStringCast>
   </file>
-  <file src="lib/private/PreviewManager.php">
-    <ForbiddenCode occurrences="2">
-      <code>shell_exec('command -v libreoffice')</code>
-      <code>shell_exec('command -v openoffice')</code>
-    </ForbiddenCode>
-  </file>
   <file src="lib/private/RedisFactory.php">
     <InvalidScalarArgument occurrences="1">
       <code>\RedisCluster::OPT_SLAVE_FAILOVER</code>

diff --git a/lib/private/Preview/HEIC.php b/lib/private/Preview/HEIC.php
@@ -30,6 +30,7 @@
 namespace OC\Preview;
 
 use OCP\Files\File;
+use OCP\Files\FileInfo;
 use OCP\IImage;
 use OCP\ILogger;
 
@@ -49,14 +50,18 @@ public function getMimeType(): string {
 	/**
 	 * {@inheritDoc}
 	 */
-	public function isAvailable(\OCP\Files\FileInfo $file): bool {
+	public function isAvailable(FileInfo $file): bool {
 		return in_array('HEIC', \Imagick::queryFormats("HEI*"));
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
+		if (!$this->isAvailable($file)) {
+			return null;
+		}
+
 		$tmpPath = $this->getLocalFile($file);
 
 		// Creates \Imagick object from the heic file

diff --git a/lib/private/Preview/Movie.php b/lib/private/Preview/Movie.php
@@ -30,26 +30,61 @@
 namespace OC\Preview;
 
 use OCP\Files\File;
+use OCP\Files\FileInfo;
 use OCP\IImage;
 use Psr\Log\LoggerInterface;
 
 class Movie extends ProviderV2 {
+
+	/**
+	 * @deprecated 23.0.0 pass option to \OCP\Preview\ProviderV2
+	 * @var string
+	 */
 	public static $avconvBinary;
+
+	/**
+	 * @deprecated 23.0.0 pass option to \OCP\Preview\ProviderV2
+	 * @var string
+	 */
 	public static $ffmpegBinary;
 
+	/** @var string */
+	private $binary;
+
 	/**
 	 * {@inheritDoc}
 	 */
 	public function getMimeType(): string {
 		return '/video\/.*/';
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
+	public function isAvailable(FileInfo $file): bool {
+		// TODO: remove when avconv is dropped
+		if (is_null($this->binary)) {
+			if (isset($this->options['movieBinary'])) {
+				$this->binary = $this->options['movieBinary'];
+			} elseif (is_string(self::$avconvBinary)) {
+				$this->binary = self::$avconvBinary;
+			} elseif (is_string(self::$ffmpegBinary)) {
+				$this->binary = self::$ffmpegBinary;
+			}
+		}
+		return is_string($this->binary);
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */
 	public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		// TODO: use proc_open() and stream the source file ?
 
+		if (!$this->isAvailable($file)) {
+			return null;
+		}
+
 		$result = null;
 		if ($this->useTempFile($file)) {
 			// try downloading 5 MB first as it's likely that the first frames are present there
@@ -92,17 +127,23 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 	private function generateThumbNail($maxX, $maxY, $absPath, $second): ?IImage {
 		$tmpPath = \OC::$server->getTempManager()->getTemporaryFile();
 
-		if (self::$avconvBinary) {
-			$cmd = self::$avconvBinary . ' -y -ss ' . escapeshellarg($second) .
+		$binaryType = substr(strrchr($this->binary, '/'), 1);
+
+		if ($binaryType === 'avconv') {
+			$cmd = $this->binary . ' -y -ss ' . escapeshellarg($second) .
 				' -i ' . escapeshellarg($absPath) .
 				' -an -f mjpeg -vframes 1 -vsync 1 ' . escapeshellarg($tmpPath) .
 				' 2>&1';
-		} else {
-			$cmd = self::$ffmpegBinary . ' -y -ss ' . escapeshellarg($second) .
+		} elseif ($binaryType === 'ffmpeg') {
+			$cmd = $this->binary . ' -y -ss ' . escapeshellarg($second) .
 				' -i ' . escapeshellarg($absPath) .
 				' -f mjpeg -vframes 1' .
 				' ' . escapeshellarg($tmpPath) .
 				' 2>&1';
+		} else {
+			// Not supported
+			unlink($tmpPath);
+			return null;
 		}
 
 		exec($cmd, $output, $returnCode);

diff --git a/lib/private/Preview/Office.php b/lib/private/Preview/Office.php
@@ -29,18 +29,23 @@
 namespace OC\Preview;
 
 use OCP\Files\File;
+use OCP\Files\FileInfo;
 use OCP\IImage;
 use OCP\ILogger;
 
 abstract class Office extends ProviderV2 {
-	private $cmd;
+	/**
+	 * {@inheritDoc}
+	 */
+	public function isAvailable(FileInfo $file): bool {
+		return is_string($this->options['officeBinary']);
+	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
-		$this->initCmd();
-		if (is_null($this->cmd)) {
+		if (!$this->isAvailable($file)) {
 			return null;
 		}
 
@@ -51,9 +56,14 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		$defaultParameters = ' -env:UserInstallation=file://' . escapeshellarg($tmpDir . '/owncloud-' . \OC_Util::getInstanceId() . '/') . ' --headless --nologo --nofirststartwizard --invisible --norestore --convert-to png --outdir ';
 		$clParameters = \OC::$server->getConfig()->getSystemValue('preview_office_cl_parameters', $defaultParameters);
 
-		$exec = $this->cmd . $clParameters . escapeshellarg($tmpDir) . ' ' . escapeshellarg($absPath);
+		$cmd = $this->options['officeBinary'] . $clParameters . escapeshellarg($tmpDir) . ' ' . escapeshellarg($absPath);
+
+		exec($cmd, $output, $returnCode);
 
-		shell_exec($exec);
+		if ($returnCode !== 0) {
+			$this->cleanTmpFiles();
+			return null;
+		}
 
 		//create imagick object from png
 		$pngPreview = null;
@@ -74,7 +84,7 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		}
 
 		$image = new \OC_Image();
-		$image->loadFromData($png);
+		$image->loadFromData((string) $png);
 
 		$this->cleanTmpFiles();
 		unlink($pngPreview);
@@ -86,29 +96,4 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		}
 		return null;
 	}
-
-	private function initCmd() {
-		$cmd = '';
-
-		$libreOfficePath = \OC::$server->getConfig()->getSystemValue('preview_libreoffice_path', null);
-		if (is_string($libreOfficePath)) {
-			$cmd = $libreOfficePath;
-		}
-
-		$whichLibreOffice = shell_exec('command -v libreoffice');
-		if ($cmd === '' && !empty($whichLibreOffice)) {
-			$cmd = 'libreoffice';
-		}
-
-		$whichOpenOffice = shell_exec('command -v openoffice');
-		if ($cmd === '' && !empty($whichOpenOffice)) {
-			$cmd = 'openoffice';
-		}
-
-		if ($cmd === '') {
-			$cmd = null;
-		}
-
-		$this->cmd = $cmd;
-	}
 }
diff --git a/lib/private/Preview/ProviderV2.php b/lib/private/Preview/ProviderV2.php
@@ -31,8 +31,10 @@
 use OCP\Preview\IProviderV2;
 
 abstract class ProviderV2 implements IProviderV2 {
-	private $options;
+	/** @var array */
+	protected $options;
 
+	/** @var array */
 	private $tmpFiles = [];
 
 	/**

diff --git a/lib/private/Preview/TXT.php b/lib/private/Preview/TXT.php
@@ -52,6 +52,10 @@ public function isAvailable(FileInfo $file): bool {
 	 * {@inheritDoc}
 	 */
 	public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
+		if (!$this->isAvailable($file)) {
+			return null;
+		}
+
 		$content = $file->fopen('r');
 
 		if ($content === false) {

diff --git a/lib/private/PreviewManager.php b/lib/private/PreviewManager.php
@@ -417,41 +417,34 @@ protected function registerCoreProviders() {
 			}
 
 			if (count($checkImagick->queryFormats('PDF')) === 1) {
-				if (\OC_Helper::is_function_enabled('shell_exec')) {
-					$officeFound = is_string($this->config->getSystemValue('preview_libreoffice_path', null));
-
-					if (!$officeFound) {
-						//let's see if there is libreoffice or openoffice on this machine
-						$whichLibreOffice = shell_exec('command -v libreoffice');
-						$officeFound = !empty($whichLibreOffice);
-						if (!$officeFound) {
-							$whichOpenOffice = shell_exec('command -v openoffice');
-							$officeFound = !empty($whichOpenOffice);
-						}
-					}
+				// Office requires openoffice or libreoffice
+				$officeBinary = $this->config->getSystemValue('preview_libreoffice_path', null);
+				if (is_null($officeBinary)) {
+					$officeBinary = \OC_Helper::findBinaryPath('libreoffice');
+				}
+				if (is_null($officeBinary)) {
+					$officeBinary = \OC_Helper::findBinaryPath('openoffice');
+				}
 
-					if ($officeFound) {
-						$this->registerCoreProvider(Preview\MSOfficeDoc::class, '/application\/msword/');
-						$this->registerCoreProvider(Preview\MSOffice2003::class, '/application\/vnd.ms-.*/');
-						$this->registerCoreProvider(Preview\MSOffice2007::class, '/application\/vnd.openxmlformats-officedocument.*/');
-						$this->registerCoreProvider(Preview\OpenDocument::class, '/application\/vnd.oasis.opendocument.*/');
-						$this->registerCoreProvider(Preview\StarOffice::class, '/application\/vnd.sun.xml.*/');
-					}
+				if (is_string($officeBinary)) {
+					$this->registerCoreProvider(Preview\MSOfficeDoc::class, '/application\/msword/', ["officeBinary" => $officeBinary]);
+					$this->registerCoreProvider(Preview\MSOffice2003::class, '/application\/vnd.ms-.*/', ["officeBinary" => $officeBinary]);
+					$this->registerCoreProvider(Preview\MSOffice2007::class, '/application\/vnd.openxmlformats-officedocument.*/', ["officeBinary" => $officeBinary]);
+					$this->registerCoreProvider(Preview\OpenDocument::class, '/application\/vnd.oasis.opendocument.*/', ["officeBinary" => $officeBinary]);
+					$this->registerCoreProvider(Preview\StarOffice::class, '/application\/vnd.sun.xml.*/', ["officeBinary" => $officeBinary]);
 				}
 			}
 		}
 
 		// Video requires avconv or ffmpeg
 		if (in_array(Preview\Movie::class, $this->getEnabledDefaultProvider())) {
-			$avconvBinary = \OC_Helper::findBinaryPath('avconv');
-			$ffmpegBinary = $avconvBinary ? null : \OC_Helper::findBinaryPath('ffmpeg');
-
-			if ($avconvBinary || $ffmpegBinary) {
-				// FIXME // a bit hacky but didn't want to use subclasses
-				\OC\Preview\Movie::$avconvBinary = $avconvBinary;
-				\OC\Preview\Movie::$ffmpegBinary = $ffmpegBinary;
+			$movieBinary = \OC_Helper::findBinaryPath('avconv');
+			if (is_null($movieBinary)) {
+				$movieBinary = \OC_Helper::findBinaryPath('ffmpeg');
+			}
 
-				$this->registerCoreProvider(Preview\Movie::class, '/video\/.*/');
+			if (is_string($movieBinary)) {
+				$this->registerCoreProvider(Preview\Movie::class, '/video\/.*/', ["movieBinary" => $movieBinary]);
 			}
 		}
 	}