Merge pull request #2117 from flarum/fl/2055-streamline-uploads

Simplify uploads, avoid Application contract

framework/core/composer.json

@@ -56,7 +56,7 @@
         "illuminate/support": "5.7.*",
         "illuminate/validation": "5.7.*",
         "illuminate/view": "5.7.*",
-        "intervention/image": "^2.3.0",
+        "intervention/image": "^2.5.0",
         "laminas/laminas-diactoros": "^1.8.4",
         "laminas/laminas-httphandlerrunner": "^1.0",
         "laminas/laminas-stratigility": "^3.0",

framework/core/src/Api/Controller/DeleteFaviconController.php

@@ -9,12 +9,10 @@
 
 namespace Flarum\Api\Controller;
 
-use Flarum\Foundation\Application;
 use Flarum\Settings\SettingsRepositoryInterface;
 use Flarum\User\AssertPermissionTrait;
 use Laminas\Diactoros\Response\EmptyResponse;
-use League\Flysystem\Adapter\Local;
+use League\Flysystem\FilesystemInterface;
-use League\Flysystem\Filesystem;
 use Psr\Http\Message\ServerRequestInterface;
 
 class DeleteFaviconController extends AbstractDeleteController
@@ -27,17 +25,18 @@ class DeleteFaviconController extends AbstractDeleteController
     protected $settings;
 
     /**
-     * @var Application
+     * @var FilesystemInterface
      */
-    protected $app;
+    protected $uploadDir;
 
     /**
      * @param SettingsRepositoryInterface $settings
+     * @param FilesystemInterface $uploadDir
      */
-    public function __construct(SettingsRepositoryInterface $settings, Application $app)
+    public function __construct(SettingsRepositoryInterface $settings, FilesystemInterface $uploadDir)
     {
         $this->settings = $settings;
-        $this->app = $app;
+        $this->uploadDir = $uploadDir;
     }
 
     /**
@@ -51,10 +50,8 @@ class DeleteFaviconController extends AbstractDeleteController
 
         $this->settings->set('favicon_path', null);
 
-        $uploadDir = new Filesystem(new Local($this->app->publicPath().'/assets'));
+        if ($this->uploadDir->has($path)) {
-
+            $this->uploadDir->delete($path);
-        if ($uploadDir->has($path)) {
-            $uploadDir->delete($path);
         }
 
         return new EmptyResponse(204);

framework/core/src/Api/Controller/DeleteLogoController.php

@@ -9,12 +9,10 @@
 
 namespace Flarum\Api\Controller;
 
-use Flarum\Foundation\Application;
 use Flarum\Settings\SettingsRepositoryInterface;
 use Flarum\User\AssertPermissionTrait;
 use Laminas\Diactoros\Response\EmptyResponse;
-use League\Flysystem\Adapter\Local;
+use League\Flysystem\FilesystemInterface;
-use League\Flysystem\Filesystem;
 use Psr\Http\Message\ServerRequestInterface;
 
 class DeleteLogoController extends AbstractDeleteController
@@ -27,17 +25,18 @@ class DeleteLogoController extends AbstractDeleteController
     protected $settings;
 
     /**
-     * @var Application
+     * @var FilesystemInterface
      */
-    protected $app;
+    protected $uploadDir;
 
     /**
      * @param SettingsRepositoryInterface $settings
+     * @param FilesystemInterface $uploadDir
      */
-    public function __construct(SettingsRepositoryInterface $settings, Application $app)
+    public function __construct(SettingsRepositoryInterface $settings, FilesystemInterface $uploadDir)
     {
         $this->settings = $settings;
-        $this->app = $app;
+        $this->uploadDir = $uploadDir;
     }
 
     /**
@@ -51,10 +50,8 @@ class DeleteLogoController extends AbstractDeleteController
 
         $this->settings->set('logo_path', null);
 
-        $uploadDir = new Filesystem(new Local($this->app->publicPath().'/assets'));
+        if ($this->uploadDir->has($path)) {
-
+            $this->uploadDir->delete($path);
-        if ($uploadDir->has($path)) {
-            $uploadDir->delete($path);
         }
 
         return new EmptyResponse(204);

framework/core/src/Api/Controller/UploadFaviconController.php

@@ -9,15 +9,12 @@
 
 namespace Flarum\Api\Controller;
 
-use Flarum\Foundation\Application;
 use Flarum\Settings\SettingsRepositoryInterface;
 use Flarum\User\AssertPermissionTrait;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Str;
 use Intervention\Image\ImageManager;
-use League\Flysystem\Adapter\Local;
+use League\Flysystem\FilesystemInterface;
-use League\Flysystem\Filesystem;
-use League\Flysystem\MountManager;
 use Psr\Http\Message\ServerRequestInterface;
 use Tobscure\JsonApi\Document;
 
@@ -31,17 +28,18 @@ class UploadFaviconController extends ShowForumController
     protected $settings;
 
     /**
-     * @var Application
+     * @var FilesystemInterface
      */
