From: Johan Cwiklinski Date: Thu, 7 Mar 2024 09:37:06 +0000 (+0100) Subject: Use FileTrait capacities for Picture uploads X-Git-Url: https://git.agnieray.net/?a=commitdiff_plain;h=5eddda380d040604f8d67021a141e88dc9fef068;p=galette.git Use FileTrait capacities for Picture uploads --- diff --git a/galette/lib/Galette/Core/Picture.php b/galette/lib/Galette/Core/Picture.php index 0b9abab60..53ac7cde7 100644 --- a/galette/lib/Galette/Core/Picture.php +++ b/galette/lib/Galette/Core/Picture.php @@ -40,12 +40,16 @@ use Galette\IO\FileTrait; */ class Picture implements FileInterface { - use FileTrait; + use FileTrait { + writeOnDisk as protected trait_writeOnDisk; + store as protected trait_store; + getMimeType as protected trait_getMimeType; + } //constants that will not be overridden public const SQL_ERROR = -10; public const SQL_BLOB_ERROR = -11; - //constants that can be overrided + //constants that can be overridden //(do not use self::CONSTANT, but get_class[$this]::CONSTANT) public const TABLE = 'pictures'; public const PK = Adherent::PK; @@ -66,6 +70,8 @@ class Picture implements FileInterface protected int $max_width = 200; protected int $max_height = 200; private StatementInterface $insert_stmt; + /** @var ?array */ + private ?array $cropping; /** * Default constructor. @@ -74,7 +80,6 @@ class Picture implements FileInterface */ public function __construct($id_adh = null) { - $this->init( null, array('jpeg', 'jpg', 'png', 'gif', 'webp'), @@ -414,116 +419,83 @@ class Picture implements FileInterface */ public function store(array $file, bool $ajax = false, array $cropping = null): bool|int { - /** TODO: fix max size (by preferences ?) */ - global $zdb; - - $class = get_class($this); - - $name = $file['name']; - $tmpfile = $file['tmp_name']; + $this->cropping = $cropping; + return $this->trait_store($file, $ajax); + } - //First, does the file have a valid name? - $reg = "/^([^" . implode('', $this->bad_chars) . "]+)\.(" . - implode('|', $this->allowed_extensions) . ")$/i"; - if (preg_match($reg, $name, $matches)) { - Analog::log( - '[' . $class . '] Filename and extension are OK, proceed.', - Analog::DEBUG - ); - $extension = strtolower($matches[2]); - if ($extension == 'jpeg') { - //jpeg is an allowed extension, - //but we change it to jpg to reduce further tests :) - $extension = 'jpg'; - } - } else { - $erreg = "/^([^" . implode('', $this->bad_chars) . "]+)\.(.*)/i"; - $m = preg_match($erreg, $name, $errmatches); - - $err_msg = '[' . $class . '] '; - if ($m == 1) { - //ok, we got a good filename and an extension. Extension is bad :) - $err_msg .= 'Invalid extension for file ' . $name . '.'; - $ret = self::INVALID_EXTENSION; - } else { - $err_msg = 'Invalid filename `' . $name . '` (Tip: '; - $err_msg .= preg_replace( - '|%s|', - htmlentities($this->getBadChars()), - "file name should not contain any of: %s). " - ); - $ret = self::INVALID_FILENAME; - } + /** + * Build destination path + * + * @return string + */ + protected function buildDestPath(): string + { + return $this->dest_dir . $this->id . '.' . $this->extension; + } - Analog::log( - $err_msg, - Analog::ERROR - ); - return $ret; + /** + * Get file mime type + * + * @param string $file File + * + * @return string + */ + public static function getMimeType(string $file): string + { + $info = getimagesize($file); + if ($info !== false) { + return $info['mime']; } - //Second, let's check file size - if ($file['size'] > ($this->maxlenght * 1024)) { - Analog::log( - '[' . $class . '] File is too big (' . ($file['size'] * 1024) . - 'Ko for maximum authorized ' . ($this->maxlenght * 1024) . - 'Ko', - Analog::ERROR - ); - return self::FILE_TOO_BIG; - } else { - Analog::log('[' . $class . '] Filesize is OK, proceed', Analog::DEBUG); - } + //fallback if file is not an image + return static::trait_getMimeType($file); + } - $current = getimagesize($tmpfile); + /** + * Write file on disk + * + * @param string $tmpfile Temporary file + * @param bool $ajax If the file comes from an ajax call (dnd) + * + * @return bool|int + */ + public function writeOnDisk(string $tmpfile, bool $ajax): bool|int + { + global $zdb; - if (!in_array($current['mime'], $this->allowed_mimes)) { - Analog::log( - '[' . $class . '] Mimetype `' . $current['mime'] . '` not allowed', - Analog::ERROR - ); - return self::MIME_NOT_ALLOWED; - } else { - Analog::log( - '[' . $class . '] Mimetype is allowed, proceed', - Analog::DEBUG - ); - } + $this->setDestDir($this->store_path); + $current = getimagesize($tmpfile); // Source image must have minimum dimensions to match the cropping process requirements // and ensure the final picture will fit the maximum allowed resizing dimensions. - if (isset($cropping['ratio']) && isset($cropping['focus'])) { + if (isset($this->cropping['ratio']) && isset($this->cropping['focus'])) { if ($current[0] < $this->mincropsize || $current[1] < $this->mincropsize) { $min_current = min($current[0], $current[1]); Analog::log( - '[' . $class . '] Image is too small. The minimum image side size allowed is ' . + '[' . get_class($this) . '] Image is too small. The minimum image side size allowed is ' . $this->mincropsize . 'px, but current is ' . $min_current . 'px.', Analog::ERROR ); return self::IMAGE_TOO_SMALL; } else { - Analog::log('[' . $class . '] Image dimensions are OK, proceed', Analog::DEBUG); + Analog::log('[' . get_class($this) . '] Image dimensions are OK, proceed', Analog::DEBUG); } } - $this->delete(); - $new_file = $this->store_path . - $this->id . '.' . $extension; - if ($ajax === true) { - rename($tmpfile, $new_file); - } else { - move_uploaded_file($tmpfile, $new_file); + $result = $this->trait_writeOnDisk($tmpfile, $ajax); + if ($result !== true) { + return $result; } // current[0] gives width ; current[1] gives height if ($current[0] > $this->max_width || $current[1] > $this->max_height) { /** FIXME: what if image cannot be resized? - Should'nt we want to stop the process here? */ - $this->resizeImage($new_file, $extension, null, $cropping); + Should'nt we want to stop the process here? */ + $this->resizeImage($this->buildDestPath(), $this->extension, null, $this->cropping); } - return $this->storeInDb($zdb, $this->db_id, $new_file, $extension); + return $this->storeInDb($zdb, $this->db_id, $this->buildDestPath(), $this->extension); } /** diff --git a/galette/lib/Galette/IO/FileTrait.php b/galette/lib/Galette/IO/FileTrait.php index 1ab3880c4..440224ca9 100644 --- a/galette/lib/Galette/IO/FileTrait.php +++ b/galette/lib/Galette/IO/FileTrait.php @@ -31,7 +31,7 @@ use Analog\Analog; trait FileTrait { - //array keys contain litteral value of each forbidden character + //array keys contain literal value of each forbidden character //(to be used when showing an error). //Maybe is there a better way to handle this... /** @var array */ @@ -51,6 +51,8 @@ trait FileTrait ); protected ?string $name; + protected ?string $name_wo_ext; + protected ?string $extension; protected ?string $dest_dir; /** @var array */ protected $allowed_extensions = array(); @@ -249,6 +251,13 @@ trait FileTrait '[' . $class . '] Filename and extension are OK, proceed.', Analog::DEBUG ); + $this->name_wo_ext = $matches[1]; + $this->extension = strtolower($matches[2]); + if ($this->extension == 'jpeg') { + //jpeg is an allowed extension, + //but we change it to jpg to reduce further tests :) + $this->extension = 'jpg'; + } } else { $erreg = "/^([^" . implode('', $this->bad_chars) . "]+)\.(.*)/i"; $m = preg_match($erreg, $this->name, $errmatches); @@ -306,11 +315,34 @@ trait FileTrait ); } - $new_file = $this->dest_dir . $this->name; + return $this->writeOnDisk($tmpfile, $ajax); + } + + /** + * Build destination path + * + * @return string + */ + protected function buildDestPath(): string + { + return $this->dest_dir . $this->name; + } + + /** + * Write file on disk + * + * @param string $tmpfile Temporary file + * @param bool $ajax If the file comes from an ajax call (dnd) + * + * @return bool|int + */ + public function writeOnDisk(string $tmpfile, bool $ajax): bool|int + { + $new_file = $this->buildDestPath(); if (file_exists($new_file)) { Analog::log( - '[' . $class . '] File `' . $new_file . '` already exists', + '[' . get_class($this) . '] File `' . $new_file . '` already exists', Analog::ERROR ); return self::NEW_FILE_EXISTS; diff --git a/tests/Galette/Core/tests/units/Picture.php b/tests/Galette/Core/tests/units/Picture.php index be60f8db1..d5a678699 100644 --- a/tests/Galette/Core/tests/units/Picture.php +++ b/tests/Galette/Core/tests/units/Picture.php @@ -142,7 +142,7 @@ class Picture extends TestCase * Test mimetype guess * FileInfo not installed, back to mime_content_type call * - * Does not actually works :/ + * Does not actually work :/ * * @return void */