Use findBinaryPath for previews

Signed-off-by: J0WI <J0WI@users.noreply.github.com>

diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
index 9ec176a4ae9b8b25d720415250e1a08a42505071..7a36e88eca3b01459de2cbb568a0441fe701b585 100644 (file)
--- 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
index 68c83fad97d395f2df10fd6aa3817a9064f37540..f2d43564c99ae1050a3901368dfb1b14e34cb704 100644 (file)
--- a/lib/private/Preview/HEIC.php
+++ b/lib/private/Preview/HEIC.php
@@ -30,6 +30,7 @@ declare(strict_types=1);
 namespace OC\Preview;
 
 use OCP\Files\File;
+use OCP\Files\FileInfo;
 use OCP\IImage;
 use OCP\ILogger;
 
@@ -49,7 +50,7 @@ class HEIC extends ProviderV2 {
        /**
         * {@inheritDoc}
         */
-       public function isAvailable(\OCP\Files\FileInfo $file): bool {
+       public function isAvailable(FileInfo $file): bool {
                return in_array('HEIC', \Imagick::queryFormats("HEI*"));
        }
 
@@ -57,6 +58,10 @@ class HEIC extends ProviderV2 {
         * {@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
index b139758596cb4b31e6543535c9f3fad6f8172694..eab81e1ed4876422a2b01fa522acf22cbcfe08ae 100644 (file)
--- a/lib/private/Preview/Movie.php
+++ b/lib/private/Preview/Movie.php
@@ -30,13 +30,27 @@
 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}
         */
@@ -44,12 +58,33 @@ class Movie extends ProviderV2 {
                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 @@ class Movie extends ProviderV2 {
        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
index fef218823e21d42e30dc4c63e38069ecbb443c78..b16544b3b233b41edb11eb6ae56b5656f45a6034 100644 (file)
--- 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 @@ abstract class Office extends ProviderV2 {
                $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 @@ abstract class Office extends ProviderV2 {
                }
 
                $image = new \OC_Image();
-               $image->loadFromData($png);
+               $image->loadFromData((string) $png);
 
                $this->cleanTmpFiles();
                unlink($pngPreview);
@@ -86,29 +96,4 @@ abstract class Office extends ProviderV2 {
                }
                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
index 1b5bee0e5cc52119345099126259b4c4f95d1c4e..4323f149702d00be6f73cd8a99160bef433fb2cb 100644 (file)
--- a/lib/private/Preview/ProviderV2.php
+++ b/lib/private/Preview/ProviderV2.php
@@ -31,8 +31,10 @@ use OCP\IImage;
 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
index 0ae35fb3eec13f2e8087673f54fac6bec97c0091..d55d32386a7af357ebcc5a828a180653f4858142 100644 (file)
--- a/lib/private/Preview/TXT.php
+++ b/lib/private/Preview/TXT.php
@@ -52,6 +52,10 @@ class TXT extends ProviderV2 {
         * {@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
index 02d017a5017ebf3e469c3e73cabf06d5b0e92269..18d4427d34670532f88a82ee1a84bf686b9c703e 100644 (file)
--- a/lib/private/PreviewManager.php
+++ b/lib/private/PreviewManager.php
@@ -417,41 +417,34 @@ class PreviewManager implements IPreview {
                        }
 
                        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]);
                        }
                }
        }