From 11b51ddbbe33cd19de09194a324669c81e7ce27d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 2 Dec 2021 11:06:17 +0100 Subject: [PATCH 1/3] Avoid calling image* methods on boolean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoids fatal errors on PHP>=8, and warnings on older versions. Log should also be clearer. Signed-off-by: Côme Chilliet --- lib/private/legacy/OC_Image.php | 41 ++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/lib/private/legacy/OC_Image.php b/lib/private/legacy/OC_Image.php index b9ef31ed9fac2..e6a815a3cf7e2 100644 --- a/lib/private/legacy/OC_Image.php +++ b/lib/private/legacy/OC_Image.php @@ -6,6 +6,7 @@ * @author Bart Visscher * @author Björn Schießle * @author Byron Marohn + * @author Côme Chilliet * @author Christopher Schäpers * @author Christoph Wurst * @author Georg Ehrke @@ -45,7 +46,7 @@ * Class for basic image manipulation */ class OC_Image implements \OCP\IImage { - /** @var false|resource */ + /** @var false|resource|\GdImage */ protected $resource = false; // tmp resource. /** @var int */ protected $imageType = IMAGETYPE_PNG; // Default to png if file type isn't evident. @@ -67,7 +68,7 @@ class OC_Image implements \OCP\IImage { /** * Constructor. * - * @param resource|string $imageRef The path to a local file, a base64 encoded string or a resource created by + * @param resource|string|\GdImage $imageRef The path to a local file, a base64 encoded string or a resource created by * an imagecreate* function. * @param \OCP\ILogger $logger * @param \OCP\IConfig $config @@ -97,7 +98,7 @@ public function __construct($imageRef = null, \OCP\ILogger $logger = null, \OCP\ * * @return bool */ - public function valid() { // apparently you can't name a method 'empty'... + public function valid() { if (is_resource($this->resource)) { return true; } @@ -326,7 +327,7 @@ public function setResource($resource) { } /** - * @return resource|\GdImage Returns the image resource in any. + * @return false|resource|\GdImage Returns the image resource if any */ public function resource() { return $this->resource; @@ -468,6 +469,10 @@ public function readExif($data) { * @return bool */ public function fixOrientation() { + if (!$this->valid()) { + $this->logger->error(__METHOD__ . '(): No image loaded', ['app' => 'core']); + return false; + } $o = $this->getOrientation(); $this->logger->debug('OC_Image->fixOrientation() Orientation: ' . $o, ['app' => 'core']); $rotate = 0; @@ -875,6 +880,10 @@ private function imagecreatefrombmp($fileName) { * @return bool */ public function resize($maxSize) { + if (!$this->valid()) { + $this->logger->error(__METHOD__ . '(): No image loaded', ['app' => 'core']); + return false; + } $result = $this->resizeNew($maxSize); imagedestroy($this->resource); $this->resource = $result; @@ -911,6 +920,10 @@ private function resizeNew($maxSize) { * @return bool */ public function preciseResize(int $width, int $height): bool { + if (!$this->valid()) { + $this->logger->error(__METHOD__ . '(): No image loaded', ['app' => 'core']); + return false; + } $result = $this->preciseResizeNew($width, $height); imagedestroy($this->resource); $this->resource = $result; @@ -990,9 +1003,8 @@ public function centerCrop($size = 0) { $targetHeight = $height; } $process = imagecreatetruecolor($targetWidth, $targetHeight); - if ($process == false) { + if ($process === false) { $this->logger->error('OC_Image->centerCrop, Error creating true color image', ['app' => 'core']); - imagedestroy($process); return false; } @@ -1004,9 +1016,8 @@ public function centerCrop($size = 0) { } imagecopyresampled($process, $this->resource, 0, 0, $x, $y, $targetWidth, $targetHeight, $width, $height); - if ($process == false) { + if ($process === false) { $this->logger->error('OC_Image->centerCrop, Error re-sampling process image ' . $width . 'x' . $height, ['app' => 'core']); - imagedestroy($process); return false; } imagedestroy($this->resource); @@ -1024,6 +1035,10 @@ public function centerCrop($size = 0) { * @return bool for success or failure */ public function crop(int $x, int $y, int $w, int $h): bool { + if (!$this->valid()) { + $this->logger->error(__METHOD__ . '(): No image loaded', ['app' => 'core']); + return false; + } $result = $this->cropNew($x, $y, $w, $h); imagedestroy($this->resource); $this->resource = $result; @@ -1037,7 +1052,7 @@ public function crop(int $x, int $y, int $w, int $h): bool { * @param int $y Vertical position * @param int $w Width * @param int $h Height - * @return resource | bool + * @return resource|\GdImage|false */ public function cropNew(int $x, int $y, int $w, int $h) { if (!$this->valid()) { @@ -1045,9 +1060,8 @@ public function cropNew(int $x, int $y, int $w, int $h) { return false; } $process = imagecreatetruecolor($w, $h); - if ($process == false) { + if ($process === false) { $this->logger->error(__METHOD__ . '(): Error creating true color image', ['app' => 'core']); - imagedestroy($process); return false; } @@ -1059,9 +1073,8 @@ public function cropNew(int $x, int $y, int $w, int $h) { } imagecopyresampled($process, $this->resource, 0, 0, $x, $y, $w, $h, $w, $h); - if ($process == false) { + if ($process === false) { $this->logger->error(__METHOD__ . '(): Error re-sampling process image ' . $w . 'x' . $h, ['app' => 'core']); - imagedestroy($process); return false; } return $process; @@ -1168,7 +1181,7 @@ public function destroy() { if ($this->valid()) { imagedestroy($this->resource); } - $this->resource = null; + $this->resource = false; } public function __destruct() { From 3fe20bfbbbc3dc9fe3651d17d45ac6e3ab3abc2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 2 Dec 2021 11:30:10 +0100 Subject: [PATCH 2/3] Fix typing problems in OC_Image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/legacy/OC_Image.php | 12 ++++++++++-- lib/public/IImage.php | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/private/legacy/OC_Image.php b/lib/private/legacy/OC_Image.php index e6a815a3cf7e2..1b1d98325baad 100644 --- a/lib/private/legacy/OC_Image.php +++ b/lib/private/legacy/OC_Image.php @@ -124,7 +124,11 @@ public function mimeType() { * @return int */ public function width() { - return $this->valid() ? imagesx($this->resource) : -1; + if ($this->valid() && (($width = imagesx($this->resource)) !== false)) { + return $width; + } else { + return -1; + } } /** @@ -133,7 +137,11 @@ public function width() { * @return int */ public function height() { - return $this->valid() ? imagesy($this->resource) : -1; + if ($this->valid() && (($height = imagesy($this->resource)) !== false)) { + return $height; + } else { + return -1; + } } /** diff --git a/lib/public/IImage.php b/lib/public/IImage.php index 9d2b31e0e2868..659cd24720d7c 100644 --- a/lib/public/IImage.php +++ b/lib/public/IImage.php @@ -98,7 +98,7 @@ public function show($mimeType = null); public function save($filePath = null, $mimeType = null); /** - * @return resource|\GdImage Returns the image resource in any. + * @return false|resource|\GdImage Returns the image resource if any * @since 8.1.0 */ public function resource(); From f889b251b1f4d7542cf628c5502548a6620f93d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 2 Dec 2021 15:38:42 +0100 Subject: [PATCH 3/3] Avoid assignment in if clause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/legacy/OC_Image.php | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/private/legacy/OC_Image.php b/lib/private/legacy/OC_Image.php index 1b1d98325baad..333a9d919efb3 100644 --- a/lib/private/legacy/OC_Image.php +++ b/lib/private/legacy/OC_Image.php @@ -124,11 +124,13 @@ public function mimeType() { * @return int */ public function width() { - if ($this->valid() && (($width = imagesx($this->resource)) !== false)) { - return $width; - } else { - return -1; + if ($this->valid()) { + $width = imagesx($this->resource); + if ($width !== false) { + return $width; + } } + return -1; } /** @@ -137,11 +139,13 @@ public function width() { * @return int */ public function height() { - if ($this->valid() && (($height = imagesy($this->resource)) !== false)) { - return $height; - } else { - return -1; + if ($this->valid()) { + $height = imagesy($this->resource); + if ($height !== false) { + return $height; + } } + return -1; } /**