Merge pull request #4620 from nextcloud/preview-error-handling

better handling of preview generation errors

9 files changed, 26 insertions, 6 deletions

diff --git a/apps/files_sharing/lib/Controller/PublicPreviewController.php b/apps/files_sharing/lib/Controller/PublicPreviewController.php
index b91b84c9c34..49e48993f5c 100644
--- a/apps/files_sharing/lib/Controller/PublicPreviewController.php
+++ b/apps/files_sharing/lib/Controller/PublicPreviewController.php
@@ -97,6 +97,8 @@ class PublicPreviewController extends Controller {
 			return new FileDisplayResponse($f, Http::STATUS_OK, ['Content-Type' => $f->getMimeType()]);
 		} catch (NotFoundException $e) {
 			return new DataResponse([], Http::STATUS_NOT_FOUND);
+		} catch (\InvalidArgumentException $e) {
+			return new DataResponse([], Http::STATUS_BAD_REQUEST);
 		}
 	}
 }
diff --git a/apps/files_trashbin/lib/Controller/PreviewController.php b/apps/files_trashbin/lib/Controller/PreviewController.php
index c73b1c17c16..ae3a106d627 100644
--- a/apps/files_trashbin/lib/Controller/PreviewController.php
+++ b/apps/files_trashbin/lib/Controller/PreviewController.php
@@ -114,6 +114,8 @@ class PreviewController extends Controller {
 			return new Http\FileDisplayResponse($f, Http::STATUS_OK, ['Content-Type' => $f->getMimeType()]);
 		} catch (NotFoundException $e) {
 			return new DataResponse([], Http::STATUS_NOT_FOUND);
+		} catch (\InvalidArgumentException $e) {
+			return new DataResponse([], Http::STATUS_BAD_REQUEST);
 		}
 	}
 }
diff --git a/apps/files_versions/lib/Controller/PreviewController.php b/apps/files_versions/lib/Controller/PreviewController.php
index 8d961f47ee6..7a7c024fb52 100644
--- a/apps/files_versions/lib/Controller/PreviewController.php
+++ b/apps/files_versions/lib/Controller/PreviewController.php
@@ -94,6 +94,8 @@ class PreviewController extends Controller {
 			return new FileDisplayResponse($f, Http::STATUS_OK, ['Content-Type' => $f->getMimeType()]);
 		} catch (NotFoundException $e) {
 			return new DataResponse([], Http::STATUS_NOT_FOUND);
+		} catch (\InvalidArgumentException $e) {
+			return new DataResponse([], Http::STATUS_BAD_REQUEST);
 		}
 	}
 }
diff --git a/core/Controller/PreviewController.php b/core/Controller/PreviewController.php
index 4b9ba8b60d4..4e2b2f0287a 100644
--- a/core/Controller/PreviewController.php
+++ b/core/Controller/PreviewController.php
@@ -126,6 +126,8 @@ class PreviewController extends Controller {
 			return $response;
 		} catch (NotFoundException $e) {
 			return new DataResponse([], Http::STATUS_NOT_FOUND);
+		} catch (\InvalidArgumentException $e) {
+			return new DataResponse([], Http::STATUS_BAD_REQUEST);
 		}
 
 	}
diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php
index fd75e51b638..5a264c2bbd5 100644
--- a/lib/private/Preview/Generator.php
+++ b/lib/private/Preview/Generator.php
@@ -84,6 +84,7 @@ class Generator {
 	 * @param string $mimeType
 	 * @return ISimpleFile
 	 * @throws NotFoundException
+	 * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid)
 	 */
 	public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null) {
 		$this->eventDispatcher->dispatch(
@@ -299,10 +300,15 @@ class Generator {
 	 * @param int $maxHeight
 	 * @return ISimpleFile
 	 * @throws NotFoundException
+	 * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid)
 	 */
 	private function generatePreview(ISimpleFolder $previewFolder, ISimpleFile $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight) {
 		$preview = $this->helper->getImage($maxPreview);
 
+		if (!$preview->valid()) {
+			throw new \InvalidArgumentException('Failed to generate preview, failed to load image');
+		}
+
 		if ($crop) {
 			if ($height !== $preview->height() && $width !== $preview->width()) {
 				//Resize
@@ -325,6 +331,7 @@ class Generator {
 			$preview->resize(max($width, $height));
 		}
 
+
 		$path = $this->generatePath($width, $height, $crop);
 		try {
 			$file = $previewFolder->newFile($path);
diff --git a/lib/private/PreviewManager.php b/lib/private/PreviewManager.php
index 8c5a7ad29f1..12fcc292c63 100644
--- a/lib/private/PreviewManager.php
+++ b/lib/private/PreviewManager.php
@@ -182,7 +182,8 @@ class PreviewManager implements IPreview {
 	 * @param string $mimeType
 	 * @return ISimpleFile
 	 * @throws NotFoundException
-	 * @since 11.0.0
+	 * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid)
+	 * @since 11.0.0 - \InvalidArgumentException was added in 12.0.0
 	 */
 	public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null) {
 		if ($this->generator === null) {
diff --git a/lib/private/legacy/image.php b/lib/private/legacy/image.php
index e26148bdf15..120b19d1cff 100644
--- a/lib/private/legacy/image.php
+++ b/lib/private/legacy/image.php
@@ -563,7 +563,7 @@ class OC_Image implements \OCP\IImage {
 			case IMAGETYPE_JPEG:
 				if (imagetypes() & IMG_JPG) {
 					if (getimagesize($imagePath) !== false) {
-						$this->resource = imagecreatefromjpeg($imagePath);
+						$this->resource = @imagecreatefromjpeg($imagePath);
 					} else {
 						$this->logger->debug('OC_Image->loadFromFile, JPG image not valid: ' . $imagePath, array('app' => 'core'));
 					}
@@ -573,7 +573,7 @@ class OC_Image implements \OCP\IImage {
 				break;
 			case IMAGETYPE_PNG:
 				if (imagetypes() & IMG_PNG) {
-					$this->resource = imagecreatefrompng($imagePath);
+					$this->resource = @imagecreatefrompng($imagePath);
 					// Preserve transparency
 					imagealphablending($this->resource, true);
 					imagesavealpha($this->resource, true);
@@ -583,14 +583,14 @@ class OC_Image implements \OCP\IImage {
 				break;
 			case IMAGETYPE_XBM:
 				if (imagetypes() & IMG_XPM) {
-					$this->resource = imagecreatefromxbm($imagePath);
+					$this->resource = @imagecreatefromxbm($imagePath);
 				} else {
 					$this->logger->debug('OC_Image->loadFromFile, XBM/XPM images not supported: ' . $imagePath, array('app' => 'core'));
 				}
 				break;
 			case IMAGETYPE_WBMP:
 				if (imagetypes() & IMG_WBMP) {
-					$this->resource = imagecreatefromwbmp($imagePath);
+					$this->resource = @imagecreatefromwbmp($imagePath);
 				} else {
 					$this->logger->debug('OC_Image->loadFromFile, WBMP images not supported: ' . $imagePath, array('app' => 'core'));
 				}
diff --git a/lib/public/IPreview.php b/lib/public/IPreview.php
index 207539b1170..7705df61a17 100644
--- a/lib/public/IPreview.php
+++ b/lib/public/IPreview.php
@@ -104,7 +104,8 @@ interface IPreview {
 	 * @param string $mimeType To force a given mimetype for the file (files_versions needs this)
 	 * @return ISimpleFile
 	 * @throws NotFoundException
-	 * @since 11.0.0
+	 * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid)
+	 * @since 11.0.0 - \InvalidArgumentException was added in 12.0.0
 	 */
 	public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null);
 
diff --git a/tests/lib/Preview/GeneratorTest.php b/tests/lib/Preview/GeneratorTest.php
index 50d46ae932d..f1383b0691b 100644
--- a/tests/lib/Preview/GeneratorTest.php
+++ b/tests/lib/Preview/GeneratorTest.php
@@ -169,6 +169,7 @@ class GeneratorTest extends \Test\TestCase {
 		$image = $this->createMock(IImage::class);
 		$image->method('width')->willReturn(2048);
 		$image->method('height')->willReturn(2048);
+		$image->method('valid')->willReturn(true);
 
 		$this->helper->method('getThumbnail')
 			->will($this->returnCallback(function ($provider, $file, $x, $y) use ($invalidProvider, $validProvider, $image) {
@@ -217,6 +218,7 @@ class GeneratorTest extends \Test\TestCase {
 			->with(128);
 		$image->method('data')
 			->willReturn('my resized data');
+		$image->method('valid')->willReturn(true);
 
 		$previewFile->expects($this->once())
 			->method('putContent')
@@ -379,6 +381,7 @@ class GeneratorTest extends \Test\TestCase {
 			->willReturn($image);
 		$image->method('height')->willReturn($maxY);
 		$image->method('width')->willReturn($maxX);
+		$image->method('valid')->willReturn(true);
 
 		$preview = $this->createMock(ISimpleFile::class);
 		$previewFolder->method('newFile')