[COMPONENT][Attachment] Vinculate note information with attachment controllers

Various minor bug fixes

components/Attachment/Attachment.php

@@ -38,10 +38,10 @@ class Attachment extends Component
 {
     public function onAddRoute(RouteLoader $r): bool
     {
-        $r->connect('attachment_show', '/object/attachment/{id<\d+>}', [C\Attachment::class, 'attachment_show']);
+        $r->connect('note_attachment_show', '/object/note/{note_id<\d+>}/attachment/{attachment_id<\d+>}', [C\Attachment::class, 'attachmentShowWithNote']);
-        $r->connect('attachment_view', '/object/attachment/{id<\d+>}/view', [C\Attachment::class, 'attachment_view']);
+        $r->connect('note_attachment_view', '/object/note/{note_id<\d+>}/attachment/{attachment_id<\d+>}/view', [C\Attachment::class, 'attachmentViewWithNote']);
-        $r->connect('attachment_download', '/object/attachment/{id<\d+>}/download', [C\Attachment::class, 'attachment_download']);
+        $r->connect('note_attachment_download', '/object/note/{note_id<\d+>}/attachment/{attachment_id<\d+>}/download', [C\Attachment::class, 'attachmentDownloadWithNote']);
-        $r->connect('attachment_thumbnail', '/object/attachment/{id<\d+>}/thumbnail/{size<big|medium|small>}', [C\Attachment::class, 'attachment_thumbnail']);
+        $r->connect('note_attachment_thumbnail', '/object/note/{note_id<\d+>}/attachment/{attachment_id<\d+>}/thumbnail/{size<big|medium|small>}', [C\Attachment::class, 'attachmentThumbnailWithNote']);
         return Event::next;
     }
 

components/Attachment/Controller/Attachment.php

@@ -28,8 +28,7 @@ use App\Core\DB\DB;
 use App\Core\Event;
 use App\Core\GSFile;
 use function App\Core\I18n\_m;
-use App\Core\Log;
+use App\Entity\Note;
-use App\Core\Router\Router;
 use App\Util\Common;
 use App\Util\Exception\ClientException;
 use App\Util\Exception\NoSuchFileException;
@@ -46,22 +45,23 @@ class Attachment extends Controller
     /**
      * Generic function that handles getting a representation for an attachment
      */
-    private function attachment(int $id, callable $handle)
+    private function attachment(int $attachment_id, Note|int $note, callable $handle)
     {
-        if ($id <= 0) { // This should never happen coming from the router, but let's bail if it does
+        $attachment = DB::findOneBy('attachment', ['id' => $attachment_id]);
-            // @codeCoverageIgnoreStart
+        $note       = \is_int($note) ? Note::getById($note) : $note;
-            Log::critical("Attachment controller called with {$id}, which should not be possible");
+
-            throw new ClientException(_m('No such attachment.'), 404);
+        // Before anything, ensure proper scope
-        // @codeCoverageIgnoreEnd
+        if (!$note->isVisibleTo(Common::actor())) {
-        } else {
+            throw new ClientException(_m('You don\'t have permissions to view this attachment.'), 401);
-            $res = null;
+        }
-            if (Event::handle('AttachmentFileInfo', [$id, &$res]) != Event::stop) {
+
-                // If no one else claims this attachment, use the default representation
+        $res = null;
-                try {
+        if (Event::handle('AttachmentFileInfo', [$attachment, $note, &$res]) !== Event::stop) {
-                    $res = GSFile::getAttachmentFileInfo($id);
+            // If no one else claims this attachment, use the default representation
-                } catch (NoSuchFileException $e) {
+            try {
-                    // Continue below
+                $res = GSFile::getAttachmentFileInfo($attachment_id);
-                }
+            } catch (NoSuchFileException $e) {
+                // Continue below
             }
         }
 
@@ -73,6 +73,9 @@ class Attachment extends Controller
                 throw new ServerException('This attachment is not stored locally.');
             // @codeCoverageIgnoreEnd
             } else {
+                $res['attachment'] = $attachment;
+                $res['note']       = $note;
+                $res['title']      = $attachment->getBestTitle($note);
                 return $handle($res);
             }
         }
@@ -81,16 +84,17 @@ class Attachment extends Controller
     /**
      * The page where the attachment and it's info is shown
      */
-    public function attachment_show(Request $request, int $id)
+    public function attachmentShowWithNote(Request $request, int $note_id, int $attachment_id)
     {
         try {
-            $attachment = DB::findOneBy('attachment', ['id' => $id]);
+            return $this->attachment($attachment_id, $note_id, function ($res) use ($note_id, $attachment_id) {
-            return $this->attachment($id, function ($res) use ($id, $attachment) {
                 return [
                     '_template'        => '/cards/attachments/show.html.twig',
-                    'download'         => Router::url('attachment_download', ['id' => $id]),
+                    'download'         => $res['attachment']->getDownloadUrl(note: $note_id),
-                    'attachment'       => $attachment,
+                    'title'            => $res['title'],
-                    'right_panel_vars' => ['attachment_id' => $id],
+                    'attachment'       => $res['attachment'],
+                    'note'             => $res['note'],
+                    'right_panel_vars' => ['attachment_id' => $attachment_id],
                 ];
             });
         } catch (NotFoundException) {
@@ -101,27 +105,29 @@ class Attachment extends Controller
     /**
      * Display the attachment inline
      */
-    public function attachment_view(Request $request, int $id)
+    public function attachmentViewWithNote(Request $request, int $note_id, int $attachment_id)
     {
         return $this->attachment(
-            $id,
+            $attachment_id,
+            $note_id,
             fn (array $res) => GSFile::sendFile(
                 $res['filepath'],
                 $res['mimetype'],
-                GSFile::ensureFilenameWithProperExtension($res['filename'], $res['mimetype']) ?? $res['filename'],
+                GSFile::ensureFilenameWithProperExtension($res['title'], $res['mimetype']) ?? $res['filename'],
                 HeaderUtils::DISPOSITION_INLINE,
             ),
         );
     }
 
-    public function attachment_download(Request $request, int $id)
+    public function attachmentDownloadWithNote(Request $request, int $note_id, int $attachment_id)
     {
         return $this->attachment(
-            $id,
+            $attachment_id,
+            $note_id,
             fn (array $res) => GSFile::sendFile(
                 $res['filepath'],
                 $res['mimetype'],
-                GSFile::ensureFilenameWithProperExtension($res['filename'], $res['mimetype']) ?? $res['filename'],
+                GSFile::ensureFilenameWithProperExtension($res['title'], $res['mimetype']) ?? $res['filename'],
                 HeaderUtils::DISPOSITION_ATTACHMENT,
             ),
         );
@@ -130,16 +136,21 @@ class Attachment extends Controller
     /**
      * Controller to produce a thumbnail for a given attachment id
      *
-     * @param int $id Attachment ID
+     * @param int $attachment_id Attachment ID
      *
      * @throws \App\Util\Exception\DuplicateFoundException
      * @throws ClientException
      * @throws NotFoundException
      * @throws ServerException
      */
-    public function attachment_thumbnail(Request $request, int $id, string $size = 'small'): Response
+    public function attachmentThumbnailWithNote(Request $request, int $note_id, int $attachment_id, string $size = 'small'): Response
     {
-        $attachment = DB::findOneBy('attachment', ['id' => $id]);
+        // Before anything, ensure proper scope
+        if (!Note::getById($note_id)->isVisibleTo(Common::actor())) {
+            throw new ClientException(_m('You don\'t have permissions to view this thumbnail.'), 401);
+        }
+
+        $attachment = DB::findOneBy('attachment', ['id' => $attachment_id]);
 
         $crop = Common::config('thumbnail', 'smart_crop');
 

components/Attachment/Entity/Attachment.php

@@ -347,9 +347,19 @@ class Attachment extends Entity
         return \is_null($filename) ? null : Common::config('attachments', 'dir') . \DIRECTORY_SEPARATOR . $filename;
     }
 
-    public function getUrl(int $type = Router::ABSOLUTE_URL): string
+    public function getUrl(Note|int $note, int $type = Router::ABSOLUTE_URL): string
     {
-        return Router::url(id: 'attachment_view', args: ['id' => $this->getId()], type: $type);
+        return Router::url(id: 'note_attachment_view', args: ['note_id' => \is_int($note) ? $note : $note->getId(), 'attachment_id' => $this->getId()], type: $type);
+    }
+
+    public function getShowUrl(Note|int $note, int $type = Router::ABSOLUTE_URL): string
+    {
+        return Router::url(id: 'note_attachment_show', args: ['note_id' => \is_int($note) ? $note : $note->getId(), 'attachment_id' => $this->getId()], type: $type);
+    }
+
+    public function getDownloadUrl(Note|int $note, int $type = Router::ABSOLUTE_URL): string
+    {
+        return Router::url(id: 'note_attachment_download', args: ['note_id' => \is_int($note) ? $note : $note->getId(), 'attachment_id' => $this->getId()], type: $type);
     }
 
     /**
@@ -364,9 +374,9 @@ class Attachment extends Entity
         return AttachmentThumbnail::getOrCreate(attachment: $this, size: $size, crop: $crop);
     }
 
-    public function getThumbnailUrl(?string $size = null)
+    public function getThumbnailUrl(Note|int $note, ?string $size = null)
     {
-        return Router::url('attachment_thumbnail', ['id' => $this->getId(), 'size' => $size ?? Common::config('thumbnail', 'default_size')]);
+        return Router::url('note_attachment_thumbnail', ['note_id' => \is_int($note) ? $note : $note->getId(), 'attachment_id' => $this->getId(), 'size' => $size ?? Common::config('thumbnail', 'default_size')]);
     }
 
     public static function schemaDef(): array

plugins/ActivityPub/Util/Model/Note.php

@@ -376,7 +376,7 @@ class Note extends Model
             $attr['attachment'][] = [
                 'type'      => 'Document',
                 'mediaType' => $attachment->getMimetype(),
-                'url'       => $attachment->getUrl(Router::ABSOLUTE_URL),
+                'url'       => $attachment->getUrl(note: $object, type: Router::ABSOLUTE_URL),
                 'name'      => AttachmentToNote::getByPK(['attachment_id' => $attachment->getId(), 'note_id' => $object->getId()])->getTitle(),
                 'width'     => $attachment->getWidth(),
                 'height'    => $attachment->getHeight(),

plugins/AttachmentShowRelated/AttachmentShowRelated.php

@@ -33,7 +33,7 @@ class AttachmentShowRelated extends Plugin
 {
     public function onAppendRightPanelBlock($vars, $request, &$res): bool
     {
-        if ($vars['path'] === 'attachment_show') {
+        if ($vars['path'] === 'note_attachment_show') {
             $related_notes = DB::dql('select n from attachment_to_note an '
         . 'join note n with n.id = an.note_id '
         . 'where an.attachment_id = :attachment_id', ['attachment_id' => $vars['vars']['attachment_id']], );

plugins/AudioEncoder/AudioEncoder.php

@@ -101,6 +101,7 @@ class AudioEncoder extends Plugin
             [
                 'attachment' => $vars['attachment'],
                 'note'       => $vars['note'],
+                'title'      => $vars['title'],
             ],
         );
         return Event::stop;

plugins/AudioEncoder/templates/audioEncoder/audioEncoderView.html.twig

@@ -1,13 +1,13 @@
 {% if attachment.getFilename() is not null %}
 <div>
   <figure>
-    <audio class="u-audio" src="{{ attachment.getUrl() }}" controls>
+    <audio class="u-audio" src="{{ attachment.getUrl(note) }}" controls>
     </audio>
     <figcaption>
       {% if attachment.getFilename() is not null %}
-        <a href="{{ path('attachment_show', {'id': attachment.getId()}) }}">{{ attachment.getBestTitle(note) }}</a>
+        <a href="{{ attachment.getShowUrl(note) }}">{{ title }}</a>
       {% else %}
-        {{ attachment.getBestTitle(note) }}
+        {{ title }}
       {% endif %}
     </figcaption>
   </figure>

plugins/Embed/Embed.php

@@ -167,7 +167,7 @@ class Embed extends Plugin
 
         $res[] = Formatting::twigRenderFile(
             'embed/embedView.html.twig',
-            ['embed' => $embed, 'attributes' => $attributes, 'link' => $link],
+            ['embed' => $embed, 'attributes' => $attributes, 'link' => $link, 'note' => $vars['note']],
         );
 
         return Event::stop;

plugins/Embed/templates/embed/embedView.html.twig

@@ -7,12 +7,13 @@
 
     {% if attributes['has_attachment'] != false %}
         {% set thumbnail_parameters = {
-            'id': embed.getAttachmentId(),
+            'note_id': note.getId(),
+            'attachment_id': embed.getAttachmentId(),
             'size': 'medium'
         } %}
         <img alt="{{embed.getTitle() | escape}}" class="{{attributes['class']}}"
              width="{{attributes['width']}}" height="{{attributes['height']}}"
-             src="{{ path('attachment_thumbnail', thumbnail_parameters) }}" />
+             src="{{ path('note_attachment_thumbnail', thumbnail_parameters) }}" />
     {% endif %}
 
     <div class="p-summary">

plugins/ImageEncoder/ImageEncoder.php

@@ -194,6 +194,7 @@ class ImageEncoder extends Plugin
             [
                 'attachment' => $vars['attachment'],
                 'note'       => $vars['note'],
+                'title'      => $vars['title'],
                 'thumbnail'  => $thumbnail,
             ],
         );

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

@@ -1,14 +1,14 @@
 <figure>
     <img class="u-photo"
-         alt="{{ attachment.getBestTitle(note) }}"
+         alt="{{ title }}"
-         src="{{ attachment.getThumbnailUrl() }}"
+         src="{{ attachment.getThumbnailUrl(note) }}"
          width="{{ thumbnail.getWidth() }}"
          height="{{ thumbnail.getHeight() }}">
         <figcaption>
             {% if attachment.getFilename() is not null %}
-                <a href="{{ path('attachment_show', {'id': attachment.getId()}) }}">{{ attachment.getBestTitle(note) }}</a>
+                <a href="{{ attachment.getShowUrl(note) }}">{{ title }}</a>
             {% else %}
-                {{ attachment.getBestTitle(note) }}
+                {{ title }}
             {% endif %}
         </figcaption>
 </figure>

plugins/VideoEncoder/VideoEncoder.php

@@ -150,6 +150,7 @@ class VideoEncoder extends Plugin
             [
                 'attachment' => $vars['attachment'],
                 'note'       => $vars['note'],
+                'title'      => $vars['title'],
             ],
         );
         return Event::stop;

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

@@ -7,15 +7,18 @@
                 {% else %}
                     class="u-audio"
                 {% endif %}
-                src="{{ attachment.getUrl() }}" controls
+                src="{{ attachment.getUrl(note) }}" controls
                 {% if attachment.getWidth() is not null %}
-                    poster="{{ attachment.getThumbnailUrl('medium')}}"
+                    poster="{{ attachment.getThumbnailUrl(note, 'medium')}}"
                 {% endif %}
             >
             </video>
             <figcaption>
-                    <a href="{{ path('attachment_show', {'id': attachment.getId()}) }}">{{ attachment.getBestTitle(note) }}</a>
+                {% if attachment.getFilename() is not null %}
-            </figcaption>
+                    <a href="{{ attachment.getShowUrl(note) }}">{{ title }}</a>
+                {% else %}
+                    {{ title }}
+                {% endif %}            </figcaption>
         </figure>
     </div>
 {% else %}

src/Core/GSFile.php

@@ -292,7 +292,7 @@ class GSFile
      * @param null|string $ext      Extension we believe to be best
      * @param bool        $force    Should we force the extension we believe to be best? Defaults to false
      *
-     * @return null|string the most appropriate filename or null if we deem it imposible
+     * @return null|string the most appropriate filename or null if we deem it impossible
      */
     public static function ensureFilenameWithProperExtension(string $title, string $mimetype, ?string $ext = null, bool $force = false): string|null
     {

src/Entity/Note.php

@@ -380,7 +380,7 @@ class Note extends Entity
                 return false;
             case VisibilityScope::GROUP:
                 // Only for the group to see
-                return DB::dql(
+                return !\is_null($actor) && DB::dql(
                     <<<'EOF'
                             select m from group_member m
                             join group_inbox i with m.group_id = i.group_id
@@ -392,7 +392,7 @@ class Note extends Entity
             case VisibilityScope::COLLECTION:
             case VisibilityScope::MESSAGE:
                 // Only for the collection to see
-                return in_array($actor->getId(), $this->getNotificationTargetIds());
+                return !\is_null($actor) && in_array($actor->getId(), $this->getNotificationTargetIds());
             default:
                 Log::error("Unknown scope found: {$this->getScope()}.");
         }

templates/cards/attachments/show.html.twig

@@ -4,7 +4,7 @@
     <div class="page">
         <section class="section-widget section-padding">
 
-            {% include '/cards/attachments/view.html.twig' with {'attachment': attachment, 'note': null} only %}
+            {% include '/cards/attachments/view.html.twig' with {'attachment': attachment, 'note': note, 'title': title} only %}
 
             <a class="section-widget-button-like" href="{{ download }}"> {{ 'Download link' | trans }}</a>
         </section>

templates/cards/attachments/view.html.twig

@@ -1,5 +1,5 @@
 {% set handled = false %}
-{% for block in handle_event('ViewAttachment', {'attachment': attachment, 'note': note}) %}
+{% for block in handle_event('ViewAttachment', {'attachment': attachment, 'note': note, 'title': title}) %}
     {% set handled = true %}
     <div class="note-attachments-unit">
         {{ block | raw }}
@@ -7,6 +7,10 @@
 {% endfor %}
 {% if not handled %}
     <div class="note-attachments-unit">
-        <i> <a href="{{ path('attachment_show', {'id': attachment.getId()}) }}">{{ attachment.getBestTitle(note) }}</a> </i>
+        {% if attachment.getFilename() is not null %}
+            <a href="{{ attachment.getShowUrl(note) }}">{{ title }}</a>
+        {% else %}
+            {{ title }}
+        {% endif %}
     </div>
 {% endif %}

templates/cards/note/view.html.twig

@@ -49,7 +49,7 @@
         {% if note.getAttachments() is not empty %}
             <section class="note-attachments" tabindex="0" title="{{ 'Note attachments.' | trans }}">
                 {% for attachment in note.getAttachments() %}
-                    {% include '/cards/attachments/view.html.twig' with {'attachment': attachment, 'note': note} only%}
+                    {% include '/cards/attachments/view.html.twig' with {'attachment': attachment, 'note': note, 'title': attachment.getBestTitle(note)} only%}
                 {% endfor %}
             </section>
         {% endif %}