From 5aa6a94290d555dba05055924d07dce2d1527c01 Mon Sep 17 00:00:00 2001 From: MateuszKolankowski Date: Mon, 6 Jul 2026 17:06:06 +0200 Subject: [PATCH 1/2] Deprecated filename-less download route in favor of filename-validated variant --- .../Resources/config/routing/internal.yml | 9 ++ src/bundle/Core/Resources/config/services.yml | 4 + .../Controller/Content/DownloadController.php | 25 ++++- .../Content/DownloadRedirectionController.php | 5 + .../ContentDownloadUrlGenerator.php | 13 ++- .../ContentDownloadUrlGeneratorTest.php | 103 ++++++++++++++++++ 6 files changed, 152 insertions(+), 7 deletions(-) create mode 100644 tests/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGeneratorTest.php diff --git a/src/bundle/Core/Resources/config/routing/internal.yml b/src/bundle/Core/Resources/config/routing/internal.yml index 2bb2682533..9b0155a983 100644 --- a/src/bundle/Core/Resources/config/routing/internal.yml +++ b/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 } diff --git a/src/bundle/Core/Resources/config/services.yml b/src/bundle/Core/Resources/config/services.yml index 1a00718c3d..7d90aea210 100644 --- a/src/bundle/Core/Resources/config/services.yml +++ b/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" diff --git a/src/lib/MVC/Symfony/Controller/Content/DownloadController.php b/src/lib/MVC/Symfony/Controller/Content/DownloadController.php index 3d87345b0a..6671dfb854 100644 --- a/src/lib/MVC/Symfony/Controller/Content/DownloadController.php +++ b/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); } diff --git a/src/lib/MVC/Symfony/Controller/Content/DownloadRedirectionController.php b/src/lib/MVC/Symfony/Controller/Content/DownloadRedirectionController.php index 2e33317b27..a550134db3 100644 --- a/src/lib/MVC/Symfony/Controller/Content/DownloadRedirectionController.php +++ b/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; diff --git a/src/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGenerator.php b/src/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGenerator.php index e067fb62b2..8100efd121 100644 --- a/src/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGenerator.php +++ b/src/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGenerator.php @@ -14,12 +14,11 @@ 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; @@ -27,7 +26,10 @@ public function __construct(RouterInterface $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'] ?? '', ]; } } diff --git a/tests/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGeneratorTest.php b/tests/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGeneratorTest.php new file mode 100644 index 0000000000..da27df4725 --- /dev/null +++ b/tests/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGeneratorTest.php @@ -0,0 +1,103 @@ +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, + ]), + ]); + } +} From 24581e19134d6ed5be07baf81a4ea46dafa0a0bc Mon Sep 17 00:00:00 2001 From: MateuszKolankowski Date: Tue, 7 Jul 2026 13:10:25 +0200 Subject: [PATCH 2/2] Deprecate filename parameter in download route and update type hints in ContentDownloadUrlGenerator --- .../MVC/Symfony/Controller/Content/DownloadController.php | 2 -- .../FieldType/BinaryBase/ContentDownloadUrlGenerator.php | 5 ++--- .../BinaryBase/ContentDownloadUrlGeneratorTest.php | 6 +++--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/lib/MVC/Symfony/Controller/Content/DownloadController.php b/src/lib/MVC/Symfony/Controller/Content/DownloadController.php index 6671dfb854..9f1fb9ebdb 100644 --- a/src/lib/MVC/Symfony/Controller/Content/DownloadController.php +++ b/src/lib/MVC/Symfony/Controller/Content/DownloadController.php @@ -45,8 +45,6 @@ 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. diff --git a/src/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGenerator.php b/src/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGenerator.php index 8100efd121..9a6d31a6a9 100644 --- a/src/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGenerator.php +++ b/src/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGenerator.php @@ -14,10 +14,9 @@ class ContentDownloadUrlGenerator implements RouteAwarePathGenerator { - private const ROUTE = 'ibexa.content.download.field_id.filename'; + private const string ROUTE = 'ibexa.content.download.field_id.filename'; - /** @var \Symfony\Component\Routing\RouterInterface */ - private $router; + private RouterInterface $router; public function __construct(RouterInterface $router) { diff --git a/tests/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGeneratorTest.php b/tests/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGeneratorTest.php index da27df4725..8d1949b3b4 100644 --- a/tests/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGeneratorTest.php +++ b/tests/lib/MVC/Symfony/FieldType/BinaryBase/ContentDownloadUrlGeneratorTest.php @@ -13,6 +13,7 @@ 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\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Component\Routing\RouterInterface; @@ -21,10 +22,9 @@ */ final class ContentDownloadUrlGeneratorTest extends TestCase { - private const ROUTE = 'ibexa.content.download.field_id.filename'; + private const string ROUTE = 'ibexa.content.download.field_id.filename'; - /** @var \Symfony\Component\Routing\RouterInterface&\PHPUnit\Framework\MockObject\MockObject */ - private RouterInterface $router; + private RouterInterface & MockObject $router; private ContentDownloadUrlGenerator $generator;