-    protected $app;
+    protected $uploadDir;
 
     /**
      * @param SettingsRepositoryInterface $settings
+     * @param FilesystemInterface $uploadDir
      */
-    public function __construct(SettingsRepositoryInterface $settings, Application $app)
+    public function __construct(SettingsRepositoryInterface $settings, FilesystemInterface $uploadDir)
     {
         $this->settings = $settings;
-        $this->app = $app;
+        $this->uploadDir = $uploadDir;
     }
 
     /**
@@ -52,36 +50,28 @@ class UploadFaviconController extends ShowForumController
         $this->assertAdmin($request->getAttribute('actor'));
 
         $file = Arr::get($request->getUploadedFiles(), 'favicon');
-
-        $tmpFile = tempnam($this->app->storagePath().'/tmp', 'favicon');
-        $file->moveTo($tmpFile);
-
         $extension = pathinfo($file->getClientFilename(), PATHINFO_EXTENSION);
 
-        if ($extension !== 'ico') {
+        if ($extension === 'ico') {
+            $image = $file->getStream();
+        } else {
             $manager = new ImageManager;
 
-            $encodedImage = $manager->make($tmpFile)->resize(64, 64, function ($constraint) {
+            $image = $manager->make($file->getStream())->resize(64, 64, function ($constraint) {
                 $constraint->aspectRatio();
                 $constraint->upsize();
             })->encode('png');
-            file_put_contents($tmpFile, $encodedImage);
 
             $extension = 'png';
         }
 
-        $mount = new MountManager([
+        if (($path = $this->settings->get('favicon_path')) && $this->uploadDir->has($path)) {
-            'source' => new Filesystem(new Local(pathinfo($tmpFile, PATHINFO_DIRNAME))),
+            $this->uploadDir->delete($path);
-            'target' => new Filesystem(new Local($this->app->publicPath().'/assets')),
-        ]);
-
-        if (($path = $this->settings->get('favicon_path')) && $mount->has($file = "target://$path")) {
-            $mount->delete($file);
         }
 
         $uploadName = 'favicon-'.Str::lower(Str::random(8)).'.'.$extension;
 
-        $mount->move('source://'.pathinfo($tmpFile, PATHINFO_BASENAME), "target://$uploadName");
+        $this->uploadDir->write($uploadName, $image);
 
         $this->settings->set('favicon_path', $uploadName);
 

framework/core/src/Api/Controller/UploadLogoController.php

@@ -9,15 +9,12 @@
 
 namespace Flarum\Api\Controller;
 
-use Flarum\Foundation\Application;
 use Flarum\Settings\SettingsRepositoryInterface;
 use Flarum\User\AssertPermissionTrait;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Str;
 use Intervention\Image\ImageManager;
-use League\Flysystem\Adapter\Local;
+use League\Flysystem\FilesystemInterface;
-use League\Flysystem\Filesystem;
-use League\Flysystem\MountManager;
 use Psr\Http\Message\ServerRequestInterface;
 use Tobscure\JsonApi\Document;
 
@@ -31,17 +28,18 @@ class UploadLogoController extends ShowForumController
     protected $settings;
 
     /**
-     * @var Application
+     * @var FilesystemInterface
      */
-    protected $app;
+    protected $uploadDir;
 
     /**
      * @param SettingsRepositoryInterface $settings
+     * @param FilesystemInterface $uploadDir
      */
