[Media] EncoderPlugins should handle the views that concern them

Ensure the intended filetypes and mimetypes during Vips conversions (part 2)
Sanitize Attachments instead of Validate (part 2)
Various bug fixes

plugins/Embed/Embed.php

@@ -46,7 +46,6 @@ use App\Core\Modules\Plugin;
 use App\Core\Router\RouteLoader;
 use App\Core\Router\Router;
 use App\Entity\Attachment;
-use App\Entity\AttachmentThumbnail;
 use App\Util\Common;
 use App\Util\Exception\DuplicateFoundException;
 use App\Util\Exception\NotFoundException;
@@ -80,9 +79,10 @@ class Embed extends Plugin
      *
      * @param $m URLMapper the router that was initialized.
      *
-     * @return bool true if successful, the exception object if it isn't.
      * @throws Exception
      *
+     * @return bool true if successful, the exception object if it isn't.
+     *
      */
     public function onAddRoute(RouteLoader $m): bool
     {
@@ -112,7 +112,7 @@ class Embed extends Plugin
                         'type'  => "application/{$format}+oembed",
                         'href'  => Router::url('embed', ['format' => $format, 'url' => $url]),
                         'title' => 'oEmbed',
-                    ],];
+                    ], ];
             }
         }
         return Event::next;
@@ -182,15 +182,19 @@ class Embed extends Plugin
     }
 
     /**
-     * Show this attachment enhanced with the corresponing Embed data, if available
+     * Show this attachment enhanced with the corresponding Embed data, if available
+     * @param array $vars
+     * @param array $res
+     * @return bool
      */
