IBX-12042: Deprecated filename-less download route in favor of filename-validated variant

src/bundle/Core/Resources/config/routing/internal.yml

@@ -33,12 +33,21 @@ ibexa.content.preview.default:
 ibexa.user_hash:
     path: /_fos_user_context_hash
 
+# Must be defined before ibexa.content.download, so that numeric {fieldId} takes precedence over {fieldIdentifier}
+ibexa.content.download.field_id.filename:
+    path: /content/download/{contentId}/{fieldId}/{filename}
+    defaults: { _controller: Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController::downloadBinaryFileByIdAction }
+    requirements:
+        contentId: '\d+'
+        fieldId: '\d+'
+
 ibexa.content.download:
     path: /content/download/{contentId}/{fieldIdentifier}/{filename}
     defaults: { _controller: Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController::downloadBinaryFileAction }
     requirements:
         contentId: '\d+'
 
+# Deprecated since 5.0, will be removed in 6.0. Use ibexa.content.download.field_id.filename instead.
 ibexa.content.download.field_id:
     path: /content/download/{contentId}/{fieldId}
     defaults: { _controller: Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController::downloadBinaryFileByIdAction }

src/bundle/Core/Resources/config/services.yml

@@ -130,6 +130,10 @@ services:
 
     Ibexa\Core\MVC\Symfony\Controller\Content\DownloadRedirectionController:
         class: Ibexa\Core\MVC\Symfony\Controller\Content\DownloadRedirectionController
+        deprecated:
+            package: 'ibexa/core'
+            version: '5.0'
+            message: 'Since ibexa/core 5.0: The "%service_id%" service is deprecated and will be removed in 6.0. No route references it, use Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController::downloadBinaryFileByIdAction instead'
         arguments:
             $contentService: '@ibexa.api.service.content'
             $router: "@router"

src/lib/MVC/Symfony/Controller/Content/DownloadController.php

