Merge pull request #7692 from nextcloud/dont_lie_preview_mimetype

Don't lie about the preview mimetype

4 files changed, 76 insertions, 9 deletions

diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php
index 402732ecda9..70a831530ed 100644
--- a/lib/private/Preview/Generator.php
+++ b/lib/private/Preview/Generator.php
@@ -128,9 +128,13 @@ class Generator {
 
 		// Try to get a cached preview. Else generate (and store) one
 		try {
-			$file = $this->getCachedPreview($previewFolder, $width, $height, $crop);
-		} catch (NotFoundException $e) {
-			$file = $this->generatePreview($previewFolder, $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight);
+			try {
+				$file = $this->getCachedPreview($previewFolder, $width, $height, $crop, $maxPreview->getMimeType());
+			} catch (NotFoundException $e) {
+				$file = $this->generatePreview($previewFolder, $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight);
+			}
+		} catch (\InvalidArgumentException $e) {
+			throw new NotFoundException();
 		}
 
 		return $file;
@@ -173,7 +177,15 @@ class Generator {
 					continue;
 				}
 
-				$path = (string)$preview->width() . '-' . (string)$preview->height() . '-max.png';
+				// Try to get the extention.
+				try {
+					$ext = $this->getExtention($preview->dataMimeType());
+				} catch (\InvalidArgumentException $e) {
+					// Just continue to the next iteration if this preview doesn't have a valid mimetype
+					continue;
+				}
+
+				$path = (string)$preview->width() . '-' . (string)$preview->height() . '-max.' . $ext;
 				try {
 					$file = $previewFolder->newFile($path);
 					$file->putContent($preview->data());
@@ -201,14 +213,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;
 	}
 
@@ -340,7 +355,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());
@@ -356,12 +371,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);
 	}
@@ -381,4 +397,22 @@ class Generator {
 
 		return $folder;
 	}
+
+	/**
+	 * @param string $mimeType
+	 * @return null|string
+	 * @throws \InvalidArgumentException
+	 */
+	private function getExtention($mimeType) {
+		switch ($mimeType) {
+			case 'image/png':
+				return 'png';
+			case 'image/jpeg':
+				return 'jpg';
+			case 'image/gif':
+				return 'gif';
+			default:
+				throw new \InvalidArgumentException('Not a valid mimetype');
+		}
+	}
 }
diff --git a/lib/private/legacy/image.php b/lib/private/legacy/image.php
index a7d702ac032..a0159b927f9 100644
--- a/lib/private/legacy/image.php
+++ b/lib/private/legacy/image.php
@@ -305,6 +305,25 @@ class OC_Image implements \OCP\IImage {
 	}
 
 	/**
+	 * @return string Returns the mimetype of the data. Returns the empty string
+	 * if the data is not valid.
+	 */
+	public function dataMimeType() {
+		if (!$this->valid()) {
+			return '';
+		}
+
+		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.
 	 */
 	public function data() {
diff --git a/lib/public/IImage.php b/lib/public/IImage.php
index f63a1b8ca60..70e8b3cff75 100644
--- a/lib/public/IImage.php
+++ b/lib/public/IImage.php
@@ -103,6 +103,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 f1383b0691b..130cccdf09e 100644
--- 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')