GDToolkit::load() doesn't check value, returned from executing $function

Created on 1 March 2024, 11 months ago
Updated 6 September 2024, 5 months ago

Problem/Motivation

Drupal version is 10.2.3, PHP is 8.2.
Function, called inside GDToolkit::load() may return FALSE in case of error. System checks execution with try/catch, but doesn't check, if $image is not FALSE. This leads to the error TypeError: Drupal\system\Plugin\ImageToolkit\GDToolkit::setImage(): Argument #1 ($image) must be of type ?GdImage, bool given, called in /web/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php on line 284 in Drupal\system\Plugin\ImageToolkit\GDToolkit->setImage() (line 201 of /web/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php)..

Steps to reproduce

I don't have exact steps to reproduce, because locally I can get error only if I hacking code in order to raise such situation.

Proposed resolution

Add checking for boolean result of the function.

Remaining tasks

provide MR/patch, review, give feedback, merge if possible.

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Image system 

Last updated 1 day ago

Created by

🇷🇺Russia ilya.no

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

  • Issue created by @ilya.no
  • Status changed to Needs review 11 months ago
  • 🇷🇺Russia ilya.no

    Attaching initial patch.

  • Status changed to Needs work 11 months ago
  • 🇺🇸United States smustgrave

    Thanks for reporting.

    Will need a failing test to show the issue as next steps

  • 🇮🇹Italy plach Venezia
    +++ b/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php
    @@ -281,6 +281,16 @@ protected function load() {
           return FALSE;
    ...
    +      $this->logger->error("The image toolkit '@toolkit' failed loading image '@image'. Failed function: @function.", [
    +        '@toolkit' => $this->getPluginId(),
    +        '@image' => $this->getSource(),
    +        '@function' => $function,
    +      ]);
    +      $this->preLoadInfo = NULL;
    +      return FALSE;
    

    Instead of duplicating this logic, why don't we just set $image to FALSE in the catch branch?

  • First commit to issue fork.
  • 🇪🇪Estonia tormi Tallinn

    I followed platch's suggestion in #4, also created a separate static patch, didn't change the status (no tests yet).

  • 🇪🇪Estonia tormi Tallinn

    Here's my stack trace from local environment.

    The size of the `public://images-2024-09/IG-post_Foli-kumppani (1).png` is 18MB and `createDerivative` fails because of this - there's no derivate in https://xxx.lndo.site/sites/default/files/styles/medium/public/images-20...

    The website encountered an unexpected error. Try again later.

    TypeError: Drupal\system\Plugin\ImageToolkit\GDToolkit::setImage(): Argument #1 ($image) must be of type ?GdImage, bool given, called in /app/web/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php on line 287 in Drupal\system\Plugin\ImageToolkit\GDToolkit->setImage() (line 203 of core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php).
    Drupal\system\Plugin\ImageToolkit\GDToolkit->load() (Line: 217)
    Drupal\system\Plugin\ImageToolkit\GDToolkit->getImage() (Line: 354)
    Drupal\system\Plugin\ImageToolkit\GDToolkit->save('/app/web/sites/default/files/styles/medium/public/images-2024-09/IG-post_Foli-kumppani (1).png') (Line: 125)
    Drupal\Core\Image\Image->save('public://styles/medium/public/images-2024-09/IG-post_Foli-kumppani (1).png') (Line: 334)
    Drupal\image\Entity\ImageStyle->createDerivative('public://images-2024-09/IG-post_Foli-kumppani (1).png', 'public://styles/medium/public/images-2024-09/IG-post_Foli-kumppani (1).png') (Line: 224)
    Drupal\image\Controller\ImageStyleDownloadController->deliver(Object, 'public', Object, 'public') (Line: 90)
    Drupal\stage_file_proxy\Controller\ImageStyleDownloadController->deliver(Object, 'public', Object, 'public')
    call_user_func_array(Array, Array) (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638)
    Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
    Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
    Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 53)
    Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
    Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
    Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 48)
    Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
    Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
    Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
    Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 741)
    Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

    And here's the image creation failure stack trace:

    Warning: imagecreatefrompng(): "public://images-2024-06/IG-post_Foli-kumppani.png" is not a valid PNG file in Drupal\system\Plugin\ImageToolkit\GDToolkit->load() (line 273 of /app/web/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php)
    #0 /app/web/core/includes/bootstrap.inc(166): _drupal_error_handler_real(2, 'imagecreatefrom...', '/app/web/core/m...', 273)
    #1 [internal function]: _drupal_error_handler(2, 'imagecreatefrom...', '/app/web/core/m...', 273)
    #2 /app/web/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php(273): imagecreatefrompng('public://images...')
    #3 /app/web/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php(217): Drupal\system\Plugin\ImageToolkit\GDToolkit->load()
    #4 /app/web/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php(354): Drupal\system\Plugin\ImageToolkit\GDToolkit->getImage()
    #5 /app/web/core/lib/Drupal/Core/Image/Image.php(125): Drupal\system\Plugin\ImageToolkit\GDToolkit->save('/app/web/sites/...')
    #6 /app/web/core/modules/image/src/Entity/ImageStyle.php(334): Drupal\Core\Image\Image->save('public://styles...')
    #7 /app/web/core/modules/image/src/Controller/ImageStyleDownloadController.php(224): Drupal\image\Entity\ImageStyle->createDerivative('public://images...', 'public://styles...')
    #8 /app/web/modules/contrib/stage_file_proxy/src/Controller/ImageStyleDownloadController.php(90): Drupal\image\Controller\ImageStyleDownloadController->deliver(Object(Symfony\Component\HttpFoundation\Request), 'public', Object(Drupal\image\Entity\ImageStyle), 'public')
    #9 [internal function]: Drupal\stage_file_proxy\Controller\ImageStyleDownloadController->deliver(Object(Symfony\Component\HttpFoundation\Request), 'public', Object(Drupal\image\Entity\ImageStyle), 'public')
    #10 /app/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)
    #11 /app/web/core/lib/Drupal/Core/Render/Renderer.php(638): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
    #12 /app/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(121): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
    #13 /app/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array)
    #14 /app/vendor/symfony/http-kernel/HttpKernel.php(181): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
    #15 /app/vendor/symfony/http-kernel/HttpKernel.php(76): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
    #16 /app/web/core/lib/Drupal/Core/StackMiddleware/Session.php(53): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
    #17 /app/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
    #18 /app/web/core/lib/Drupal/Core/StackMiddleware/ContentLength.php(28): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
    #19 /app/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\Core\StackMiddleware\ContentLength->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
    #20 /app/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
    #21 /app/web/core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php(36): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
    #22 /app/web/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\Core\StackMiddleware\AjaxPageState->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
    #23 /app/web/core/lib/Drupal/Core/DrupalKernel.php(741): Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
    #24 /app/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
    #25 {main}

    This might be related to https://www.drupal.org/project/drupal/issues/3261924 🐛 Avoid warning from imagecreatefrompng when loading png with obscure iCCP profiles Needs work

  • 🇪🇪Estonia tormi Tallinn

    Okay, this did the trick for me: max_filesize: '10 MB'.

Production build 0.71.5 2024