From 23aa60bb8ddf038a531f7aec1d7f48bebf8965dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 25 Oct 2021 16:13:50 +0200 Subject: [PATCH 1/4] Fix resource usages in OC_Image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes sure using resource or GdImage (PHP>=8) behaves the same. Signed-off-by: Côme Chilliet --- lib/private/Avatar/UserAvatar.php | 7 +++++-- lib/private/Preview/Bitmap.php | 2 +- lib/private/Preview/HEIC.php | 2 +- lib/private/Preview/SVG.php | 2 +- lib/private/legacy/OC_Image.php | 34 +++++++++++++++---------------- lib/public/IImage.php | 2 +- psalm.xml | 4 ++++ 7 files changed, 30 insertions(+), 23 deletions(-) diff --git a/lib/private/Avatar/UserAvatar.php b/lib/private/Avatar/UserAvatar.php index f47809425ed40..8573adf5d87c0 100644 --- a/lib/private/Avatar/UserAvatar.php +++ b/lib/private/Avatar/UserAvatar.php @@ -122,7 +122,7 @@ public function set($data) { /** * Returns an image from several sources. * - * @param IImage|resource|string $data An image object, imagedata or path to the avatar + * @param IImage|resource|string|\GdImage $data An image object, imagedata or path to the avatar * @return IImage */ private function getAvatarImage($data) { @@ -131,7 +131,10 @@ private function getAvatarImage($data) { } $img = new OC_Image(); - if (is_resource($data) && get_resource_type($data) === 'gd') { + if ( + (is_resource($data) && get_resource_type($data) === 'gd') || + (is_object($data) && get_class($data) === \GdImage::class) + ) { $img->setResource($data); } elseif (is_resource($data)) { $img->loadFromFileHandle($data); diff --git a/lib/private/Preview/Bitmap.php b/lib/private/Preview/Bitmap.php index 7322e07ab3454..b573808447646 100644 --- a/lib/private/Preview/Bitmap.php +++ b/lib/private/Preview/Bitmap.php @@ -61,7 +61,7 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage { //new bitmap image object $image = new \OC_Image(); - $image->loadFromData($bp); + $image->loadFromData((string) $bp); //check if image object is valid return $image->valid() ? $image : null; } diff --git a/lib/private/Preview/HEIC.php b/lib/private/Preview/HEIC.php index f9f85090a8008..43d1cf65ada6e 100644 --- a/lib/private/Preview/HEIC.php +++ b/lib/private/Preview/HEIC.php @@ -77,7 +77,7 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage { //new bitmap image object $image = new \OC_Image(); - $image->loadFromData($bp); + $image->loadFromData((string) $bp); //check if image object is valid return $image->valid() ? $image : null; } diff --git a/lib/private/Preview/SVG.php b/lib/private/Preview/SVG.php index 6630dc2697876..790dad5c26cd2 100644 --- a/lib/private/Preview/SVG.php +++ b/lib/private/Preview/SVG.php @@ -70,7 +70,7 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage { //new image object $image = new \OC_Image(); - $image->loadFromData($svg); + $image->loadFromData((string) $svg); //check if image object is valid if ($image->valid()) { $image->scaleDownToFit($maxX, $maxY); diff --git a/lib/private/legacy/OC_Image.php b/lib/private/legacy/OC_Image.php index ed173849c769c..f93bcfaaaf481 100644 --- a/lib/private/legacy/OC_Image.php +++ b/lib/private/legacy/OC_Image.php @@ -102,7 +102,7 @@ public function valid() { // apparently you can't name a method 'empty'... if (is_resource($this->resource)) { return true; } - if (is_object($this->resource) && get_class($this->resource) === 'GdImage') { + if (is_object($this->resource) && get_class($this->resource) === \GdImage::class) { return true; } @@ -309,7 +309,7 @@ public function __invoke() { } /** - * @param resource Returns the image resource in any. + * @param resource|\GdImage $resource * @throws \InvalidArgumentException in case the supplied resource does not have the type "gd" */ public function setResource($resource) { @@ -319,7 +319,7 @@ public function setResource($resource) { return; } // PHP 8 has real objects for GD stuff - if (is_object($resource) && get_class($resource) === 'GdImage') { + if (is_object($resource) && get_class($resource) === \GdImage::class) { $this->resource = $resource; return; } @@ -327,7 +327,7 @@ public function setResource($resource) { } /** - * @return resource Returns the image resource in any. + * @return resource|\GdImage Returns the image resource in any. */ public function resource() { return $this->resource; @@ -537,7 +537,7 @@ public function fixOrientation() { * It is the responsibility of the caller to position the pointer at the correct place and to close the handle again. * * @param resource $handle - * @return resource|false An image resource or false on error + * @return resource|\GdImage|false An image resource or false on error */ public function loadFromFileHandle($handle) { $contents = stream_get_contents($handle); @@ -551,7 +551,7 @@ public function loadFromFileHandle($handle) { * Loads an image from a local file. * * @param bool|string $imagePath The path to a local file. - * @return bool|resource An image resource or false on error + * @return bool|resource|\GdImage An image resource or false on error */ public function loadFromFile($imagePath = false) { // exif_imagetype throws "read error!" if file is less than 12 byte @@ -667,17 +667,17 @@ public function loadFromFile($imagePath = false) { * Loads an image from a string of data. * * @param string $str A string of image data as read from a file. - * @return bool|resource An image resource or false on error + * @return bool|resource|\GdImage An image resource or false on error */ public function loadFromData($str) { - if (is_resource($str)) { + if (!is_string($str)) { return false; } $this->resource = @imagecreatefromstring($str); if ($this->fileInfo) { $this->mimeType = $this->fileInfo->buffer($str); } - if (is_resource($this->resource)) { + if ($this->valid()) { imagealphablending($this->resource, false); imagesavealpha($this->resource, true); } @@ -693,7 +693,7 @@ public function loadFromData($str) { * Loads an image from a base64 encoded string. * * @param string $str A string base64 encoded string of image data. - * @return bool|resource An image resource or false on error + * @return bool|resource|\GdImage An image resource or false on error */ public function loadFromBase64($str) { if (!is_string($str)) { @@ -723,7 +723,7 @@ public function loadFromBase64($str) { * @param string $fileName

* Path to the BMP image. *

- * @return bool|resource an image resource identifier on success, FALSE on errors. + * @return bool|resource|\GdImage an image resource identifier on success, FALSE on errors. */ private function imagecreatefrombmp($fileName) { if (!($fh = fopen($fileName, 'rb'))) { @@ -879,12 +879,12 @@ public function resize($maxSize) { $result = $this->resizeNew($maxSize); imagedestroy($this->resource); $this->resource = $result; - return is_resource($result); + return $this->valid(); } /** * @param $maxSize - * @return resource | bool + * @return resource|bool|\GdImage */ private function resizeNew($maxSize) { if (!$this->valid()) { @@ -915,14 +915,14 @@ public function preciseResize(int $width, int $height): bool { $result = $this->preciseResizeNew($width, $height); imagedestroy($this->resource); $this->resource = $result; - return is_resource($result); + return $this->valid(); } /** * @param int $width * @param int $height - * @return resource | bool + * @return resource|bool|\GdImage */ public function preciseResizeNew(int $width, int $height) { if (!$this->valid()) { @@ -1024,7 +1024,7 @@ public function crop(int $x, int $y, int $w, int $h): bool { $result = $this->cropNew($x, $y, $w, $h); imagedestroy($this->resource); $this->resource = $result; - return is_resource($result); + return $this->valid(); } /** @@ -1192,7 +1192,7 @@ public function __destruct() { * @link http://www.programmierer-forum.de/imagebmp-gute-funktion-gefunden-t143716.htm * @author mgutt * @version 1.00 - * @param resource $im + * @param resource|\GdImage $im * @param string $fileName [optional]

The path to save the file to.

* @param int $bit [optional]

Bit depth, (default is 24).

* @param int $compression [optional] diff --git a/lib/public/IImage.php b/lib/public/IImage.php index aa1cf344e5fc2..ea9f6e40190c0 100644 --- a/lib/public/IImage.php +++ b/lib/public/IImage.php @@ -99,7 +99,7 @@ public function show($mimeType = null); public function save($filePath = null, $mimeType = null); /** - * @return resource Returns the image resource in any. + * @return resource|\GdImage Returns the image resource in any. * @since 8.1.0 */ public function resource(); diff --git a/psalm.xml b/psalm.xml index d51dbb9dde686..bc21dbceffeac 100644 --- a/psalm.xml +++ b/psalm.xml @@ -80,6 +80,8 @@ + + @@ -123,6 +125,8 @@ + + From efd689ea828020bb6fc1e2de57f617e9f2e6a718 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Oct 2021 11:55:43 +0200 Subject: [PATCH 2/4] Revert "Do not run image tests on php8" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit d690f909284ae4bb4dee7d00318104ee76720bfa. Signed-off-by: Côme Chilliet --- tests/lib/ImageTest.php | 68 ----------------------------------------- 1 file changed, 68 deletions(-) diff --git a/tests/lib/ImageTest.php b/tests/lib/ImageTest.php index ebf00392d875f..5b83c4ac57fbb 100644 --- a/tests/lib/ImageTest.php +++ b/tests/lib/ImageTest.php @@ -20,10 +20,6 @@ public static function tearDownAfterClass(): void { } public function testConstructDestruct() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $this->assertInstanceOf('\OC_Image', $img); @@ -51,10 +47,6 @@ public function testConstructDestruct() { } public function testValid() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $this->assertTrue($img->valid()); @@ -69,10 +61,6 @@ public function testValid() { } public function testMimeType() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $this->assertEquals('image/png', $img->mimeType()); @@ -90,10 +78,6 @@ public function testMimeType() { } public function testWidth() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $this->assertEquals(128, $img->width()); @@ -111,10 +95,6 @@ public function testWidth() { } public function testHeight() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $this->assertEquals(128, $img->height()); @@ -132,10 +112,6 @@ public function testHeight() { } public function testSave() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $img->resize(16); @@ -150,10 +126,6 @@ public function testSave() { } public function testData() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $raw = imagecreatefromstring(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.png')); @@ -188,10 +160,6 @@ public function testData() { } public function testDataNoResource() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $this->assertNull($img->data()); } @@ -200,10 +168,6 @@ public function testDataNoResource() { * @depends testData */ public function testToString() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $expected = base64_encode($img->data()); @@ -221,10 +185,6 @@ public function testToString() { } public function testResize() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $this->assertTrue($img->resize(32)); @@ -245,10 +205,6 @@ public function testResize() { } public function testPreciseResize() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $this->assertTrue($img->preciseResize(128, 512)); @@ -269,10 +225,6 @@ public function testPreciseResize() { } public function testCenterCrop() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $img->centerCrop(); @@ -293,10 +245,6 @@ public function testCenterCrop() { } public function testCrop() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $this->assertTrue($img->crop(0, 0, 50, 20)); @@ -332,10 +280,6 @@ public static function sampleProvider() { * @param int[] $expected */ public function testFitIn($filename, $asked, $expected) { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT . '/tests/data/' . $filename); $this->assertTrue($img->fitIn($asked[0], $asked[1])); @@ -359,10 +303,6 @@ public static function sampleFilenamesProvider() { * @param string $filename */ public function testScaleDownToFitWhenSmallerAlready($filename) { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/' . $filename); $currentWidth = $img->width(); @@ -396,10 +336,6 @@ public static function largeSampleProvider() { * @param int[] $expected */ public function testScaleDownWhenBigger($filename, $asked, $expected) { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/' . $filename); //$this->assertTrue($img->scaleDownToFit($asked[0], $asked[1])); @@ -420,10 +356,6 @@ public function convertDataProvider() { * @dataProvider convertDataProvider */ public function testConvert($mimeType) { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $img = new \OC_Image(); $img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png'); $tempFile = tempnam(sys_get_temp_dir(), 'img-test'); From 8bc3bed0930939f887ddda53fdc50ef2a6f04119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 26 Oct 2021 12:11:45 +0200 Subject: [PATCH 3/4] Enable Avatar tests as well for PHP>=8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/lib/Avatar/UserAvatarTest.php | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/tests/lib/Avatar/UserAvatarTest.php b/tests/lib/Avatar/UserAvatarTest.php index 31f2a6ebf5bd5..6c902e10829c2 100644 --- a/tests/lib/Avatar/UserAvatarTest.php +++ b/tests/lib/Avatar/UserAvatarTest.php @@ -52,10 +52,6 @@ public function avatarTextData() { } public function testGetNoAvatar() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $file = $this->createMock(ISimpleFile::class); $this->folder->method('newFile') ->willReturn($file); @@ -91,10 +87,6 @@ public function testGetNoAvatar() { } public function testGetAvatarSizeMatch() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $this->folder->method('fileExists') ->willReturnMap([ ['avatar.jpg', true], @@ -112,10 +104,6 @@ public function testGetAvatarSizeMatch() { } public function testGetAvatarSizeMinusOne() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $this->folder->method('fileExists') ->willReturnMap([ ['avatar.jpg', true], @@ -132,10 +120,6 @@ public function testGetAvatarSizeMinusOne() { } public function testGetAvatarNoSizeMatch() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $this->folder->method('fileExists') ->willReturnMap([ ['avatar.png', true], @@ -200,10 +184,6 @@ public function testExistsPNG() { } public function testSetAvatar() { - if (PHP_MAJOR_VERSION > 7) { - $this->markTestSkipped('Only run on php7'); - } - $avatarFileJPG = $this->createMock(File::class); $avatarFileJPG->method('getName') ->willReturn('avatar.jpg'); From bd59877ec56b5bdb93c0a47d37f7c48b798677c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 2 Nov 2021 11:07:40 +0100 Subject: [PATCH 4/4] Fix psalm-ocp error about GdImage class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- psalm-ocp.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/psalm-ocp.xml b/psalm-ocp.xml index 5574ae0fb5dc1..4519b7e9ca46a 100644 --- a/psalm-ocp.xml +++ b/psalm-ocp.xml @@ -16,4 +16,12 @@ + + + + + + + +