fix(Images): handle unsupported or missing extension in ImageMagickHandler::save()

docs/changelogs/v4.7.7.md

@@ -17,6 +17,7 @@ Release Date: Unreleased
 ## Bugs Fixed
 
 - **HTTP:** Fixed a bug where `UploadedFile::move()` and `UploadedFile::store()` on Windows could store a file under a different name than `getName()` reported, or write its contents to an NTFS alternate data stream. Filenames ending in a period or space or containing a colon are now rejected, including explicitly supplied names.
+- **Images:** Fixed a bug in `ImageMagickHandler::save()` where saving without an extension or with an unsupported extension threw an `ImagickException` instead of falling back to the image's format.
 - **Security:** Fixed a bug where `Security::__construct()` threw a `TypeError` when the CSRF cookie was an array, causing an HTTP 500 response for affected requests.
 - **Worker Mode:** Fixed a bug where every request that ended with an uncaught exception, including a `404 Page Not Found` without a 404 override, called `exit()` in the exception handler. This ended the worker script, so FrankenPHP booted a new one, and the worker loop skipped its cleanup after the request. The exception handler now returns to the worker loop.
 

docs/libraries/images.md

@@ -86,7 +86,7 @@ This example would take the same image and first fix any mobile phone orientatio
     The parameter `$quality` for WebP can be used since v4.4.0.
 
 !!! note "Note"
-    When using ImageMagick, the target path passed to `save()` determines the output format by its extension. ImageMagick may also interpret explicit format prefixes like `jpg:filename.png`. If the target path is based on user input, validate it against the formats your application allows, including any format prefix or extension.
+    When using ImageMagick, the target path passed to `save()` determines the output format by its extension. If the target has no extension or an unsupported extension, the handler falls back to the image's `$imageType`. ImageMagick may also interpret explicit format prefixes like `jpg:filename.png`. If the target path is based on user input, validate it against the formats your application allows, including any format prefix or extension.
 
 ```php
 <?php
@@ -192,7 +192,7 @@ convert(int $imageType)
     ```
 
 !!! note "Note"
-    ImageMagick already saves files in the type indicated by their extension, ignoring `$imageType`.
+    When using ImageMagick, a supported target filename extension determines the output format and takes precedence over `$imageType`. If the target has no extension or an unsupported extension, the handler uses `$imageType` instead.
 
 ### Fitting Images
 

system/Images/Handlers/ImageMagickHandler.php

@@ -321,9 +321,20 @@ public function save(?string $target = null, int $quality = 90): bool
         $this->resource->setImageCompressionQuality($quality);
 
         $extension = pathinfo($target, PATHINFO_EXTENSION);
-        $this->resource->setImageFormat($extension);
 
         try {
+            $formats = Imagick::queryFormats();
+
+            if ($extension !== '' && in_array(strtoupper($extension), $formats, true)) {
+                $this->resource->setImageFormat($extension);
+            } elseif ($this->image()->imageType !== 0) {
+                $format = image_type_to_extension($this->image()->imageType, false);
+
+                if ($format !== false && in_array(strtoupper($format), $formats, true)) {
+                    $this->resource->setImageFormat($format);
+                }
+            }
+
             $result = $this->resource->writeImage($target);
 
             chmod($target, $this->filePermissions);

tests/system/Images/ImageMagickHandlerTest.php

@@ -408,6 +408,59 @@ public function testImageConvert(): void
         $this->assertSame(IMAGETYPE_PNG, exif_imagetype($this->root . 'ci-logo.png'));
     }
 
+    public function testSaveWithoutExtension(): void
+    {
+        $this->handler->withFile($this->origin . 'ci-logo.jpeg');
+        $this->handler->resize(100, 100);
+        $target = $this->root . 'ci-logo-no-ext';
+        $this->handler->save($target);
+
+        $this->assertFileExists($target);
+        $this->assertSame(IMAGETYPE_JPEG, exif_imagetype($target));
+    }
+
+    public function testSaveWithUnsupportedExtension(): void
+    {
+        $this->handler->withFile($this->origin . 'ci-logo.jpeg');
+        $this->handler->resize(100, 100);
+        $target = $this->root . 'ci-logo.tmp';
+        $this->handler->save($target);
+
+        $this->assertFileExists($target);
+        $this->assertSame(IMAGETYPE_JPEG, exif_imagetype($target));
+    }
+
+    public function testSaveWithoutExtensionAfterConvert(): void
+    {
+        $this->handler->withFile($this->origin . 'ci-logo.jpeg');
+        $this->handler->convert(IMAGETYPE_PNG);
+        $target = $this->root . 'ci-logo-converted-no-ext';
+        $this->handler->save($target);
+
+        $this->assertFileExists($target);
+        $this->assertSame(IMAGETYPE_PNG, exif_imagetype($target));
+    }
+
+    public function testSaveWithUnsupportedExtensionAfterConvert(): void
+    {
+        $this->handler->withFile($this->origin . 'ci-logo.jpeg');
+        $this->handler->convert(IMAGETYPE_PNG);
+        $target = $this->root . 'ci-logo-converted.unsupported';
+        $this->handler->save($target);
+
+        $this->assertFileExists($target);
+        $this->assertSame(IMAGETYPE_PNG, exif_imagetype($target));
+    }
+
+    public function testSaveFailedThrowsImageException(): void
+    {
+        $this->expectException(ImageException::class);
+        $this->expectExceptionMessage('Unable to save the image. Please make sure the image and file directory are writable.');
+
+        $this->handler->withFile($this->origin . 'ci-logo.jpeg');
+        $this->handler->save('/non_existent_directory_xyz/ci-logo.jpg');
+    }
+
     public function testImageReorientLandscape(): void
     {
         for ($i = 0; $i <= 8; $i++) {