-    public function __construct(SettingsRepositoryInterface $settings, Application $app)
+    public function __construct(SettingsRepositoryInterface $settings, FilesystemInterface $uploadDir)
     {
         $this->settings = $settings;
-        $this->app = $app;
+        $this->uploadDir = $uploadDir;
     }
 
     /**
@@ -53,28 +51,19 @@ class UploadLogoController extends ShowForumController
 
         $file = Arr::get($request->getUploadedFiles(), 'logo');
 
-        $tmpFile = tempnam($this->app->storagePath().'/tmp', 'logo');
-        $file->moveTo($tmpFile);
-
         $manager = new ImageManager;
 
-        $encodedImage = $manager->make($tmpFile)->heighten(60, function ($constraint) {
+        $encodedImage = $manager->make($file->getStream())->heighten(60, function ($constraint) {
             $constraint->upsize();
         })->encode('png');
-        file_put_contents($tmpFile, $encodedImage);
 
-        $mount = new MountManager([
+        if (($path = $this->settings->get('logo_path')) && $this->uploadDir->has($path)) {
-            'source' => new Filesystem(new Local(pathinfo($tmpFile, PATHINFO_DIRNAME))),
+            $this->uploadDir->delete($path);
-            'target' => new Filesystem(new Local($this->app->publicPath().'/assets')),
-        ]);
-
-        if (($path = $this->settings->get('logo_path')) && $mount->has($file = "target://$path")) {
-            $mount->delete($file);
         }
 
         $uploadName = 'logo-'.Str::lower(Str::random(8)).'.png';
 
-        $mount->move('source://'.pathinfo($tmpFile, PATHINFO_BASENAME), "target://$uploadName");
+        $this->uploadDir->write($uploadName, $encodedImage);
 
         $this->settings->set('logo_path', $uploadName);
 

framework/core/src/Settings/SettingsServiceProvider.php

@@ -9,8 +9,15 @@
 
 namespace Flarum\Settings;
 
+use Flarum\Api\Controller\DeleteFaviconController;
+use Flarum\Api\Controller\DeleteLogoController;
+use Flarum\Api\Controller\UploadFaviconController;
+use Flarum\Api\Controller\UploadLogoController;
 use Flarum\Foundation\AbstractServiceProvider;
+use Illuminate\Contracts\Container\Container;
+use Illuminate\Contracts\Filesystem\Factory;
 use Illuminate\Database\ConnectionInterface;
+use League\Flysystem\FilesystemInterface;
 
 class SettingsServiceProvider extends AbstractServiceProvider
 {
@@ -28,5 +35,18 @@ class SettingsServiceProvider extends AbstractServiceProvider
         });
 
         $this->app->alias(SettingsRepositoryInterface::class, 'flarum.settings');
+
+        $assets = function (Container $app) {
+            return $app->make(Factory::class)->disk('flarum-assets')->getDriver();
+        };
+
+        $this->app->when([
+            DeleteFaviconController::class,
+            DeleteLogoController::class,
+            UploadFaviconController::class,
+            UploadLogoController::class,
+        ])
+            ->needs(FilesystemInterface::class)
+            ->give($assets);
     }
 }

framework/core/src/User/AvatarValidator.php

@@ -10,14 +10,76 @@
 namespace Flarum\User;
 
 use Flarum\Foundation\AbstractValidator;
+use Flarum\Foundation\ValidationException;
+use Psr\Http\Message\UploadedFileInterface;
+use Symfony\Component\Mime\MimeTypes;
 
 class AvatarValidator extends AbstractValidator
 {
-    protected $rules = [
+    /**
-        'avatar' => [
+     * Throw an exception if a model is not valid.
-            'required',
+     *
-            'mimes:jpeg,png,bmp,gif',
+     * @param array $attributes
-            'max:2048'
+     */
-        ]
+    public function assertValid(array $attributes)
-    ];
+    {
+        $this->assertFileRequired($attributes['avatar']);
+        $this->assertFileMimes($attributes['avatar']);
+        $this->assertFileSize($attributes['avatar']);
+    }
+
+    protected function assertFileRequired(UploadedFileInterface $file)
+    {
+        if ($file->getError() !== UPLOAD_ERR_OK) {
+            $this->raise('required');
+        }
+    }
+
+    protected function assertFileMimes(UploadedFileInterface $file)
+    {
+        $allowedTypes = $this->getAllowedTypes();
+
+        // Block PHP files masquerading as images
+        $phpExtensions = ['php', 'php3', 'php4', 'php5', 'phtml'];
+        $fileExtension = pathinfo($file->getClientFilename(), PATHINFO_EXTENSION);
+
+        if (in_array(trim(strtolower($fileExtension)), $phpExtensions)) {
+            $this->raise('mimes', [':values' => implode(', ', $allowedTypes)]);
+        }
+
+        $guessedExtension = MimeTypes::getDefault()->getExtensions($file->getClientMediaType())[0] ?? null;
+
+        if (! in_array($guessedExtension, $allowedTypes)) {
+            $this->raise('mimes', [':values' => implode(', ', $allowedTypes)]);
+        }
+    }
+
+    protected function assertFileSize(UploadedFileInterface $file)
+    {
+        $maxSize = $this->getMaxSize();
+
+        if ($file->getSize() / 1024 > $maxSize) {
+            $this->raise('max.file', [':max' => $maxSize]);
+        }
+    }
+
+    protected function raise($error, array $parameters = [])
+    {
+        $message = $this->translator->trans(
+            "validation.$error",
+            $parameters + [':attribute' => 'avatar']
+        );
+
+        throw new ValidationException(['avatar' => $message]);
+    }
+
+    protected function getMaxSize()
+    {
+        return 2048;
+    }
+
+    protected function getAllowedTypes()
+    {
+        return ['jpeg', 'png', 'bmp', 'gif'];
+    }
 }

