From 50b880edeba3166cd7718c1fb3447f618194337b Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 4 Jan 2018 10:00:07 +0100 Subject: [PATCH] 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 --- lib/private/Preview/Generator.php | 36 +++++++++++++++++++++++------ lib/private/legacy/image.php | 18 +++++++++++++++ lib/public/IImage.php | 6 +++++ tests/lib/Preview/GeneratorTest.php | 8 +++++++ 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 5a264c2bbd5..4f9f7f3bc1f 100644 --- 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 120b19d1cff..eb7baf5134c 100644 --- 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 f63a1b8ca60..70e8b3cff75 100644 --- 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 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') -- 2.39.5