-    public function onShowAttachment(Attachment $attachment, array &$res)
+    public function onViewRemoteAttachment(array $vars, array &$res): bool
     {
+        $attachment = $vars['attachment'];
         try {
             $embed = Cache::get('attachment-embed-' . $attachment->getId(),
-                fn() => DB::findOneBy('attachment_embed', ['attachment_id' => $attachment->getId()]));
+                fn () => DB::findOneBy('attachment_embed', ['attachment_id' => $attachment->getId()]));
         } catch (DuplicateFoundException $e) {
-            Log::waring($e);
+            Log::warning($e);
             return Event::next;
         } catch (NotFoundException) {
             return Event::next;
@@ -242,13 +246,14 @@ END, ['embed' => $embed, 'attributes' => $attributes, 'attachment' => $attachmen
     }
 
     /**
-     * @return bool false on no check made, provider name on success
-     * @return string|false on no check made, provider name on success
-     *
      * @throws ServerException if check is made but fails
      *
+     * @return bool         false on no check made, provider name on success
+     * @return false|string on no check made, provider name on success
+     *
+     *
      */
-    protected function checkAllowlist(string $url): string|bool
+    protected function checkAllowlist(string $url): string | bool
     {
         if ($this->check_allowlist ?? false) {
             return false;   // indicates "no check made"
@@ -270,9 +275,9 @@ END, ['embed' => $embed, 'attributes' => $attributes, 'attachment' => $attachmen
      * reliable enough for our purposes.
      *
      * @param string     $url
-     * @param array|null $headers - if we already made a request
+     * @param null|array $headers - if we already made a request
      *
-     * @return int|null the file size if it succeeds, false otherwise.
+     * @return null|int the file size if it succeeds, false otherwise.
      */
     private function getRemoteFileSize(string $url, ?array $headers = null): ?int
     {
@@ -354,10 +359,12 @@ END, ['embed' => $embed, 'attributes' => $attributes, 'attachment' => $attachmen
      *
      * @param Attachment $attachment
      * @param string     $media_url  URL for the actual media representation
-     * @return array|bool
+     *
      * @throws Exception
+     *
+     * @return array|bool
      */
-    protected function fetchValidateWriteRemoteImage(Attachment $attachment, string $media_url): array|bool
+    protected function fetchValidateWriteRemoteImage(Attachment $attachment, string $media_url): array | bool
     {
         if ($attachment->hasFilename() && file_exists($attachment->getPath())) {
             throw new AlreadyFulfilledException(_m('A thumbnail seems to already exist for remote file with id=={id}', ['id' => $attachment->getId()]));
@@ -421,6 +428,7 @@ END, ['embed' => $embed, 'attributes' => $attributes, 'attachment' => $attachmen
      *
      * @param string     $url
      * @param Attachment $attachment
+     *
      * @return array
      */
     public function getEmbed(string $url, Attachment $attachment): array
@@ -439,7 +447,7 @@ END, ['embed' => $embed, 'attributes' => $attributes, 'attachment' => $attachmen
             $metadata['provider_url']  = $info->providerUrl;
 
             if (!is_null($info->image)) {
-                $image_url = (string)$info->image;
+                $image_url = (string) $info->image;
 
                 if (Formatting::startsWith($image_url, 'data')) {
                     // Inline image

plugins/ImageEncoder/ImageEncoder.php

@@ -21,17 +21,18 @@ namespace Plugin\ImageEncoder;
 
 use App\Core\Event;
 use App\Core\GSFile;
-use App\Util\Exception\TemporaryFileException;
-use SplFileInfo;
 use function App\Core\I18n\_m;
 use App\Core\Log;
 use App\Core\Modules\Plugin;
 use App\Entity\Attachment;
 use App\Entity\AttachmentThumbnail;
 use App\Util\Common;
+use App\Util\Exception\TemporaryFileException;
+use App\Util\Formatting;
 use App\Util\TemporaryFile;
 use Exception;
 use Jcupitt\Vips;
+use SplFileInfo;
 
 /**
  * Create thumbnails and validate image attachments
@@ -56,7 +57,8 @@ class ImageEncoder extends Plugin
     }
 
     /**
-     * Encodes the image to self::preferredType() format ensuring it's valid.
+     * Re-encodes the image ensuring it's valid.
+     * Also ensures that the image is not greater than the max width and height configured.
      *
      * @param SplFileInfo $file
      * @param null|string $mimetype in/out
@@ -69,7 +71,7 @@ class ImageEncoder extends Plugin
      *
      * @return bool
      */
-    public function onAttachmentValidation(SplFileInfo &$file, ?string &$mimetype, ?string &$title, ?int &$width, ?int &$height): bool
+    public function onAttachmentSanitization(SplFileInfo &$file, ?string &$mimetype, ?string &$title, ?int &$width, ?int &$height): bool
     {
         $original_mimetype = $mimetype;
         if (GSFile::mimetypeMajor($original_mimetype) != 'image') {
@@ -77,13 +79,18 @@ class ImageEncoder extends Plugin
             return Event::next;
         }
 
-        $type      = self::preferredType();
+        // Try to maintain original mimetype extension, otherwise default to preferred.
-        $extension = image_type_to_extension($type, include_dot: true);
+        $extension = image_type_to_extension($this->preferredType(), include_dot: true);
-        // If title seems to be a filename with an extension
+        GSFile::titleToFilename(
-        if (preg_match('/\.[a-z0-9]/i', $title) === 1) {
+            title: $title,
-            $title = substr($title, 0, strrpos($title, '.')) . $extension;
+            mimetype: $original_mimetype,
-        }
+            ext: $extension,
+            force: false
+        );
+
         // TemporaryFile handles deleting the file if some error occurs
+        // IMPORTANT: We have to specify the extension for the temporary file
+        // in order to have a format conversion
         $temp = new TemporaryFile(['prefix' => 'image', 'suffix' => $extension]);
 
         $image  = Vips\Image::newFromFile($file->getRealPath(), ['access' => 'sequential']);
@@ -92,10 +99,33 @@ class ImageEncoder extends Plugin
         $image  = $image->crop(0, 0, $width, $height);
         $image->writeToFile($temp->getRealPath());
 
-        $filesize = $temp->getSize();
+        // Replace original file with the sanitized one
+        $temp->commit($file->getRealPath());
 
-        Event::handle('EnforceQuota', [$filesize]);
+        return Event::stop;
+    }
 
+    /**
+     * @param $event_map
+     *
+     * @return bool
+     */
+    public function onResizerAvailable(&$event_map): bool
+    {
+        $event_map['image'] = 'ResizeImagePath';
+        return Event::next;
+    }
+
+    /**
+     * Generates the view for attachments of type Image
+     *
+     * @param array $vars
+     * @param array $res
+     * @return bool
+     */
+    public function onViewAttachmentImage(array $vars, array &$res): bool
+    {
+        $res[] = Formatting::twigRenderFile('imageEncoder/imageEncoderView.html.twig', ['attachment' => $vars['attachment'], 'thumbnail_parameters' => $vars['thumbnail_parameters']]);
         return Event::stop;
     }
 
@@ -120,7 +150,7 @@ class ImageEncoder extends Plugin
      * @return bool
      *
      */
-    public function onResizeImagePath(string $source, string $destination, int &$width, int &$height, bool $smart_crop, ?string &$mimetype)
+    public function onResizeImagePath(string $source, ?TemporaryFile &$destination, int &$width, int &$height, bool $smart_crop, ?string &$mimetype): bool
     {
         $old_limit = ini_set('memory_limit', Common::config('attachments', 'memory_limit'));
         try {
@@ -132,8 +162,13 @@ class ImageEncoder extends Plugin
                 throw new Exception(_m('Unknown file type'));
             }
 
-            if ($source === $destination) {
+            if (is_null($destination)) {
-                @unlink($destination);
+                // IMPORTANT: We have to specify the extension for the temporary file
+                // in order to have a format conversion
+                $ext         = image_type_to_extension($this->preferredType(), include_dot: true);
+                $destination = new TemporaryFile(['prefix' => 'gs-thumbnail', 'suffix' => $ext]);
+            } elseif ($source === $destination->getRealPath()) {
+                @unlink($destination->getRealPath());
             }
 
             $type     = self::preferredType();
@@ -146,7 +181,7 @@ class ImageEncoder extends Plugin
             $width  = $image->width;
             $height = $image->height;
 
-            $image->writeToFile($destination);
+            $image->writeToFile($destination->getRealPath());
             unset($image);
         } finally {
             ini_set('memory_limit', $old_limit); // Restore the old memory limit

plugins/ImageEncoder/templates/imageEncoder/imageEncoderView.html.twig

@@ -0,0 +1,8 @@
+<div>
+    <figure>
+        <img src="{{ path('attachment_thumbnail', thumbnail_parameters) }}" alt="{{ attachment.getTitle() }}">
+        <figcaption><a
+                href="{{ path('attachment_show', {'id': attachment.getId()}) }}">{{ attachment.getTitle() }}</a>
+        </figcaption>
+    </figure>
+</div>

plugins/VideoEncoder/VideoEncoder.php

@@ -30,7 +30,9 @@
 
 namespace Plugin\VideoEncoder;
 
+use App\Core\Event;
 use App\Core\Modules\Plugin;
+use App\Util\Formatting;
 
 class VideoEncoder extends Plugin
 {
@@ -55,6 +57,19 @@ class VideoEncoder extends Plugin
         return true;
     }
 
+    /**
+     * Generates the view for attachments of type Video
+     *
+     * @param array $vars
+     * @param array $res
+     * @return bool
+     */
+    public function onViewAttachmentVideo(array $vars, array &$res): bool
+    {
+        $res[] = Formatting::twigRenderFile('videoEncoder/videoEncoderView.html.twig', ['attachment' => $vars['attachment'], 'thumbnail_parameters' => $vars['thumbnail_parameters']]);
+        return Event::stop;
+    }
+
     /**
      * High quality GIF conversion.
      *

plugins/VideoEncoder/templates/videoEncoder/videoEncoderView.html.twig

@@ -0,0 +1,9 @@
+<div>
+    <figure>
+        <video src="{{ path('attachment_view', {'id': attachment.getId()}) }}" controls poster="{{ path('attachment_thumbnail', thumbnail_parameters) }}">
+        </video>
+        <figcaption><a
+                href="{{ path('attachment_show', {'id': attachment.getId()}) }}">{{ attachment.getTitle() }}</a>
+        </figcaption>
+    </figure>
+</div>

templates/attachments/view.html.twig

@@ -1,28 +1,17 @@
 {% set thumbnail_parameters = {'id': attachment.getId(), 'w': config('thumbnail','width'), 'h': config('thumbnail','height')} %}
-{% if attachment.mimetype starts with 'image/' %}
+{% if attachment.getIsLocal() %}
+    {% set handled = false %}
+    {% for block in handle_event('ViewAttachment' ~ attachment.getMimetypeMajor() | capitalize , {'attachment': attachment, 'thumbnail_parameters': thumbnail_parameters}) %}
+        {% set handled = true %}
+        {{ block | raw }}
+    {% endfor %}
+    {% if not handled %}
         <div>
-        <figure>
-            <img src="{{ path('attachment_thumbnail', thumbnail_parameters) }}" alt="{{ attachment.getTitle() }}">
-            <figcaption><a
-                        href="{{ path('attachment_show', {'id': attachment.getId()}) }}">{{ attachment.getTitle() }}</a>
-            </figcaption>
-        </figure>
-    </div>
-{% elseif attachment.mimetype starts with 'video/' %}
-    <div>
-        <video src="{{ path('attachment_view', {'id': attachment.getId()}) }}" controls
-               poster="{{ path('attachment_thumbnail') }}, thumbnail_parameters">
             <i> <a href="{{ path('attachment_show', {'id': attachment.getId()}) }}">{{ attachment.getTitle() }}</a> </i>
-        </video>
         </div>
+    {% endif %}
 {% else %}
-    {% for show in handle_event('ShowAttachment', attachment) %}
+    {% for block in handle_event('ViewRemoteAttachment', {'attachment': attachment, 'thumbnail_parameters': thumbnail_parameters}) %}
-        <div>
+        {{ block | raw }}
-            {{ show | raw }}
-        </div>
-    {% else %}
-        <div>
-            <i> <a href="{{ path('attachment_show', {'id': attachment.getId()}) }}">{{ attachment.getTitle() }}</a> </i>
-        </div>
     {% endfor %}
 {% endif %}