From 88e45ed7a4d4ac9f5ee70285a64cc9ce92d97a9d Mon Sep 17 00:00:00 2001 From: Kari Laari Date: Tue, 12 Nov 2024 13:19:25 +0200 Subject: [PATCH 1/4] Fix Image::fromString to support PNG and WebP files - Add missing fromString methods to PNG and WebP classes - Update Image::fromString to read image size and MIME type with getimagesizefromstring - Update and refactor related tests --- src/Format/PNG.php | 10 ++ src/Format/WebP.php | 11 ++- src/Image.php | 36 +++---- tests/Format/JPEGTest.php | 59 ++++++----- tests/Format/PNGTest.php | 202 ++++++++++++++++++++++++++------------ tests/Format/WebPTest.php | 171 +++++++++++++++++++++++++++----- tests/ImageTest.php | 49 +++++++++ 7 files changed, 407 insertions(+), 131 deletions(-) diff --git a/src/Format/PNG.php b/src/Format/PNG.php index 7287a47..2aded8b 100644 --- a/src/Format/PNG.php +++ b/src/Format/PNG.php @@ -104,6 +104,16 @@ public static function fromFile($filename) return new self(file_get_contents($filename)); } + /** + * @param $string + * + * @return PNG + */ + public static function fromString($string) + { + return new self($string); + } + /** * @param string $contents * diff --git a/src/Format/WebP.php b/src/Format/WebP.php index f12c791..a91c130 100644 --- a/src/Format/WebP.php +++ b/src/Format/WebP.php @@ -144,10 +144,19 @@ public function getIptc() */ public static function fromFile($filename) { - // var_dump($filename); return new self(file_get_contents($filename)); } + /** + * @param $string + * + * @return PNG + */ + public static function fromString($string) + { + return new self($string); + } + /** * @param string $contents * diff --git a/src/Image.php b/src/Image.php index b956594..14f9da7 100644 --- a/src/Image.php +++ b/src/Image.php @@ -210,29 +210,31 @@ public static function fromFile($fileName) * * @return JPEG|WebP|PNG|false */ + public static function fromString($string) { - $len = strlen($string); + $imageInfo = getimagesizefromstring($string); - // try JPEG - if ($len >= 2) { - if (JPEG::SOI === substr($string, 0, 2)) { - return JPEG::fromString($string); - } + if (!$imageInfo) { + return false; } - // try WebP - if ($len >= 4) { - if ('RIFF' === substr($string, 0, 4) && 'WEBP' === substr($string, 8, 4)) { - return WebP::fromString($string); - } - } + $width = $imageInfo[0]; + $height = $imageInfo[1]; + $mime = $imageInfo['mime']; - // try PNG - if ($len >= 8) { - if (PNG::SIGNATURE === substr($string, 0, 8)) { - return PNG::fromString($string); - } + $mimeToClass = [ + 'image/jpeg' => JPEG::class, + 'image/png' => PNG::class, + 'image/webp' => WebP::class, + ]; + + if (isset($mimeToClass[$mime])) { + $class = $mimeToClass[$mime]; + $image = $class::fromString($string); + $image->width = $width; + $image->height = $height; + return $image; } return false; diff --git a/tests/Format/JPEGTest.php b/tests/Format/JPEGTest.php index 1360871..594423a 100644 --- a/tests/Format/JPEGTest.php +++ b/tests/Format/JPEGTest.php @@ -1,53 +1,58 @@ - * * @coversDefaultClass \CSD\Image\Format\JPEG */ class JPEGTest extends \PHPUnit\Framework\TestCase { /** - * Test that JPEG can read XMP embedded with Photo Mechanic. + * Data provider for testGetXmp method. + * + * @return array */ - public function testGetXmpPhotoMechanic() + public function providerTestGetXmp() { - $jpeg = JPEG::fromFile(__DIR__ . '/../Fixtures/metapm.jpg'); - - $xmp = $jpeg->getXmp(); - - $this->assertInstanceOf(Xmp::class, $xmp); - $this->assertSame('Headline', $xmp->getHeadline()); + return [ + // [method, filename, expectedHeadline] + ['fromFile', 'metapm.jpg', 'Headline'], + ['fromString', 'metapm.jpg', 'Headline'], + ['fromFile', 'metaphotoshop.jpg', 'Headline'], + ['fromString', 'metaphotoshop.jpg', 'Headline'], + ['fromFile', 'nometa.jpg', null], + ['fromString', 'nometa.jpg', null], + ]; } /** - * Test that JPEG can read XMP embedded with Photoshop. + * Test that JPEG can read XMP data using both fromFile and fromString methods. + * + * @dataProvider providerTestGetXmp + * + * @param string $method The method to use ('fromFile' or 'fromString') + * @param string $filename The filename of the test image + * @param string|null $expectedHeadline The expected headline in the XMP data */ - public function testGetXmpPhotoshop() + public function testGetXmp($method, $filename, $expectedHeadline) { - $jpeg = JPEG::fromFile(__DIR__ . '/../Fixtures/metaphotoshop.jpg'); - - $xmp = $jpeg->getXmp(); + $filePath = __DIR__ . '/../Fixtures/' . $filename; - $this->assertInstanceOf(Xmp::class, $xmp); - $this->assertSame('Headline', $xmp->getHeadline()); - } - - /** - * Test that JPEG class returns an empty XMP object when there is no XMP data. - */ - public function testGetXmpNoMeta() - { - $jpeg = JPEG::fromFile(__DIR__ . '/../Fixtures/nometa.jpg'); + if ($method === 'fromFile') { + $jpeg = JPEG::fromFile($filePath); + } elseif ($method === 'fromString') { + $string = file_get_contents($filePath); + $jpeg = JPEG::fromString($string); + } else { + throw new \InvalidArgumentException("Invalid method: $method"); + } $xmp = $jpeg->getXmp(); $this->assertInstanceOf(Xmp::class, $xmp); - $this->assertNull($xmp->getHeadline()); + $this->assertSame($expectedHeadline, $xmp->getHeadline()); } } + diff --git a/tests/Format/PNGTest.php b/tests/Format/PNGTest.php index 46a1c37..421461f 100644 --- a/tests/Format/PNGTest.php +++ b/tests/Format/PNGTest.php @@ -1,118 +1,196 @@ - * * @coversDefaultClass \CSD\Image\Format\PNG */ class PNGTest extends \PHPUnit\Framework\TestCase { /** - * Test that a non-PNG file throws an exception. + * Data provider for testGetXmp method. * - * @covers ::fromFile + * @return array */ - public function testFromFileInvalidPNG() + public function providerTestGetXmp() { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('Invalid PNG file signature'); - PNG::fromFile(__DIR__ . '/../Fixtures/nometa.jpg'); + return [ + // [method, filename, expectedPhotographerName, expectedHeadline] + ['fromFile', 'metaphotoshop.png', 'Author', null], + ['fromString', 'metaphotoshop.png', 'Author', null], + ['fromFile', 'metapm.png', null, 'Headline'], + ['fromString', 'metapm.png', null, 'Headline'], + ['fromFile', 'nometa.png', null, null], + ['fromString', 'nometa.png', null, null], + ]; } /** - * @covers ::getXmp - * @covers ::getXmpChunk + * Test that PNG can read XMP data using both fromFile and fromString methods. + * + * @dataProvider providerTestGetXmp + * + * @param string $method The method to use ('fromFile' or 'fromString') + * @param string $filename The filename of the test image + * @param string|null $expectedPhotographerName The expected photographer name in the XMP data + * @param string|null $expectedHeadline The expected headline in the XMP data */ - public function testGetXmpWithMetadataWrittenInPhotoshop() + public function testGetXmp($method, $filename, $expectedPhotographerName, $expectedHeadline) { - $png = PNG::fromFile(__DIR__ . '/../Fixtures/metaphotoshop.png'); + $filePath = __DIR__ . '/../Fixtures/' . $filename; + + if ($method === 'fromFile') { + $png = PNG::fromFile($filePath); + } elseif ($method === 'fromString') { + $string = file_get_contents($filePath); + $png = PNG::fromString($string); + } else { + throw new \InvalidArgumentException("Invalid method: $method"); + } $xmp = $png->getXmp(); - $this->assertInstanceOf(XMP::class, $xmp); - $this->assertEquals('Author', $xmp->getPhotographerName()); + $this->assertInstanceOf(Xmp::class, $xmp); + $this->assertSame($expectedPhotographerName, $xmp->getPhotographerName()); + $this->assertSame($expectedHeadline, $xmp->getHeadline()); } /** - * @covers ::getXmp - * @covers ::getXmpChunk + * Data provider for testInvalidPNG method. + * + * @return array */ - public function testGetXmpWithMetaWrittenInPhotoMechanic() + public function providerTestInvalidPNG() { - $png = PNG::fromFile(__DIR__ . '/../Fixtures/metapm.png'); - - $xmp = $png->getXmp(); - - $this->assertInstanceOf(XMP::class, $xmp); - $this->assertEquals('Headline', $xmp->getHeadline()); + return [ + // [method, filename, expectedExceptionMessage] + ['fromFile', 'nometa.jpg', 'Invalid PNG file signature'], + ['fromString', 'nometa.jpg', 'Invalid PNG file signature'], + ]; } /** - * @covers ::getXmp - * @covers ::getXmpChunk + * Test that a non-PNG file throws an exception. + * + * @dataProvider providerTestInvalidPNG + * + * @param string $method The method to use ('fromFile' or 'fromString') + * @param string $filename The filename of the test image + * @param string $expectedExceptionMessage The expected exception message */ - public function testGetXmpNoMeta() + public function testInvalidPNG($method, $filename, $expectedExceptionMessage) { - $png = PNG::fromFile(__DIR__ . '/../Fixtures/nometa.png'); - - $xmp = $png->getXmp(); - - $this->assertInstanceOf(XMP::class, $xmp); + $filePath = __DIR__ . '/../Fixtures/' . $filename; - // check it's an empty XMP string - $this->assertEquals(' - - - -', $xmp->getString()); + $this->expectException(\Exception::class); + $this->expectExceptionMessage($expectedExceptionMessage); + + if ($method === 'fromFile') { + $png = PNG::fromFile($filePath); + } elseif ($method === 'fromString') { + $string = file_get_contents($filePath); + $png = PNG::fromString($string); + } else { + throw new \InvalidArgumentException("Invalid method: $method"); + } } /** - * @covers ::fromFile - * @covers ::getChunksFromContents - * @covers ::__construct + * Data provider for testUnsupportedMetadata method. + * + * @return array */ - public function testFromFileValidPNG() + public function providerTestUnsupportedMetadata() { - $png = PNG::fromFile(__DIR__ . '/../Fixtures/nometa.png'); - - $this->assertInstanceOf(PNG::class, $png); + return [ + // [method, filename, metadataMethod, expectedExceptionMessage] + ['fromFile', 'nometa.png', 'getExif', 'PNG files do not support EXIF metadata'], + ['fromString', 'nometa.png', 'getExif', 'PNG files do not support EXIF metadata'], + ['fromFile', 'nometa.png', 'getIptc', 'PNG files do not support IPTC metadata'], + ['fromString', 'nometa.png', 'getIptc', 'PNG files do not support IPTC metadata'], + ]; } /** - * @covers ::getChunksFromContents + * Test that calling unsupported metadata methods throws an exception. + * + * @dataProvider providerTestUnsupportedMetadata + * + * @param string $method The method to use ('fromFile' or 'fromString') + * @param string $filename The filename of the test image + * @param string $metadataMethod The metadata method to call ('getExif' or 'getIptc') + * @param string $expectedExceptionMessage The expected exception message */ - public function testFromFileWithMalformedChunks() + public function testUnsupportedMetadata($method, $filename, $metadataMethod, $expectedExceptionMessage) { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('Invalid CRC for chunk with type: IHDR'); - PNG::fromFile(__DIR__ . '/../Fixtures/malformedchunks.png'); + $filePath = __DIR__ . '/../Fixtures/' . $filename; + + $this->expectException(UnsupportedException::class); + $this->expectExceptionMessage($expectedExceptionMessage); + + if ($method === 'fromFile') { + $png = PNG::fromFile($filePath); + } elseif ($method === 'fromString') { + $string = file_get_contents($filePath); + $png = PNG::fromString($string); + } else { + throw new \InvalidArgumentException("Invalid method: $method"); + } + + $png->$metadataMethod(); } /** - * @covers ::getExif + * Test that a valid PNG file can be loaded using both fromFile and fromString. + * + * @return void */ - public function testGetExif() + public function testValidPNG() { - $this->expectException(\CSD\Image\Metadata\UnsupportedException::class); - $this->expectExceptionMessage('PNG files do not support EXIF metadata'); - $png = PNG::fromFile(__DIR__ . '/../Fixtures/nometa.png'); - $png->getExif(); + $methods = ['fromFile', 'fromString']; + $filename = 'nometa.png'; + + foreach ($methods as $method) { + $filePath = __DIR__ . '/../Fixtures/' . $filename; + + if ($method === 'fromFile') { + $png = PNG::fromFile($filePath); + } elseif ($method === 'fromString') { + $string = file_get_contents($filePath); + $png = PNG::fromString($string); + } + + $this->assertInstanceOf(PNG::class, $png); + } } /** - * @covers ::getIptc + * Test that a PNG file with malformed chunks throws an exception. + * + * @return void */ - public function testGetIptc() + public function testMalformedChunks() { - $this->expectException(\CSD\Image\Metadata\UnsupportedException::class); - $this->expectExceptionMessage('PNG files do not support IPTC metadata'); - $png = PNG::fromFile(__DIR__ . '/../Fixtures/nometa.png'); - $png->getIptc(); + $methods = ['fromFile', 'fromString']; + $filename = 'malformedchunks.png'; + $expectedExceptionMessage = 'Invalid CRC for chunk with type: IHDR'; + + foreach ($methods as $method) { + $this->expectException(\Exception::class); + $this->expectExceptionMessage($expectedExceptionMessage); + + $filePath = __DIR__ . '/../Fixtures/' . $filename; + + if ($method === 'fromFile') { + $png = PNG::fromFile($filePath); + } elseif ($method === 'fromString') { + $string = file_get_contents($filePath); + $png = PNG::fromString($string); + } + } } } + diff --git a/tests/Format/WebPTest.php b/tests/Format/WebPTest.php index d0e6dd0..3352db6 100644 --- a/tests/Format/WebPTest.php +++ b/tests/Format/WebPTest.php @@ -5,63 +5,186 @@ use CSD\Image\Format\WebP; use CSD\Image\Metadata\Exif; use CSD\Image\Metadata\Xmp; +use CSD\Image\Metadata\UnsupportedException; /** - * @author Daniel Chesterton - * * @coversDefaultClass \CSD\Image\Format\WebP */ class WebPTest extends \PHPUnit\Framework\TestCase { /** - * Test that a non-WebP file throws an exception. + * Data provider for valid WebP tests. * - * @covers ::fromFile - * @covers ::__construct + * @return array */ - public function testFromFileInvalidWebP() + public function providerTestValidWebP() { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('Invalid WebP file'); - WebP::fromFile(__DIR__ . '/../Fixtures/nometa.jpg'); + return [ + // [method, filename, expectedHeadline] + ['fromFile', 'meta.webp', 'Headline'], + ['fromString', 'meta.webp', 'Headline'], + ]; } - public function testFromFile() + /** + * Test that WebP can read XMP data using both fromFile and fromString methods. + * + * @dataProvider providerTestValidWebP + * + * @param string $method The method to use ('fromFile' or 'fromString') + * @param string $filename The filename of the test image + * @param string $expectedHeadline The expected headline in the XMP data + */ + public function testValidWebP($method, $filename, $expectedHeadline) { - $webp = WebP::fromFile(__DIR__ . '/../Fixtures/meta.webp'); + $filePath = __DIR__ . '/../Fixtures/' . $filename; + + if ($method === 'fromFile') { + $webp = WebP::fromFile($filePath); + } elseif ($method === 'fromString') { + $string = file_get_contents($filePath); + $webp = WebP::fromString($string); + } else { + throw new \InvalidArgumentException("Invalid method: $method"); + } + $this->assertInstanceOf(WebP::class, $webp); $xmp = $webp->getXmp(); - $this->assertInstanceOf(XMP::class, $xmp); - $this->assertSame('Headline', $xmp->getHeadline()); + $this->assertInstanceOf(Xmp::class, $xmp); + $this->assertSame($expectedHeadline, $xmp->getHeadline()); } - public function testGetExif() + /** + * Data provider for invalid WebP tests. + * + * @return array + */ + public function providerTestInvalidWebP() { - $webp = WebP::fromFile(__DIR__ . '/../Fixtures/exif.webp'); + return [ + // [method, filename, expectedExceptionMessage] + ['fromFile', 'nometa.jpg', 'Invalid WebP file'], + ['fromString', 'nometa.jpg', 'Invalid WebP file'], + ]; + } + + /** + * Test that a non-WebP file throws an exception. + * + * @dataProvider providerTestInvalidWebP + * + * @param string $method The method to use ('fromFile' or 'fromString') + * @param string $filename The filename of the test image + * @param string $expectedExceptionMessage The expected exception message + */ + public function testInvalidWebP($method, $filename, $expectedExceptionMessage) + { + $filePath = __DIR__ . '/../Fixtures/' . $filename; + + $this->expectException(\Exception::class); + $this->expectExceptionMessage($expectedExceptionMessage); + + if ($method === 'fromFile') { + WebP::fromFile($filePath); + } elseif ($method === 'fromString') { + $string = file_get_contents($filePath); + WebP::fromString($string); + } else { + throw new \InvalidArgumentException("Invalid method: $method"); + } + } + + /** + * Data provider for Exif tests. + * + * @return array + */ + public function providerTestGetExif() + { + return [ + // [method, filename] + ['fromFile', 'exif.webp'], + ['fromString', 'exif.webp'], + ]; + } + + /** + * Test that Exif data can be retrieved from WebP files. + * + * @dataProvider providerTestGetExif + * + * @param string $method The method to use ('fromFile' or 'fromString') + * @param string $filename The filename of the test image + */ + public function testGetExif($method, $filename) + { + $filePath = __DIR__ . '/../Fixtures/' . $filename; + + if ($method === 'fromFile') { + $webp = WebP::fromFile($filePath); + } elseif ($method === 'fromString') { + $string = file_get_contents($filePath); + $webp = WebP::fromString($string); + } else { + throw new \InvalidArgumentException("Invalid method: $method"); + } + $exif = $webp->getExif(); $this->assertInstanceOf(Exif::class, $exif); - // todo: test actual value of exif + // TODO: Add assertions to test actual Exif data } /** - * @covers ::getIptc + * Data provider for unsupported metadata tests. + * + * @return array + */ + public function providerTestUnsupportedMetadata() + { + return [ + // [method, filename, metadataMethod, expectedExceptionMessage] + ['fromFile', 'meta.webp', 'getIptc', 'WebP files do not support IPTC metadata'], + ['fromString', 'meta.webp', 'getIptc', 'WebP files do not support IPTC metadata'], + ]; + } + + /** + * Test that calling unsupported metadata methods throws an exception. + * + * @dataProvider providerTestUnsupportedMetadata + * + * @param string $method The method to use ('fromFile' or 'fromString') + * @param string $filename The filename of the test image + * @param string $metadataMethod The metadata method to call ('getIptc') + * @param string $expectedExceptionMessage The expected exception message */ - public function testGetIptc() + public function testUnsupportedMetadata($method, $filename, $metadataMethod, $expectedExceptionMessage) { - $this->expectException(\CSD\Image\Metadata\UnsupportedException::class); - $this->expectExceptionMessage('WebP files do not support IPTC metadata'); - $webp = WebP::fromFile(__DIR__ . '/../Fixtures/meta.webp'); - $webp->getIptc(); + $filePath = __DIR__ . '/../Fixtures/' . $filename; + + $this->expectException(UnsupportedException::class); + $this->expectExceptionMessage($expectedExceptionMessage); + + if ($method === 'fromFile') { + $webp = WebP::fromFile($filePath); + } elseif ($method === 'fromString') { + $string = file_get_contents($filePath); + $webp = WebP::fromString($string); + } else { + throw new \InvalidArgumentException("Invalid method: $method"); + } + + $webp->$metadataMethod(); } public function ttestSimpleUnsupported() # FIXME - this test fails { $this->expectException(\Exception::class); $this->expectExceptionMessage('Only extended WebP format is supported'); - WebP::fromFile(__DIR__ . '/../Fixtures/simple.webp'); - } + $image = WebP::fromFile(__DIR__ . '/../Fixtures/simple.webp'); + } } diff --git a/tests/ImageTest.php b/tests/ImageTest.php index 17fe9d8..ac9c0cc 100644 --- a/tests/ImageTest.php +++ b/tests/ImageTest.php @@ -29,6 +29,22 @@ public function testPNG() ], $image->getSize()); } + /** + * @covers ::fromString + * @covers ::getSize + */ + public function testPNGFromString() + { + $string = \file_get_contents(__DIR__ . '/Fixtures/nometa.png'); + $image = Image::fromString($string); + + $this->assertInstanceOf(PNG::class, $image); + $this->assertSame([ + 'width' => 10, + 'height' => 10, + ], $image->getSize()); + } + /** * @covers ::fromFile * @covers ::getSize @@ -43,6 +59,22 @@ public function testJPG() ], $image->getSize()); } + /** + * @covers ::fromString + * @covers ::getSize + */ + public function testJPGFromString() + { + $string = \file_get_contents(__DIR__ . '/Fixtures/nometa.jpg'); + + $image = Image::fromString($string); + $this->assertInstanceOf(JPEG::class, $image); + $this->assertSame([ + 'width' => 10, + 'height' => 10, + ], $image->getSize()); + } + /** * @covers ::fromFile * @covers ::getSize @@ -57,6 +89,23 @@ public function testWebP() ], $image->getSize()); } + /** + * @covers ::fromString + * @covers ::getSize + */ + public function testWebpFromString() + { + $string = \file_get_contents(__DIR__ . '/Fixtures/simple.webp'); + $image = Image::fromString($string); + + $this->assertInstanceOf(WebP::class, $image); + $this->assertSame([ + 'width' => 550, + 'height' => 368, + ], $image->getSize()); + } + + /** * @covers ::fromFile * @covers ::getSize From 7b805320561659dc098b006a7f46999726c2f74b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Lourot?= Date: Thu, 14 Nov 2024 12:45:45 +0100 Subject: [PATCH 2/4] qual: add test assertions on image sizes Expected to fail because in the current implementation, Image::fromString() calculates the image size but methods like PNG::fromString() and JPEG::fromFile() don't. --- tests/Format/JPEGTest.php | 3 +++ tests/Format/PNGTest.php | 3 +++ tests/Format/WebPTest.php | 1 + 3 files changed, 7 insertions(+) diff --git a/tests/Format/JPEGTest.php b/tests/Format/JPEGTest.php index 594423a..f3574ae 100644 --- a/tests/Format/JPEGTest.php +++ b/tests/Format/JPEGTest.php @@ -49,6 +49,9 @@ public function testGetXmp($method, $filename, $expectedHeadline) throw new \InvalidArgumentException("Invalid method: $method"); } + $this->assertInstanceOf(JPEG::class, $jpeg); + $this->assertGreaterThan(0, $jpeg->getSize()["width"]); + $xmp = $jpeg->getXmp(); $this->assertInstanceOf(Xmp::class, $xmp); diff --git a/tests/Format/PNGTest.php b/tests/Format/PNGTest.php index 421461f..e5c74ca 100644 --- a/tests/Format/PNGTest.php +++ b/tests/Format/PNGTest.php @@ -51,6 +51,9 @@ public function testGetXmp($method, $filename, $expectedPhotographerName, $expec throw new \InvalidArgumentException("Invalid method: $method"); } + $this->assertInstanceOf(PNG::class, $png); + $this->assertGreaterThan(0, $png->getSize()["width"]); + $xmp = $png->getXmp(); $this->assertInstanceOf(Xmp::class, $xmp); diff --git a/tests/Format/WebPTest.php b/tests/Format/WebPTest.php index 3352db6..76cf9b5 100644 --- a/tests/Format/WebPTest.php +++ b/tests/Format/WebPTest.php @@ -49,6 +49,7 @@ public function testValidWebP($method, $filename, $expectedHeadline) } $this->assertInstanceOf(WebP::class, $webp); + $this->assertGreaterThan(0, $webp->getSize()["width"]); $xmp = $webp->getXmp(); From 43ee1c4b9b9d17c649be7dc562e4df54296709aa Mon Sep 17 00:00:00 2001 From: Kari Laari Date: Fri, 15 Nov 2024 11:38:05 +0200 Subject: [PATCH 3/4] Calculate image dimensions in JPEG, PNG, and WebP classes --- src/Format/JPEG.php | 31 ++++++++++++++++++++++++++++--- src/Format/PNG.php | 30 ++++++++++++++++++++++++++++-- src/Format/WebP.php | 31 +++++++++++++++++++++++++++++-- tests/Format/JPEGTest.php | 1 + tests/Format/PNGTest.php | 1 + tests/Format/WebPTest.php | 1 + 6 files changed, 88 insertions(+), 7 deletions(-) diff --git a/src/Format/JPEG.php b/src/Format/JPEG.php index 072a0d5..25eb8f2 100644 --- a/src/Format/JPEG.php +++ b/src/Format/JPEG.php @@ -81,11 +81,24 @@ public static function fromResource($gd) */ public static function fromString($string) { + $imageSize = getimagesizefromstring($string); + if ($imageSize === false) { + throw new \Exception('Invalid image data'); + } + + $width = $imageSize[0]; + $height = $imageSize[1]; + $stream = fopen('php://temp', 'r+'); fwrite($stream, $string); rewind($stream); - return self::fromStream($stream); + $jpeg = self::fromStream($stream); + + $jpeg->width = $width; + $jpeg->height = $height; + + return $jpeg; } /** @@ -189,12 +202,24 @@ public static function fromStream($fileHandle) public static function fromFile($filename) { $fileHandle = @fopen($filename, 'rb'); - if (!$fileHandle) { throw new \Exception(sprintf('Could not open file %s', $filename)); } - return self::fromStream($fileHandle); + $imageSize = getimagesize($filename); + if ($imageSize === false) { + throw new \Exception(sprintf('Could not get image size for %s', $filename)); + } + + $width = $imageSize[0]; + $height = $imageSize[1]; + + $jpeg = self::fromStream($fileHandle); + + $jpeg->width = $width; + $jpeg->height = $height; + + return $jpeg; } /** diff --git a/src/Format/PNG.php b/src/Format/PNG.php index 2aded8b..75c8dad 100644 --- a/src/Format/PNG.php +++ b/src/Format/PNG.php @@ -101,7 +101,20 @@ public function getIptc() */ public static function fromFile($filename) { - return new self(file_get_contents($filename)); + $imageSize = getimagesize($filename); + if ($imageSize === false) { + throw new \Exception(sprintf('Could not get image size for %s', $filename)); + } + + $width = $imageSize[0]; + $height = $imageSize[1]; + + $png = new self(file_get_contents($filename)); + + $png->width = $width; + $png->height = $height; + + return $png; } /** @@ -111,7 +124,20 @@ public static function fromFile($filename) */ public static function fromString($string) { - return new self($string); + $imageSize = getimagesizefromstring($string); + if ($imageSize === false) { + throw new \Exception('Invalid PNG data'); + } + + $width = $imageSize[0]; + $height = $imageSize[1]; + + $png = new self($string); + + $png->width = $width; + $png->height = $height; + + return $png; } /** diff --git a/src/Format/WebP.php b/src/Format/WebP.php index a91c130..0c7ad73 100644 --- a/src/Format/WebP.php +++ b/src/Format/WebP.php @@ -144,9 +144,23 @@ public function getIptc() */ public static function fromFile($filename) { - return new self(file_get_contents($filename)); + $imageSize = getimagesize($filename); + if ($imageSize === false) { + throw new \Exception(sprintf('Could not get image size for %s', $filename)); + } + + $width = $imageSize[0]; + $height = $imageSize[1]; + + $webp = new self(file_get_contents($filename)); + + $webp->width = $width; + $webp->height = $height; + + return $webp; } + /** * @param $string * @@ -154,7 +168,20 @@ public static function fromFile($filename) */ public static function fromString($string) { - return new self($string); + $imageSize = getimagesizefromstring($string); + if ($imageSize === false) { + throw new \Exception('Invalid WebP data'); + } + + $width = $imageSize[0]; + $height = $imageSize[1]; + + $webp = new self($string); + + $webp->width = $width; + $webp->height = $height; + + return $webp; } /** diff --git a/tests/Format/JPEGTest.php b/tests/Format/JPEGTest.php index f3574ae..d33b4d8 100644 --- a/tests/Format/JPEGTest.php +++ b/tests/Format/JPEGTest.php @@ -51,6 +51,7 @@ public function testGetXmp($method, $filename, $expectedHeadline) $this->assertInstanceOf(JPEG::class, $jpeg); $this->assertGreaterThan(0, $jpeg->getSize()["width"]); + $this->assertGreaterThan(0, $jpeg->getSize()["height"]); $xmp = $jpeg->getXmp(); diff --git a/tests/Format/PNGTest.php b/tests/Format/PNGTest.php index e5c74ca..9b51665 100644 --- a/tests/Format/PNGTest.php +++ b/tests/Format/PNGTest.php @@ -53,6 +53,7 @@ public function testGetXmp($method, $filename, $expectedPhotographerName, $expec $this->assertInstanceOf(PNG::class, $png); $this->assertGreaterThan(0, $png->getSize()["width"]); + $this->assertGreaterThan(0, $png->getSize()["height"]); $xmp = $png->getXmp(); diff --git a/tests/Format/WebPTest.php b/tests/Format/WebPTest.php index 76cf9b5..54b481a 100644 --- a/tests/Format/WebPTest.php +++ b/tests/Format/WebPTest.php @@ -50,6 +50,7 @@ public function testValidWebP($method, $filename, $expectedHeadline) $this->assertInstanceOf(WebP::class, $webp); $this->assertGreaterThan(0, $webp->getSize()["width"]); + $this->assertGreaterThan(0, $webp->getSize()["height"]); $xmp = $webp->getXmp(); From 99d28e908b7cc75081555651a33ccb3d718df582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Lourot?= Date: Fri, 15 Nov 2024 11:31:13 +0100 Subject: [PATCH 4/4] qual: de-duplicate code for image size calculation --- src/Format/JPEG.php | 26 ++++---------------------- src/Format/PNG.php | 33 +++++++-------------------------- src/Format/PSD.php | 9 ++++++++- src/Format/WebP.php | 33 +++++++-------------------------- src/Image.php | 25 +++++++++++++++++-------- 5 files changed, 43 insertions(+), 83 deletions(-) diff --git a/src/Format/JPEG.php b/src/Format/JPEG.php index 25eb8f2..5184f79 100644 --- a/src/Format/JPEG.php +++ b/src/Format/JPEG.php @@ -81,22 +81,12 @@ public static function fromResource($gd) */ public static function fromString($string) { - $imageSize = getimagesizefromstring($string); - if ($imageSize === false) { - throw new \Exception('Invalid image data'); - } - - $width = $imageSize[0]; - $height = $imageSize[1]; - $stream = fopen('php://temp', 'r+'); fwrite($stream, $string); rewind($stream); $jpeg = self::fromStream($stream); - - $jpeg->width = $width; - $jpeg->height = $height; + $jpeg->setSizeFromString($string); return $jpeg; } @@ -121,6 +111,8 @@ public static function fromImagick(\Imagick $imagick) * * @return self * @throws \Exception + * + * @todo calculate and set image size */ public static function fromStream($fileHandle) { @@ -206,18 +198,8 @@ public static function fromFile($filename) throw new \Exception(sprintf('Could not open file %s', $filename)); } - $imageSize = getimagesize($filename); - if ($imageSize === false) { - throw new \Exception(sprintf('Could not get image size for %s', $filename)); - } - - $width = $imageSize[0]; - $height = $imageSize[1]; - $jpeg = self::fromStream($fileHandle); - - $jpeg->width = $width; - $jpeg->height = $height; + $jpeg->setSizeFromFile($filename); return $jpeg; } diff --git a/src/Format/PNG.php b/src/Format/PNG.php index 75c8dad..f23a548 100644 --- a/src/Format/PNG.php +++ b/src/Format/PNG.php @@ -40,6 +40,8 @@ public function __construct($contents) } $this->chunks = $this->getChunksFromContents($contents); + + $this->setSizeFromString($contents); } /** @@ -101,20 +103,12 @@ public function getIptc() */ public static function fromFile($filename) { - $imageSize = getimagesize($filename); - if ($imageSize === false) { - throw new \Exception(sprintf('Could not get image size for %s', $filename)); + $contents = file_get_contents($filename); + if ($contents === false) { + throw new \Exception(sprintf('Could not open file %s', $filename)); } - $width = $imageSize[0]; - $height = $imageSize[1]; - - $png = new self(file_get_contents($filename)); - - $png->width = $width; - $png->height = $height; - - return $png; + return new self($contents); } /** @@ -124,20 +118,7 @@ public static function fromFile($filename) */ public static function fromString($string) { - $imageSize = getimagesizefromstring($string); - if ($imageSize === false) { - throw new \Exception('Invalid PNG data'); - } - - $width = $imageSize[0]; - $height = $imageSize[1]; - - $png = new self($string); - - $png->width = $width; - $png->height = $height; - - return $png; + return new self($string); } /** diff --git a/src/Format/PSD.php b/src/Format/PSD.php index 4f1d0cb..707d005 100644 --- a/src/Format/PSD.php +++ b/src/Format/PSD.php @@ -4,6 +4,7 @@ use CSD\Image\Metadata\Exif; use CSD\Image\Metadata\Iptc; +use CSD\Image\Metadata\UnsupportedException; use CSD\Image\Metadata\Xmp; use CSD\Image\Image; @@ -70,6 +71,8 @@ public static function fromResource($gd) /** * Load PSD from string. + * + * @todo calculate and set image size */ public static function fromString($string) { @@ -96,6 +99,8 @@ public static function fromImagick(\Imagick $imagick) * * @return self * @throws \Exception + * + * @todo calculate and set image size */ public static function fromStream($fileHandle) { @@ -194,7 +199,9 @@ public static function fromFile($filename) throw new \Exception(sprintf('Could not open file %s', $filename)); } - return self::fromStream($fileHandle); + $psd = self::fromStream($fileHandle); + $psd->setSizeFromFile($filename); + return $psd; } /** diff --git a/src/Format/WebP.php b/src/Format/WebP.php index 0c7ad73..33fabd9 100644 --- a/src/Format/WebP.php +++ b/src/Format/WebP.php @@ -47,6 +47,8 @@ public function __construct($contents) if (!$this->isExtendedFormat()) { // throw new \Exception('Only extended WebP format is supported'); } + + $this->setSizeFromString($contents); } /** @@ -144,20 +146,12 @@ public function getIptc() */ public static function fromFile($filename) { - $imageSize = getimagesize($filename); - if ($imageSize === false) { - throw new \Exception(sprintf('Could not get image size for %s', $filename)); + $contents = file_get_contents($filename); + if ($contents === false) { + throw new \Exception(sprintf('Could not open file %s', $filename)); } - $width = $imageSize[0]; - $height = $imageSize[1]; - - $webp = new self(file_get_contents($filename)); - - $webp->width = $width; - $webp->height = $height; - - return $webp; + return new self($contents); } @@ -168,20 +162,7 @@ public static function fromFile($filename) */ public static function fromString($string) { - $imageSize = getimagesizefromstring($string); - if ($imageSize === false) { - throw new \Exception('Invalid WebP data'); - } - - $width = $imageSize[0]; - $height = $imageSize[1]; - - $webp = new self($string); - - $webp->width = $width; - $webp->height = $height; - - return $webp; + return new self($string); } /** diff --git a/src/Image.php b/src/Image.php index 14f9da7..27cf824 100644 --- a/src/Image.php +++ b/src/Image.php @@ -198,10 +198,6 @@ public static function fromFile($fileName) if (!$result) { throw new \Exception('Unrecognised file name'); } - - $size = getimagesize($fileName); - $result->width = $size[0]; - $result->height = $size[1]; return $result; } @@ -219,8 +215,6 @@ public static function fromString($string) return false; } - $width = $imageInfo[0]; - $height = $imageInfo[1]; $mime = $imageInfo['mime']; $mimeToClass = [ @@ -232,11 +226,26 @@ public static function fromString($string) if (isset($mimeToClass[$mime])) { $class = $mimeToClass[$mime]; $image = $class::fromString($string); - $image->width = $width; - $image->height = $height; return $image; } return false; } + + protected function setSizeFromFile($fileName) + { + $imageSize = getimagesize($fileName); + if ($imageSize === false) { + throw new \Exception(sprintf('Could not get image size for %s', $fileName)); + } + $this->width = $imageSize[0]; + $this->height = $imageSize[1]; + } + + protected function setSizeFromString($string) + { + $size = getimagesizefromstring($string); + $this->width = $size[0]; + $this->height = $size[1]; + } }