From c560a932b1356a96ebf1e17b33ecb674dcc9616c Mon Sep 17 00:00:00 2001 From: Marcel Werk Date: Tue, 9 May 2023 13:28:13 +0200 Subject: [PATCH] Fix handling of WebP avatars Closes #5264 --- .../user/avatar/UserAvatarEditor.class.php | 60 ++++--------------- ...atarUploadFileValidationStrategy.class.php | 38 ++++-------- 2 files changed, 25 insertions(+), 73 deletions(-) diff --git a/wcfsetup/install/files/lib/data/user/avatar/UserAvatarEditor.class.php b/wcfsetup/install/files/lib/data/user/avatar/UserAvatarEditor.class.php index 753ee3be2d..a90f6401b2 100644 --- a/wcfsetup/install/files/lib/data/user/avatar/UserAvatarEditor.class.php +++ b/wcfsetup/install/files/lib/data/user/avatar/UserAvatarEditor.class.php @@ -3,10 +3,8 @@ namespace wcf\data\user\avatar; use wcf\data\DatabaseObjectEditor; -use wcf\system\exception\NotImplementedException; -use wcf\system\image\ImageHandler; use wcf\system\WCF; -use wcf\util\FileUtil; +use wcf\util\ImageUtil; /** * Provides functions to edit avatars. @@ -90,55 +88,21 @@ class UserAvatarEditor extends DatabaseObjectEditor return false; } - $filename = $this->getLocation(); - $filenameWebP = $this->getLocation(null, true); - - $imageAdapter = ImageHandler::getInstance()->getAdapter(); - $imageAdapter->loadFile($filename); - $image = $imageAdapter->getImage(); - - $data = ["hasWebP" => 1]; - - // If the uploaded avatar is already a WebP image, then create a JPEG - // as a fallback image and flip the image data to match the JPEG. - if ($this->avatarExtension === "webp") { - // This entire code path is completely broken, because writing into the - // ->data array of the UserAvatarEditor does not actually do anything - // for ->getLocation(), which is a method on the base UserAvatar. - // This is also unreachable, because WebP files are rejected unconditionally - // in AvatarUploadFileValidationStrategy. - throw new NotImplementedException(); - $filenameJpeg = \preg_replace('~\.webp$~', '.jpeg', $filenameWebP); - - $imageAdapter->saveImageAs($image, $filenameJpeg, "jpeg", 80); - - // The new file has a different SHA1 hash, which means that apart from - // updating the `fileHash` we also need to move it to a different physical - // location. - $newFileHash = \sha1_file($filenameJpeg); - - $tmpAvatar = clone $this; - $tmpAvatar->data["avatarExtension"] = "jpeg"; - $tmpAvatar->data["fileHash"] = $newFileHash; - $newLocation = $tmpAvatar->getLocation(null, false); - - $dir = \dirname($newLocation); - if (!\file_exists($dir)) { - FileUtil::makePath($dir); + $outputFilenameWithoutExtension = \preg_replace('~\.[a-z]+$~', '', $this->getLocation()); + $result = ImageUtil::createWebpVariant($this->getLocation(), $outputFilenameWithoutExtension); + if ($result !== null) { + $data = ['hasWebP' => 1]; + + // A fallback jpeg image was just created. + if ($result === false) { + $data['avatarExtension'] = 'jpg'; } - \rename($filenameJpeg, $newLocation); + $this->update($data); - $data = [ - "avatarExtension" => "jpeg", - "fileHash" => $newFileHash, - ]; - } else { - $imageAdapter->saveImageAs($image, $this->getLocation(null, true), "webp", 80); + return true; } - $this->update($data); - - return true; + return false; } } diff --git a/wcfsetup/install/files/lib/system/upload/AvatarUploadFileValidationStrategy.class.php b/wcfsetup/install/files/lib/system/upload/AvatarUploadFileValidationStrategy.class.php index 3de4886fe1..0def81e155 100644 --- a/wcfsetup/install/files/lib/system/upload/AvatarUploadFileValidationStrategy.class.php +++ b/wcfsetup/install/files/lib/system/upload/AvatarUploadFileValidationStrategy.class.php @@ -37,32 +37,20 @@ class AvatarUploadFileValidationStrategy extends DefaultUploadFileValidationStra return false; } else { - switch ($imageData[2]) { - case \IMAGETYPE_WEBP: - // Reject WebP images regardless of any file extension restriction, they are - // neither supported in Safari nor in Internet Explorer 11. We can safely lift - // this restriction once Apple implements the support or if any sort of fall- - // back mechanism is implemented: https://github.com/WoltLab/WCF/issues/2838 + // Validate the mime type against the list of allowed extensions. + // + // We usually don't care about the extension, restricting allowed file extensions + // primarily exists to prevent users from uploaded clickable '.exe'. The software + // itself only ever uses the mime type. + // + // In the case of avatars, though, the administrator might want to disallow uploading + // GIF files to prevent the most common case of animated avatar, thus we specifically + // validate the mime type against the extension. + $extension = \image_type_to_extension($imageData[2], false); + if (!\in_array($extension, $this->fileExtensions)) { + $uploadFile->setValidationErrorType('invalidExtension'); - $uploadFile->setValidationErrorType('invalidExtension'); - - return false; - default: - // Validate the mime type against the list of allowed extensions. - // - // We usually don't care about the extension, restricting allowed file extensions - // primarily exists to prevent users from uploaded clickable '.exe'. The software - // itself only ever uses the mime type. - // - // In the case of avatars, though, the administrator might want to disallow uploading - // GIF files to prevent the most common case of animated avatar, thus we specifically - // validate the mime type against the extension. - $extension = \image_type_to_extension($imageData[2], false); - if (!\in_array($extension, $this->fileExtensions)) { - $uploadFile->setValidationErrorType('invalidExtension'); - - return false; - } + return false; } } } catch (SystemException $e) { -- 2.20.1