Fix handling of WebP avatars
authorMarcel Werk <burntime@woltlab.com>
Tue, 9 May 2023 11:28:13 +0000 (13:28 +0200)
committerMarcel Werk <burntime@woltlab.com>
Tue, 9 May 2023 11:28:13 +0000 (13:28 +0200)
Closes #5264

wcfsetup/install/files/lib/data/user/avatar/UserAvatarEditor.class.php
wcfsetup/install/files/lib/system/upload/AvatarUploadFileValidationStrategy.class.php

index 753ee3be2de4dd4a93496d2950b1eb0cb640493b..a90f6401b248a1cd9f0de80c01ab8918505291a1 100644 (file)
@@ -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;
     }
 }
index 3de4886fe1c80b748ff81f9b2f7ba89199be3471..0def81e1553d470a7ddd326fcbfa5f762a3f5f22 100644 (file)
@@ -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) {