@@ -45,12 +45,29 @@ public function __construct(
     /**
      * Download binary file identified by field ID.
      *
+     * @param string|null $filename
+     *
      * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException If the field $fieldId can't be found, or the translation can't be found.
      * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException If the content is trashed, or can't be found.
      * @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException If the user has no access to read content and in case of un-published content: read versions.
      */
-    public function downloadBinaryFileByIdAction(Request $request, int $contentId, int $fieldId): BinaryStreamResponse
-    {
+    public function downloadBinaryFileByIdAction(
+        Request $request,
+        int $contentId,
+        int $fieldId,
+        ?string $filename = null
+    ): BinaryStreamResponse {
+        if ($filename === null) {
+            trigger_deprecation(
+                'ibexa/core',
+                '5.0',
+                'The "ibexa.content.download.field_id" route (/content/download/{contentId}/{fieldId}) is deprecated'
+                . ' and will be removed in 6.0.'
+                . ' Use the "ibexa.content.download.field_id.filename" route'
+                . ' (/content/download/{contentId}/{fieldId}/{filename}) instead.'
+            );
+        }
+
         $versionNo = $request->query->has('version') ? $request->query->getInt('version') : null;
         $language = $request->query->has('inLanguage') ? $request->query->get('inLanguage') : null;
 
@@ -65,6 +82,10 @@ public function downloadBinaryFileByIdAction(Request $request, int $contentId, i
             throw new NotFoundException('File', $fieldId);
         }
 
+        if ($filename !== null && $field->value->fileName !== $filename) {
+            throw new NotFoundException('File', $filename);
+        }
+
         return $this->downloadBinaryFileAction($contentId, $field->fieldDefIdentifier, $field->value->fileName, $request);
     }
 

src/lib/MVC/Symfony/Controller/Content/DownloadRedirectionController.php

@@ -20,6 +20,11 @@
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\RouterInterface;
 
+/**
+ * @deprecated since ibexa/core 5.0, will be removed in 6.0. No route has referenced this controller since the
+ * "ibexa.content.download.field_id" route started serving files directly.
+ * Use {@see \Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController::downloadBinaryFileByIdAction} instead.
+ */
 class DownloadRedirectionController extends Controller
 {
     private ContentService $contentService;

src/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGenerator.php

@@ -14,20 +14,22 @@
 
 class ContentDownloadUrlGenerator implements RouteAwarePathGenerator
 {
+    private const ROUTE = 'ibexa.content.download.field_id.filename';
+
     /** @var \Symfony\Component\Routing\RouterInterface */
     private $router;
 
-    /** @var string */
-    private $route = 'ibexa.content.download.field_id';
-
     public function __construct(RouterInterface $router)
     {
         $this->router = $router;
     }
 
     public function getStoragePathForField(Field $field, VersionInfo $versionInfo): string
     {
-        return $this->generate($this->route, $this->getParameters($field, $versionInfo));
+        return $this->generate(
+            $this->getRoute($field, $versionInfo),
+            $this->getParameters($field, $versionInfo)
+        );
     }
 
     public function generate(string $route, ?array $parameters = []): string
@@ -37,7 +39,7 @@ public function generate(string $route, ?array $parameters = []): string
 
     public function getRoute(Field $field, VersionInfo $versionInfo): string
     {
-        return $this->route;
+        return self::ROUTE;
     }
 
     public function getParameters(Field $field, VersionInfo $versionInfo): array
@@ -46,6 +48,7 @@ public function getParameters(Field $field, VersionInfo $versionInfo): array
             'contentId' => $versionInfo->contentInfo->id,
             'fieldId' => $field->id,
             'version' => $versionInfo->versionNo,
+            'filename' => $field->value->externalData['fileName'] ?? '',
         ];
     }
 }

tests/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGeneratorTest.php

@@ -0,0 +1,103 @@
+<?php
+
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
+declare(strict_types=1);
+
+namespace Ibexa\Tests\Core\MVC\Symfony\FieldType\BinaryBase;
+
+use Ibexa\Contracts\Core\Persistence\Content\ContentInfo;
+use Ibexa\Contracts\Core\Persistence\Content\Field;
+use Ibexa\Contracts\Core\Persistence\Content\FieldValue;
+use Ibexa\Contracts\Core\Persistence\Content\VersionInfo;
+use Ibexa\Core\MVC\Symfony\FieldType\BinaryBase\ContentDownloadUrlGenerator;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Routing\RouterInterface;
+
+/**
+ * @covers \Ibexa\Core\MVC\Symfony\FieldType\BinaryBase\ContentDownloadUrlGenerator
+ */
+final class ContentDownloadUrlGeneratorTest extends TestCase
+{
+    private const ROUTE = 'ibexa.content.download.field_id.filename';
+
+    /** @var \Symfony\Component\Routing\RouterInterface&\PHPUnit\Framework\MockObject\MockObject */
+    private RouterInterface $router;
+
+    private ContentDownloadUrlGenerator $generator;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->router = $this->createMock(RouterInterface::class);
+        $this->generator = new ContentDownloadUrlGenerator($this->router);
+    }
+
+    public function testGetRouteReturnsFilenameAwareRoute(): void
+    {
+        self::assertSame(
+            self::ROUTE,
+            $this->generator->getRoute($this->createField(), $this->createVersionInfo())
+        );
+    }
+
+    public function testGetParametersContainFilename(): void
+    {
+        self::assertSame(
+            [
+                'contentId' => 42,
+                'fieldId' => 7,
+                'version' => 2,
+                'filename' => 'Test-file.pdf',
+            ],
+            $this->generator->getParameters($this->createField(), $this->createVersionInfo())
+        );
+    }
+
+    public function testGetStoragePathForFieldGeneratesFilenameAwareUrl(): void
+    {
+        $this->router
+            ->expects(self::once())
+            ->method('generate')
+            ->with(
+                self::ROUTE,
+                [
+                    'contentId' => 42,
+                    'fieldId' => 7,
+                    'version' => 2,
+                    'filename' => 'Test-file.pdf',
+                ]
+            )
+            ->willReturn('/content/download/42/7/Test-file.pdf?version=2');
+
+        self::assertSame(
+            '/content/download/42/7/Test-file.pdf?version=2',
+            $this->generator->getStoragePathForField($this->createField(), $this->createVersionInfo())
+        );
+    }
+
+    private function createField(): Field
+    {
+        return new Field([
+            'id' => 7,
+            'value' => new FieldValue([
+                'externalData' => [
+                    'fileName' => 'Test-file.pdf',
+                ],
+            ]),
+        ]);
+    }
+
+    private function createVersionInfo(): VersionInfo
+    {
+        return new VersionInfo([
+            'versionNo' => 2,
+            'contentInfo' => new ContentInfo([
+                'id' => 42,
+            ]),
+        ]);
+    }
+}