From 23edd01fb43889726fe92b5d6fce701af96db907 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Fri, 30 Dec 2022 16:35:53 +0000
Subject: [PATCH] Issue #2583041 by mondrake, fietserwin, claudiu.cristea,
alexpott, catch: GD toolkit & operations should catch \Throwable to fail
gracefully in case of errors
---
.../Core/ImageToolkit/ImageToolkitBase.php | 10 +-
.../ImageToolkitOperationBase.php | 3 +
.../src/Plugin/ImageToolkit/GDToolkit.php | 93 ++++++++++++++-----
.../KernelTests/Core/Image/ToolkitGdTest.php | 33 +++++++
4 files changed, 113 insertions(+), 26 deletions(-)
diff --git a/core/lib/Drupal/Core/ImageToolkit/ImageToolkitBase.php b/core/lib/Drupal/Core/ImageToolkit/ImageToolkitBase.php
index ce0e283c8efc..1fba36080485 100644
--- a/core/lib/Drupal/Core/ImageToolkit/ImageToolkitBase.php
+++ b/core/lib/Drupal/Core/ImageToolkit/ImageToolkitBase.php
@@ -127,8 +127,14 @@ public function apply($operation, array $arguments = []) {
$this->logger->error("The selected image handling toolkit '@toolkit' can not process operation '@operation'.", ['@toolkit' => $this->getPluginId(), '@operation' => $operation]);
return FALSE;
}
- catch (\InvalidArgumentException $e) {
- $this->logger->warning($e->getMessage(), []);
+ catch (\Throwable $t) {
+ $this->logger->warning("The image toolkit '@toolkit' failed processing '@operation' for image '@image'. Reported error: @class - @message", [
+ '@toolkit' => $this->getPluginId(),
+ '@operation' => $operation,
+ '@image' => $this->getSource(),
+ '@class' => get_class($t),
+ '@message' => $t->getMessage(),
+ ]);
return FALSE;
}
}
diff --git a/core/lib/Drupal/Core/ImageToolkit/ImageToolkitOperationBase.php b/core/lib/Drupal/Core/ImageToolkit/ImageToolkitOperationBase.php
index dcc3b3d12134..95b8a9c4f788 100644
--- a/core/lib/Drupal/Core/ImageToolkit/ImageToolkitOperationBase.php
+++ b/core/lib/Drupal/Core/ImageToolkit/ImageToolkitOperationBase.php
@@ -185,6 +185,9 @@ final public function apply(array $arguments) {
*
* @return bool
* TRUE if the operation was performed successfully, FALSE otherwise.
+ *
+ * @throws \RuntimeException
+ * If the operation can not be performed.
*/
abstract protected function execute(array $arguments);
diff --git a/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php b/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php
index 2753273a4132..2abda1bdbecf 100644
--- a/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php
+++ b/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php
@@ -180,31 +180,54 @@ protected function load() {
return FALSE;
}
+ // Invalidate the image object and return if there's no function to load the
+ // image file.
$function = 'imagecreatefrom' . image_type_to_extension($this->getType(), FALSE);
- if (function_exists($function) && $resource = $function($this->getSource())) {
- $this->setResource($resource);
- if (imageistruecolor($resource)) {
- return TRUE;
- }
- else {
- // Convert indexed images to truecolor, copying the image to a new
- // truecolor resource, so that filters work correctly and don't result
- // in unnecessary dither.
- $data = [
- 'width' => imagesx($resource),
- 'height' => imagesy($resource),
- 'extension' => image_type_to_extension($this->getType(), FALSE),
- 'transparent_color' => $this->getTransparentColor(),
- 'is_temp' => TRUE,
- ];
- if ($this->apply('create_new', $data)) {
- imagecopy($this->getResource(), $resource, 0, 0, 0, 0, imagesx($resource), imagesy($resource));
- imagedestroy($resource);
- }
+ if (!function_exists($function)) {
+ $this->logger->error("The image toolkit '@toolkit' can not process image '@image'.", [
+ '@toolkit' => $this->getPluginId(),
+ '@image' => $this->getSource(),
+ ]);
+ $this->preLoadInfo = NULL;
+ return FALSE;
+ }
+
+ // Invalidate the image object and return if the load fails.
+ try {
+ $resource = $function($this->getSource());
+ }
+ catch (\Throwable $t) {
+ $this->logger->error("The image toolkit '@toolkit' failed loading image '@image'. Reported error: @class - @message", [
+ '@toolkit' => $this->getPluginId(),
+ '@image' => $this->getSource(),
+ '@class' => get_class($t),
+ '@message' => $t->getMessage(),
+ ]);
+ $this->preLoadInfo = NULL;
+ return FALSE;
+ }
+
+ $this->setResource($resource);
+ if (imageistruecolor($resource)) {
+ return TRUE;
+ }
+ else {
+ // Convert indexed images to truecolor, copying the image to a new
+ // truecolor resource, so that filters work correctly and don't result
+ // in unnecessary dither.
+ $data = [
+ 'width' => imagesx($resource),
+ 'height' => imagesy($resource),
+ 'extension' => image_type_to_extension($this->getType(), FALSE),
+ 'transparent_color' => $this->getTransparentColor(),
+ 'is_temp' => TRUE,
+ ];
+ if ($this->apply('create_new', $data)) {
+ imagecopy($this->getResource(), $resource, 0, 0, 0, 0, imagesx($resource), imagesy($resource));
+ imagedestroy($resource);
}
- return (bool) $this->getResource();
}
- return FALSE;
+ return (bool) $this->getResource();
}
/**
@@ -236,7 +259,18 @@ public function save($destination) {
return FALSE;
}
if ($this->getType() == IMAGETYPE_JPEG) {
- $success = $function($this->getResource(), $destination, $this->configFactory->get('system.image.gd')->get('jpeg_quality'));
+ try {
+ $success = $function($this->getResource(), $destination, $this->configFactory->get('system.image.gd')->get('jpeg_quality'));
+ }
+ catch (\Throwable $t) {
+ $this->logger->error("The image toolkit '@toolkit' failed saving image '@image'. Reported error: @class - @message", [
+ '@toolkit' => $this->getPluginId(),
+ '@image' => $destination,
+ '@class' => get_class($t),
+ '@message' => $t->getMessage(),
+ ]);
+ $success = FALSE;
+ }
}
else {
// Image types that support alpha need to be saved accordingly.
@@ -244,7 +278,18 @@ public function save($destination) {
imagealphablending($this->getResource(), FALSE);
imagesavealpha($this->getResource(), TRUE);
}
- $success = $function($this->getResource(), $destination);
+ try {
+ $success = $function($this->getResource(), $destination);
+ }
+ catch (\Throwable $t) {
+ $this->logger->error("The image toolkit '@toolkit' failed saving image '@image'. Reported error: @class - @message", [
+ '@toolkit' => $this->getPluginId(),
+ '@image' => $destination,
+ '@class' => get_class($t),
+ '@message' => $t->getMessage(),
+ ]);
+ $success = FALSE;
+ }
}
// Move temporary local file to remote destination.
if (isset($permanent_destination) && $success) {
diff --git a/core/tests/Drupal/KernelTests/Core/Image/ToolkitGdTest.php b/core/tests/Drupal/KernelTests/Core/Image/ToolkitGdTest.php
index e4c44674f9a2..aaa3a9aa3123 100644
--- a/core/tests/Drupal/KernelTests/Core/Image/ToolkitGdTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Image/ToolkitGdTest.php
@@ -452,6 +452,39 @@ public function testCreateNewFailures(): void {
$this->assertTrue($image->isValid(), 'CreateNew with valid arguments validates the Image.');
}
+ /**
+ * Tests creation of an image that will exceed the memory limit.
+ */
+ public function testInsufficientMemory(): void {
+ // ini_get() may return -1 or null for memory_limit to indicate there is no
+ // limit set. In that case, we need to skip this test.
+ $size = ini_get('memory_limit');
+ if (!$size || (int) $size === -1) {
+ $this->markTestSkipped("There is no memory limit set in the PHP environment.");
+ }
+
+ $image = $this->imageFactory->get('core/tests/fixtures/files/image-test.png');
+
+ $oldGdImage = $image->getToolkit()->getResource();
+ $this->assertFalse($image->createNew(2000000, 2000000));
+ $newGdImage = $image->getToolkit()->getResource();
+
+ // Check that a new resource has not been created, and the old one is still
+ // valid.
+ $this->assertEquals($oldGdImage, $newGdImage);
+ }
+
+ /**
+ * Tests resizing of an image that will exceed the memory available.
+ */
+ public function testInsufficientAvailableMemory(): void {
+ $image = $this->imageFactory->get('core/tests/fixtures/files/image-test.png');
+
+ $memory_in_use = memory_get_usage(TRUE);
+ ini_set('memory_limit', $memory_in_use + 2048);
+ $this->assertFalse($image->resize(200000, 200000));
+ }
+
/**
* Tests for GIF images with transparency.
*/
--
GitLab