Merge pull request #3268 from michalsn/fix/image_save

Fix Image::save() when target value is null
codeigniter4 · Jul 11, 2020 · ee0c554 · ee0c554
2 parents 246ab82 + 8f9ef32
commit ee0c554
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 6 deletions.
diff --git a/system/Images/Handlers/GDHandler.php b/system/Images/Handlers/GDHandler.php
@@ -278,18 +278,26 @@ protected function process(string $action)
 	 */
 	public function save(string $target = null, int $quality = 90): bool
 	{
-		$target = empty($target) ? $this->image()->getPathname() : $target;
+		$original = $target;
+		$target   = empty($target) ? $this->image()->getPathname() : $target;
 
 		// If no new resource has been created, then we're
 		// simply copy the existing one.
-		if (empty($this->resource))
+		if (empty($this->resource) && $quality === 100)
 		{
+			if ($original === null)
+			{
+				return true;
+			}
+
 			$name = basename($target);
 			$path = pathinfo($target, PATHINFO_DIRNAME);
 
 			return $this->image()->copy($path, $name);
 		}
 
+		$this->ensureResource();
+
 		switch ($this->image()->imageType)
 		{
 			case IMAGETYPE_GIF:

diff --git a/system/Images/Handlers/ImageMagickHandler.php b/system/Images/Handlers/ImageMagickHandler.php
@@ -299,18 +299,26 @@ protected function process(string $action, int $quality = 100): array
 	 */
 	public function save(string $target = null, int $quality = 90): bool
 	{
-		$target = empty($target) ? $this->image() : $target;
+		$original = $target;
+		$target   = empty($target) ? $this->image()->getPathname() : $target;
 
 		// If no new resource has been created, then we're
 		// simply copy the existing one.
-		if (empty($this->resource))
+		if (empty($this->resource) && $quality === 100)
 		{
+			if ($original === null)
+			{
+				return true;
+			}
+
 			$name = basename($target);
 			$path = pathinfo($target, PATHINFO_DIRNAME);
 
 			return $this->image()->copy($path, $name);
 		}
 
+		$this->ensureResource();
+
 		// Copy the file through ImageMagick so that it has
 		// a chance to convert file format.
 		$action = escapeshellarg($this->resource) . ' ' . escapeshellarg($target);

diff --git a/tests/system/Images/GDHandlerTest.php b/tests/system/Images/GDHandlerTest.php
@@ -331,17 +331,38 @@ public function testImageCopy()
 	{
 		foreach (['gif', 'jpeg', 'png', 'webp'] as $type)
 		{
+			if ($type === 'webp' && ! function_exists('imagecreatefromwebp'))
+			{
+				$this->expectException(ImageException::class);
+				$this->expectExceptionMessage('Your server does not support the GD function required to process this type of image.');
+			}
+
 			$this->handler->withFile($this->origin . 'ci-logo.' . $type);
 			$this->handler->save($this->start . 'work/ci-logo.' . $type);
 			$this->assertTrue($this->root->hasChild('work/ci-logo.' . $type));
 
-			$this->assertEquals(
+			$this->assertNotEquals(
 				file_get_contents($this->origin . 'ci-logo.' . $type),
 				$this->root->getChild('work/ci-logo.' . $type)->getContent()
 			);
 		}
 	}
 
+	public function testImageCopyWithNoTargetAndMaxQuality()
+	{
+		foreach (['gif', 'jpeg', 'png', 'webp'] as $type)
+		{
+			$this->handler->withFile($this->origin . 'ci-logo.' . $type);
+			$this->handler->save(null, 100);
+			$this->assertTrue(file_exists($this->origin . 'ci-logo.' . $type));
+
+			$this->assertEquals(
+				file_get_contents($this->origin . 'ci-logo.' . $type),
+				file_get_contents($this->origin . 'ci-logo.' . $type)
+			);
+		}
+	}
+
 	public function testImageCompressionGetResource()
 	{
 		foreach (['gif', 'jpeg', 'png', 'webp'] as $type)

diff --git a/tests/system/Images/ImageMagickHandlerTest.php b/tests/system/Images/ImageMagickHandlerTest.php
@@ -327,17 +327,38 @@ public function testImageCopy()
 	{
 		foreach (['gif', 'jpeg', 'png', 'webp'] as $type)
 		{
+			if ($type === 'webp' && ! in_array('WEBP', \Imagick::queryFormats()))
+			{
+				$this->expectException(ImageException::class);
+				$this->expectExceptionMessage('Your server does not support the GD function required to process this type of image.');
+			}
+
 			$this->handler->withFile($this->origin . 'ci-logo.' . $type);
 			$this->handler->save($this->root . 'ci-logo.' . $type);
 			$this->assertTrue(file_exists($this->root . 'ci-logo.' . $type));
 
-			$this->assertEquals(
+			$this->assertNotEquals(
 				file_get_contents($this->origin . 'ci-logo.' . $type),
 				file_get_contents($this->root . 'ci-logo.' . $type)
 			);
 		}
 	}
 
+	public function testImageCopyWithNoTargetAndMaxQuality()
+	{
+		foreach (['gif', 'jpeg', 'png', 'webp'] as $type)
+		{
+			$this->handler->withFile($this->origin . 'ci-logo.' . $type);
+			$this->handler->save(null, 100);
+			$this->assertTrue(file_exists($this->origin . 'ci-logo.' . $type));
+
+			$this->assertEquals(
+				file_get_contents($this->origin . 'ci-logo.' . $type),
+				file_get_contents($this->origin . 'ci-logo.' . $type)
+			);
+		}
+	}
+
 	public function testImageCompressionGetResource()
 	{
 		foreach (['gif', 'jpeg', 'png', 'webp'] as $type)