From c4dacd7626d79921a8acdb8fab2467233c3961a1 Mon Sep 17 00:00:00 2001 From: Diogo Peralta Cordeiro Date: Mon, 27 Dec 2021 02:47:04 +0000 Subject: [PATCH] [COMPONENT][Attachment] Vinculate note information with attachment controllers Various minor bug fixes --- components/Attachment/Attachment.php | 8 +- .../Attachment/Controller/Attachment.php | 75 +++++++++++-------- components/Attachment/Entity/Attachment.php | 18 ++++- plugins/ActivityPub/Util/Model/Note.php | 2 +- .../AttachmentShowRelated.php | 2 +- plugins/AudioEncoder/AudioEncoder.php | 1 + .../audioEncoder/audioEncoderView.html.twig | 6 +- plugins/Embed/Embed.php | 2 +- .../Embed/templates/embed/embedView.html.twig | 5 +- plugins/ImageEncoder/ImageEncoder.php | 1 + .../imageEncoder/imageEncoderView.html.twig | 8 +- plugins/VideoEncoder/VideoEncoder.php | 1 + .../videoEncoder/videoEncoderView.html.twig | 11 ++- src/Core/GSFile.php | 2 +- src/Entity/Note.php | 4 +- templates/cards/attachments/show.html.twig | 2 +- templates/cards/attachments/view.html.twig | 8 +- templates/cards/note/view.html.twig | 2 +- 18 files changed, 95 insertions(+), 63 deletions(-) diff --git a/components/Attachment/Attachment.php b/components/Attachment/Attachment.php index 3688183419..1f39e397f2 100644 --- a/components/Attachment/Attachment.php +++ b/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('attachment_view', '/object/attachment/{id<\d+>}/view', [C\Attachment::class, 'attachment_view']); - $r->connect('attachment_download', '/object/attachment/{id<\d+>}/download', [C\Attachment::class, 'attachment_download']); - $r->connect('attachment_thumbnail', '/object/attachment/{id<\d+>}/thumbnail/{size}', [C\Attachment::class, 'attachment_thumbnail']); + $r->connect('note_attachment_show', '/object/note/{note_id<\d+>}/attachment/{attachment_id<\d+>}', [C\Attachment::class, 'attachmentShowWithNote']); + $r->connect('note_attachment_view', '/object/note/{note_id<\d+>}/attachment/{attachment_id<\d+>}/view', [C\Attachment::class, 'attachmentViewWithNote']); + $r->connect('note_attachment_download', '/object/note/{note_id<\d+>}/attachment/{attachment_id<\d+>}/download', [C\Attachment::class, 'attachmentDownloadWithNote']); + $r->connect('note_attachment_thumbnail', '/object/note/{note_id<\d+>}/attachment/{attachment_id<\d+>}/thumbnail/{size}', [C\Attachment::class, 'attachmentThumbnailWithNote']); return Event::next; } diff --git a/components/Attachment/Controller/Attachment.php b/components/Attachment/Controller/Attachment.php index f5ea6e3908..934388d26f 100644 --- a/components/Attachment/Controller/Attachment.php +++ b/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\Core\Router\Router; +use App\Entity\Note; 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 - // @codeCoverageIgnoreStart - Log::critical("Attachment controller called with {$id}, which should not be possible"); - throw new ClientException(_m('No such attachment.'), 404); - // @codeCoverageIgnoreEnd - } else { - $res = null; - if (Event::handle('AttachmentFileInfo', [$id, &$res]) != Event::stop) { - // If no one else claims this attachment, use the default representation - try { - $res = GSFile::getAttachmentFileInfo($id); - } catch (NoSuchFileException $e) { - // Continue below - } + $attachment = DB::findOneBy('attachment', ['id' => $attachment_id]); + $note = \is_int($note) ? Note::getById($note) : $note; + + // Before anything, ensure proper scope + if (!$note->isVisibleTo(Common::actor())) { + throw new ClientException(_m('You don\'t have permissions to view this attachment.'), 401); + } + + $res = null; + if (Event::handle('AttachmentFileInfo', [$attachment, $note, &$res]) !== Event::stop) { + // If no one else claims this attachment, use the default representation + try { + $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($id, function ($res) use ($id, $attachment) { + return $this->attachment($attachment_id, $note_id, function ($res) use ($note_id, $attachment_id) { return [ '_template' => '/cards/attachments/show.html.twig', - 'download' => Router::url('attachment_download', ['id' => $id]), - 'attachment' => $attachment, - 'right_panel_vars' => ['attachment_id' => $id], + 'download' => $res['attachment']->getDownloadUrl(note: $note_id), + 'title' => $res['title'], + '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'); diff --git a/components/Attachment/Entity/Attachment.php b/components/Attachment/Entity/Attachment.php index dea3e1fde6..dcf4c687c7 100644 --- a/components/Attachment/Entity/Attachment.php +++ b/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 diff --git a/plugins/ActivityPub/Util/Model/Note.php b/plugins/ActivityPub/Util/Model/Note.php index 4c2043e8ea..f620dd414b 100644 --- a/plugins/ActivityPub/Util/Model/Note.php +++ b/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(), diff --git a/plugins/AttachmentShowRelated/AttachmentShowRelated.php b/plugins/AttachmentShowRelated/AttachmentShowRelated.php index df6f9883a5..1d75739773 100644 --- a/plugins/AttachmentShowRelated/AttachmentShowRelated.php +++ b/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']], ); diff --git a/plugins/AudioEncoder/AudioEncoder.php b/plugins/AudioEncoder/AudioEncoder.php index 67e4e014c7..8b57d16a2b 100644 --- a/plugins/AudioEncoder/AudioEncoder.php +++ b/plugins/AudioEncoder/AudioEncoder.php @@ -101,6 +101,7 @@ class AudioEncoder extends Plugin [ 'attachment' => $vars['attachment'], 'note' => $vars['note'], + 'title' => $vars['title'], ], ); return Event::stop; diff --git a/plugins/AudioEncoder/templates/audioEncoder/audioEncoderView.html.twig b/plugins/AudioEncoder/templates/audioEncoder/audioEncoderView.html.twig index 23c9e188aa..24ad5ae976 100644 --- a/plugins/AudioEncoder/templates/audioEncoder/audioEncoderView.html.twig +++ b/plugins/AudioEncoder/templates/audioEncoder/audioEncoderView.html.twig @@ -1,13 +1,13 @@ {% if attachment.getFilename() is not null %}
-
diff --git a/plugins/Embed/Embed.php b/plugins/Embed/Embed.php index 5af8046518..adbcc4065f 100644 --- a/plugins/Embed/Embed.php +++ b/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; diff --git a/plugins/Embed/templates/embed/embedView.html.twig b/plugins/Embed/templates/embed/embedView.html.twig index 624b02b93d..60a8079e08 100644 --- a/plugins/Embed/templates/embed/embedView.html.twig +++ b/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' } %} {{embed.getTitle() | escape}} + src="{{ path('note_attachment_thumbnail', thumbnail_parameters) }}" /> {% endif %}
diff --git a/plugins/ImageEncoder/ImageEncoder.php b/plugins/ImageEncoder/ImageEncoder.php index d3eaf2b658..2927fa73bf 100644 --- a/plugins/ImageEncoder/ImageEncoder.php +++ b/plugins/ImageEncoder/ImageEncoder.php @@ -194,6 +194,7 @@ class ImageEncoder extends Plugin [ 'attachment' => $vars['attachment'], 'note' => $vars['note'], + 'title' => $vars['title'], 'thumbnail' => $thumbnail, ], ); diff --git a/plugins/ImageEncoder/templates/imageEncoder/imageEncoderView.html.twig b/plugins/ImageEncoder/templates/imageEncoder/imageEncoderView.html.twig index 9922dbc08b..2f78bc9d34 100644 --- a/plugins/ImageEncoder/templates/imageEncoder/imageEncoderView.html.twig +++ b/plugins/ImageEncoder/templates/imageEncoder/imageEncoderView.html.twig @@ -1,14 +1,14 @@
{{ attachment.getBestTitle(note) }}
{% if attachment.getFilename() is not null %} - {{ attachment.getBestTitle(note) }} + {{ title }} {% else %} - {{ attachment.getBestTitle(note) }} + {{ title }} {% endif %}
diff --git a/plugins/VideoEncoder/VideoEncoder.php b/plugins/VideoEncoder/VideoEncoder.php index cf0b3d0432..adffacc31b 100644 --- a/plugins/VideoEncoder/VideoEncoder.php +++ b/plugins/VideoEncoder/VideoEncoder.php @@ -150,6 +150,7 @@ class VideoEncoder extends Plugin [ 'attachment' => $vars['attachment'], 'note' => $vars['note'], + 'title' => $vars['title'], ], ); return Event::stop; diff --git a/plugins/VideoEncoder/templates/videoEncoder/videoEncoderView.html.twig b/plugins/VideoEncoder/templates/videoEncoder/videoEncoderView.html.twig index 49f632cdb4..787aceed49 100644 --- a/plugins/VideoEncoder/templates/videoEncoder/videoEncoderView.html.twig +++ b/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 %} >
- {{ attachment.getBestTitle(note) }} -
+ {% if attachment.getFilename() is not null %} + {{ title }} + {% else %} + {{ title }} + {% endif %}
{% else %} diff --git a/src/Core/GSFile.php b/src/Core/GSFile.php index c0116cedf2..eea67c3f73 100644 --- a/src/Core/GSFile.php +++ b/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 { diff --git a/src/Entity/Note.php b/src/Entity/Note.php index 78232f0ae1..978f008230 100644 --- a/src/Entity/Note.php +++ b/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()}."); } diff --git a/templates/cards/attachments/show.html.twig b/templates/cards/attachments/show.html.twig index 1626c90735..95f63b04bb 100644 --- a/templates/cards/attachments/show.html.twig +++ b/templates/cards/attachments/show.html.twig @@ -4,7 +4,7 @@
- {% 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 %} {{ 'Download link' | trans }}
diff --git a/templates/cards/attachments/view.html.twig b/templates/cards/attachments/view.html.twig index a4858a918c..ffbe764cc0 100644 --- a/templates/cards/attachments/view.html.twig +++ b/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 %}
{{ block | raw }} @@ -7,6 +7,10 @@ {% endfor %} {% if not handled %}
- {{ attachment.getBestTitle(note) }} + {% if attachment.getFilename() is not null %} + {{ title }} + {% else %} + {{ title }} + {% endif %}
{% endif %} diff --git a/templates/cards/note/view.html.twig b/templates/cards/note/view.html.twig index 2762e0f65d..73c5341270 100644 --- a/templates/cards/note/view.html.twig +++ b/templates/cards/note/view.html.twig @@ -49,7 +49,7 @@ {% if note.getAttachments() is not empty %}
{% 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 %}
{% endif %}