Skip to content

Commit f080b2d

Browse files
committed
Fix Image::save() when target value is null
1 parent f5d899c commit f080b2d

4 files changed

Lines changed: 58 additions & 6 deletions

File tree

system/Images/Handlers/GDHandler.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,18 +278,26 @@ protected function process(string $action)
278278
*/
279279
public function save(string $target = null, int $quality = 90): bool
280280
{
281-
$target = empty($target) ? $this->image()->getPathname() : $target;
281+
$original = $target;
282+
$target = empty($target) ? $this->image()->getPathname() : $target;
282283

283284
// If no new resource has been created, then we're
284285
// simply copy the existing one.
285-
if (empty($this->resource))
286+
if (empty($this->resource) && $quality === 100)
286287
{
288+
if ($original === null)
289+
{
290+
return true;
291+
}
292+
287293
$name = basename($target);
288294
$path = pathinfo($target, PATHINFO_DIRNAME);
289295

290296
return $this->image()->copy($path, $name);
291297
}
292298

299+
$this->ensureResource();
300+
293301
switch ($this->image()->imageType)
294302
{
295303
case IMAGETYPE_GIF:

system/Images/Handlers/ImageMagickHandler.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,18 +299,26 @@ protected function process(string $action, int $quality = 100): array
299299
*/
300300
public function save(string $target = null, int $quality = 90): bool
301301
{
302-
$target = empty($target) ? $this->image() : $target;
302+
$original = $target;
303+
$target = empty($target) ? $this->image()->getPathname() : $target;
303304

304305
// If no new resource has been created, then we're
305306
// simply copy the existing one.
306-
if (empty($this->resource))
307+
if (empty($this->resource) && $quality === 100)
307308
{
309+
if ($original === null)
310+
{
311+
return true;
312+
}
313+
308314
$name = basename($target);
309315
$path = pathinfo($target, PATHINFO_DIRNAME);
310316

311317
return $this->image()->copy($path, $name);
312318
}
313319

320+
$this->ensureResource();
321+
314322
// Copy the file through ImageMagick so that it has
315323
// a chance to convert file format.
316324
$action = escapeshellarg($this->resource) . ' ' . escapeshellarg($target);

tests/system/Images/GDHandlerTest.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,13 +335,28 @@ public function testImageCopy()
335335
$this->handler->save($this->start . 'work/ci-logo.' . $type);
336336
$this->assertTrue($this->root->hasChild('work/ci-logo.' . $type));
337337

338-
$this->assertEquals(
338+
$this->assertNotEquals(
339339
file_get_contents($this->origin . 'ci-logo.' . $type),
340340
$this->root->getChild('work/ci-logo.' . $type)->getContent()
341341
);
342342
}
343343
}
344344

345+
public function testImageCopyWithNoTargetAndMaxQuality()
346+
{
347+
foreach (['gif', 'jpeg', 'png', 'webp'] as $type)
348+
{
349+
$this->handler->withFile($this->origin . 'ci-logo.' . $type);
350+
$this->handler->save(null, 100);
351+
$this->assertTrue(file_exists($this->origin . 'ci-logo.' . $type));
352+
353+
$this->assertEquals(
354+
file_get_contents($this->origin . 'ci-logo.' . $type),
355+
file_get_contents($this->origin . 'ci-logo.' . $type)
356+
);
357+
}
358+
}
359+
345360
public function testImageCompressionGetResource()
346361
{
347362
foreach (['gif', 'jpeg', 'png', 'webp'] as $type)

tests/system/Images/ImageMagickHandlerTest.php

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,17 +327,38 @@ public function testImageCopy()
327327
{
328328
foreach (['gif', 'jpeg', 'png', 'webp'] as $type)
329329
{
330+
if ($type === 'webp' && ! in_array('WEBP', \Imagick::queryFormats()))
331+
{
332+
$this->expectException(ImageException::class);
333+
$this->expectExceptionMessage('Your server does not support the GD function required to process this type of image.');
334+
}
335+
330336
$this->handler->withFile($this->origin . 'ci-logo.' . $type);
331337
$this->handler->save($this->root . 'ci-logo.' . $type);
332338
$this->assertTrue(file_exists($this->root . 'ci-logo.' . $type));
333339

334-
$this->assertEquals(
340+
$this->assertNotEquals(
335341
file_get_contents($this->origin . 'ci-logo.' . $type),
336342
file_get_contents($this->root . 'ci-logo.' . $type)
337343
);
338344
}
339345
}
340346

347+
public function testImageCopyWithNoTargetAndMaxQuality()
348+
{
349+
foreach (['gif', 'jpeg', 'png', 'webp'] as $type)
350+
{
351+
$this->handler->withFile($this->origin . 'ci-logo.' . $type);
352+
$this->handler->save(null, 100);
353+
$this->assertTrue(file_exists($this->origin . 'ci-logo.' . $type));
354+
355+
$this->assertEquals(
356+
file_get_contents($this->origin . 'ci-logo.' . $type),
357+
file_get_contents($this->origin . 'ci-logo.' . $type)
358+
);
359+
}
360+
}
361+
341362
public function testImageCompressionGetResource()
342363
{
343364
foreach (['gif', 'jpeg', 'png', 'webp'] as $type)

0 commit comments

Comments
 (0)