framework/core/src/User/Command/UploadAvatarHandler.php

@@ -9,7 +9,6 @@
 
 namespace Flarum\User\Command;
 
-use Flarum\Foundation\Application;
 use Flarum\Foundation\DispatchEventsTrait;
 use Flarum\User\AssertPermissionTrait;
 use Flarum\User\AvatarUploader;
@@ -18,7 +17,6 @@ use Flarum\User\Event\AvatarSaving;
 use Flarum\User\UserRepository;
 use Illuminate\Contracts\Events\Dispatcher;
 use Intervention\Image\ImageManager;
-use Symfony\Component\HttpFoundation\File\UploadedFile;
 
 class UploadAvatarHandler
 {
@@ -30,11 +28,6 @@ class UploadAvatarHandler
      */
     protected $users;
 
-    /**
-     * @var Application
-     */
-    protected $app;
-
     /**
      * @var AvatarUploader
      */
@@ -48,15 +41,13 @@ class UploadAvatarHandler
     /**
      * @param Dispatcher $events
      * @param UserRepository $users
-     * @param Application $app
      * @param AvatarUploader $uploader
      * @param AvatarValidator $validator
      */
-    public function __construct(Dispatcher $events, UserRepository $users, Application $app, AvatarUploader $uploader, AvatarValidator $validator)
+    public function __construct(Dispatcher $events, UserRepository $users, AvatarUploader $uploader, AvatarValidator $validator)
     {
         $this->events = $events;
         $this->users = $users;
-        $this->app = $app;
         $this->uploader = $uploader;
         $this->validator = $validator;
     }
@@ -65,6 +56,7 @@ class UploadAvatarHandler
      * @param UploadAvatar $command
      * @return \Flarum\User\User
      * @throws \Flarum\User\Exception\PermissionDeniedException
+     * @throws \Flarum\Foundation\ValidationException
      */
     public function handle(UploadAvatar $command)
     {
@@ -76,24 +68,9 @@ class UploadAvatarHandler
             $this->assertCan($actor, 'edit', $user);
         }
 
-        $file = $command->file;
+        $this->validator->assertValid(['avatar' => $command->file]);
 
-        $tmpFile = tempnam($this->app->storagePath().'/tmp', 'avatar');
+        $image = (new ImageManager)->make($command->file->getStream());
-        $file->moveTo($tmpFile);
-
-        try {
-            $file = new UploadedFile(
-                $tmpFile,
-                $file->getClientFilename(),
-                $file->getClientMediaType(),
-                $file->getSize(),
-                $file->getError(),
-                true
-            );
-
-            $this->validator->assertValid(['avatar' => $file]);
-
-            $image = (new ImageManager)->make($tmpFile);
 
         $this->events->dispatch(
             new AvatarSaving($user, $actor, $image)
@@ -102,9 +79,6 @@ class UploadAvatarHandler
         $this->uploader->upload($user, $image);
 
         $user->save();
-        } finally {
-            @unlink($tmpFile);
-        }
 
         return $user;
     }