Don't lie about the preview mimetype

For legacy reasons we stored all the previews with a png extention.
However we did not put png data in them all the time.

This caused the preview endpoints to always report that a preview is a
png file. Which was a lie.

Since we abstract away from the storage etc in the previewmanager. There
is no need anymore to store them as .png files and instead we can use
the actual file extention.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php
index 5a264c2bbd5432b34034e19385f422a4e4339ec8..4f9f7f3bc1ff0902859398045163822332f6080a 100644 (file)
--- a/lib/private/Preview/Generator.php
+++ b/lib/private/Preview/Generator.php
@@ -120,7 +120,7 @@ class Generator {
 
                // Try to get a cached preview. Else generate (and store) one
                try {
-                       $file = $this->getCachedPreview($previewFolder, $width, $height, $crop);
+                       $file = $this->getCachedPreview($previewFolder, $width, $height, $crop, $maxPreview->getMimeType());
                } catch (NotFoundException $e) {
                        $file = $this->generatePreview($previewFolder, $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight);
                }
@@ -165,7 +165,8 @@ class Generator {
                                        continue;
                                }
 
-                               $path = (string)$preview->width() . '-' . (string)$preview->height() . '-max.png';
+                               $ext = $this->getExtention($preview->dataMimeType());
+                               $path = (string)$preview->width() . '-' . (string)$preview->height() . '-max.' . $ext;
                                try {
                                        $file = $previewFolder->newFile($path);
                                        $file->putContent($preview->data());
@@ -193,14 +194,17 @@ class Generator {
         * @param int $width
         * @param int $height
         * @param bool $crop
+        * @param string $mimeType
         * @return string
         */
-       private function generatePath($width, $height, $crop) {
+       private function generatePath($width, $height, $crop, $mimeType) {
                $path = (string)$width . '-' . (string)$height;
                if ($crop) {
                        $path .= '-crop';
                }
-               $path .= '.png';
+
+               $ext = $this->getExtention($mimeType);
+               $path .= '.' . $ext;
                return $path;
        }
 
@@ -332,7 +336,7 @@ class Generator {
                }
 
 
-               $path = $this->generatePath($width, $height, $crop);
+               $path = $this->generatePath($width, $height, $crop, $preview->dataMimeType());
                try {
                        $file = $previewFolder->newFile($path);
                        $file->putContent($preview->data());
@@ -348,12 +352,13 @@ class Generator {
         * @param int $width
         * @param int $height
         * @param bool $crop
+        * @param string $mimeType
         * @return ISimpleFile
         *
         * @throws NotFoundException
         */
-       private function getCachedPreview(ISimpleFolder $previewFolder, $width, $height, $crop) {
-               $path = $this->generatePath($width, $height, $crop);
+       private function getCachedPreview(ISimpleFolder $previewFolder, $width, $height, $crop, $mimeType) {
+               $path = $this->generatePath($width, $height, $crop, $mimeType);
 
                return $previewFolder->getFile($path);
        }
@@ -373,4 +378,21 @@ class Generator {
 
                return $folder;
        }
+
+       /**
+        * @param string $mimeType
+        * @return null|string
+        */
+       private function getExtention($mimeType) {
+               switch ($mimeType) {
+                       case 'image/png':
+                               return 'png';
+                       case 'image/jpeg':
+                               return 'jpg';
+                       case 'image/gif':
+                               return 'gif';
+                       default:
+                               return null;
+               }
+       }
 }

diff --git a/lib/private/legacy/image.php b/lib/private/legacy/image.php
index 120b19d1cff12c37f827066631c038ec5c4abca3..eb7baf5134cd3566d4f0a8c459f09af37120209f 100644 (file)
--- a/lib/private/legacy/image.php
+++ b/lib/private/legacy/image.php
@@ -317,6 +317,24 @@ class OC_Image implements \OCP\IImage {
                return $this->resource;
        }
 
+       /**
+        * @return null|string Returns the mimetype of the data
+        */
+       public function dataMimeType() {
+               if (!$this->valid()) {
+                       return null;
+               }
+
+               switch ($this->mimeType) {
+                       case 'image/png':
+                       case 'image/jpeg':
+                       case 'image/gif':
+                               return $this->mimeType;
+                       default:
+                               return 'image/png';
+               }
+       }
+
        /**
         * @return null|string Returns the raw image data.
         */

diff --git a/lib/public/IImage.php b/lib/public/IImage.php
index f63a1b8ca608db1ddc3b791c393a2b4f22669677..70e8b3cff751ee9ea14fcadf6c6118eff8e9f0e1 100644 (file)
--- a/lib/public/IImage.php
+++ b/lib/public/IImage.php
@@ -102,6 +102,12 @@ interface IImage {
         */
        public function resource();
 
+       /**
+        * @return string Returns the raw data mimetype
+        * @since 13.0.0
+        */
+       public function dataMimeType();
+
        /**
         * @return string Returns the raw image data.
         * @since 8.1.0

diff --git a/tests/lib/Preview/GeneratorTest.php b/tests/lib/Preview/GeneratorTest.php
index f1383b0691b4e583a449d01b9f705d094b0cfbba..130cccdf09e2f9b5c1e1b8b93511f9d9646ae65f 100644 (file)
--- a/tests/lib/Preview/GeneratorTest.php
+++ b/tests/lib/Preview/GeneratorTest.php
@@ -93,6 +93,8 @@ class GeneratorTest extends \Test\TestCase {
                $maxPreview = $this->createMock(ISimpleFile::class);
                $maxPreview->method('getName')
                        ->willReturn('1000-1000-max.png');
+               $maxPreview->method('getMimeType')
+                       ->willReturn('image/png');
 
                $previewFolder->method('getDirectoryListing')
                        ->willReturn([$maxPreview]);
@@ -170,6 +172,7 @@ class GeneratorTest extends \Test\TestCase {
                $image->method('width')->willReturn(2048);
                $image->method('height')->willReturn(2048);
                $image->method('valid')->willReturn(true);
+               $image->method('dataMimeType')->willReturn('image/png');
 
                $this->helper->method('getThumbnail')
                        ->will($this->returnCallback(function ($provider, $file, $x, $y) use ($invalidProvider, $validProvider, $image) {
@@ -185,6 +188,7 @@ class GeneratorTest extends \Test\TestCase {
 
                $maxPreview = $this->createMock(ISimpleFile::class);
                $maxPreview->method('getName')->willReturn('2048-2048-max.png');
+               $maxPreview->method('getMimeType')->willReturn('image/png');
 
                $previewFile = $this->createMock(ISimpleFile::class);
 
@@ -219,6 +223,7 @@ class GeneratorTest extends \Test\TestCase {
                $image->method('data')
                        ->willReturn('my resized data');
                $image->method('valid')->willReturn(true);
+               $image->method('dataMimeType')->willReturn('image/png');
 
                $previewFile->expects($this->once())
                        ->method('putContent')
@@ -362,6 +367,8 @@ class GeneratorTest extends \Test\TestCase {
                $maxPreview = $this->createMock(ISimpleFile::class);
                $maxPreview->method('getName')
                        ->willReturn($maxX . '-' . $maxY . '-max.png');
+               $maxPreview->method('getMimeType')
+                       ->willReturn('image/png');
 
                $previewFolder->method('getDirectoryListing')
                        ->willReturn([$maxPreview]);
@@ -382,6 +389,7 @@ class GeneratorTest extends \Test\TestCase {
                $image->method('height')->willReturn($maxY);
                $image->method('width')->willReturn($maxX);
                $image->method('valid')->willReturn(true);
+               $image->method('dataMimeType')->willReturn('image/png');
 
                $preview = $this->createMock(ISimpleFile::class);
                $previewFolder->method('newFile')