IBX-11681: Added filename validation in DownloadController and corresponding tests

phpstan-baseline-7.4.neon

@@ -180,12 +180,6 @@ parameters:
 			count: 1
 			path: src/lib/MVC/Symfony/Matcher/ContentBased/UrlAlias.php
 
-		-
-			message: '#^Method Ibexa\\Core\\Persistence\\Legacy\\Content\\FieldValue\\Converter\\DateAndTimeConverter\:\:getDateIntervalFromXML\(\) should return DateInterval but returns DateInterval\|false\.$#'
-			identifier: return.type
-			count: 1
-			path: src/lib/Persistence/Legacy/Content/FieldValue/Converter/DateAndTimeConverter.php
-
 		-
 			message: '#^Cannot access property \$ownerDocument on DOMElement\|false\.$#'
 			identifier: property.nonObject

phpstan-baseline-lte-8.1.neon

@@ -1,5 +1,11 @@
 parameters:
 	ignoreErrors:
+		-
+			message: '#^Method Ibexa\\Core\\Persistence\\Legacy\\Content\\FieldValue\\Converter\\DateAndTimeConverter\:\:getDateIntervalFromXML\(\) should return DateInterval but returns DateInterval\|false\.$#'
+			identifier: return.type
+			count: 1
+			path: src/lib/Persistence/Legacy/Content/FieldValue/Converter/DateAndTimeConverter.php
+
 		-
 			message: "#^Class SensitiveParameterValue not found\\.$#"
 			count: 1

phpstan-baseline.neon

@@ -69666,6 +69666,12 @@ parameters:
 			count: 1
 			path: tests/lib/Repository/Service/Mock/ContentTest.php
 
+		-
+			message: '#^Call to new Ibexa\\Core\\Repository\\ContentService\(\) on a separate line has no effect\.$#'
+			identifier: new.resultUnused
+			count: 1
+			path: tests/lib/Repository/Service/Mock/ContentTest.php
+
 		-
 			message: '#^Method Ibexa\\Tests\\Core\\Repository\\Service\\Mock\\ContentTest\:\:assertForCreateContentContentValidationException\(\) has no return type specified\.$#'
 			identifier: missingType.return

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

@@ -8,15 +8,17 @@
 
 use Ibexa\Bundle\IO\BinaryStreamResponse;
 use Ibexa\Contracts\Core\Repository\ContentService;
+use Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException as RepositoryNotFoundException;
 use Ibexa\Contracts\Core\Repository\Values\Content\Content;
 use Ibexa\Contracts\Core\Repository\Values\Content\Field;
 use Ibexa\Core\Base\Exceptions\InvalidArgumentException;
-use Ibexa\Core\Base\Exceptions\NotFoundException;
 use Ibexa\Core\Helper\TranslationHelper;
 use Ibexa\Core\IO\IOServiceInterface;
 use Ibexa\Core\MVC\Symfony\Controller\Controller;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\ResponseHeaderBag;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+use Throwable;
 
 class DownloadController extends Controller
 {
@@ -48,15 +50,15 @@ public function downloadBinaryFileByIdAction(Request $request, int $contentId, i
         $versionNo = $request->query->has('version') ? $request->query->getInt('version') : null;
         $language = $request->query->has('inLanguage') ? $request->query->get('inLanguage') : null;
 
-        $content = $this->contentService->loadContent(
-            $contentId,
-            $language !== null ? [$language] : null,
-            $versionNo,
-        );
         try {
+            $content = $this->contentService->loadContent(
+                $contentId,
+                $language !== null ? [$language] : null,
+                $versionNo,
+            );
             $field = $this->findFieldInContent($fieldId, $content);
-        } catch (InvalidArgumentException $e) {
-            throw new NotFoundException('File', $fieldId);
+        } catch (RepositoryNotFoundException | InvalidArgumentException $e) {
+            throw $this->createFileNotFoundException($e);
         }
 
         return $this->downloadBinaryFileAction($contentId, $field->fieldDefIdentifier, $field->value->fileName, $request);
@@ -90,18 +92,22 @@ protected function findFieldInContent(int $fieldId, Content $content): Field
      */
     public function downloadBinaryFileAction(int $contentId, string $fieldIdentifier, string $filename, Request $request): BinaryStreamResponse
     {
-        if ($request->query->has('version')) {
-            $version = (int) $request->query->get('version');
-            if ($version <= 0) {
-                throw new NotFoundException('File', $filename);
+        try {
+            if ($request->query->has('version')) {
+                $version = (int) $request->query->get('version');
+                if ($version <= 0) {
+                    throw $this->createFileNotFoundException();
+                }
+                $content = $this->contentService->loadContent($contentId, null, $version);
+            } else {
+                $content = $this->contentService->loadContent($contentId);
             }
-            $content = $this->contentService->loadContent($contentId, null, $version);
-        } else {
-            $content = $this->contentService->loadContent($contentId);
+        } catch (RepositoryNotFoundException $e) {
+            throw $this->createFileNotFoundException($e);
         }
 
         if ($content->contentInfo->isTrashed()) {
-            throw new NotFoundException('File', $filename);
+            throw $this->createFileNotFoundException();
         }
 
         $field = $this->translationHelper->getTranslatedField(
@@ -110,10 +116,11 @@ public function downloadBinaryFileAction(int $contentId, string $fieldIdentifier
             $request->query->has('inLanguage') ? $request->query->get('inLanguage') : null
         );
         if (!$field instanceof Field) {
-            throw new InvalidArgumentException(
-                '$fieldIdentifier',
-                "'{$fieldIdentifier}' field not present on content #{$content->contentInfo->id} '{$content->contentInfo->name}'"
-            );
+            throw $this->createFileNotFoundException();
+        }
+
+        if ($field->value->fileName !== $filename) {
+            throw $this->createFileNotFoundException();
         }
 
         $response = new BinaryStreamResponse($this->ioService->loadBinaryFile($field->value->id), $this->ioService);
@@ -125,6 +132,11 @@ public function downloadBinaryFileAction(int $contentId, string $fieldIdentifier
 
         return $response;
     }
+
+    private function createFileNotFoundException(?Throwable $previous = null): NotFoundHttpException
+    {
+        return new NotFoundHttpException('File not found', $previous);
+    }
 }
 
 class_alias(DownloadController::class, 'eZ\Publish\Core\MVC\Symfony\Controller\Content\DownloadController');

tests/integration/Core/MVC/Symfony/Controller/Content/DownloadControllerRequestFlowTest.php

@@ -0,0 +1,201 @@
+<?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\Integration\Core\MVC\Symfony\Controller\Content;
+
+use DateTime;
+use Ibexa\Bundle\IO\BinaryStreamResponse;
+use Ibexa\Contracts\Core\Repository\ContentService;
+use Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo;
+use Ibexa\Contracts\Core\Repository\Values\Content\Field;
+use Ibexa\Contracts\Core\Test\IbexaKernelTestCase;
+use Ibexa\Core\FieldType\BinaryFile\Value as BinaryFileValue;
+use Ibexa\Core\Helper\TranslationHelper;
+use Ibexa\Core\IO\IOServiceInterface;
+use Ibexa\Core\IO\Values\BinaryFile;
+use Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController;
+use Ibexa\Core\Repository\Values\Content\Content;
+use Ibexa\Core\Repository\Values\Content\VersionInfo;
+use Ibexa\Tests\Integration\Core\MVC\Symfony\InternalRoutingTestKernel;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Controller\ArgumentResolver;
+use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
+use Symfony\Component\HttpKernel\EventListener\RouterListener;
+use Symfony\Component\HttpKernel\HttpKernel;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\Routing\Generator\UrlGenerator;
+use Symfony\Component\Routing\Matcher\UrlMatcher;
+use Symfony\Component\Routing\RequestContext;
+use Symfony\Component\Routing\Route;
+use Symfony\Component\Routing\RouteCollection;
+
+/**
+ * @group integration
+ *
+ * @covers \Ibexa\Core\MVC\Symfony\Controller\Content\DownloadController
+ */
+final class DownloadControllerRequestFlowTest extends IbexaKernelTestCase
+{
+    private const FILENAME = 'Q1 report #1 + 100%.jpg';
+
+    /** @var \Ibexa\Contracts\Core\Repository\ContentService&\PHPUnit\Framework\MockObject\MockObject */
+    private ContentService $contentService;
+
+    /** @var \Ibexa\Core\IO\IOServiceInterface&\PHPUnit\Framework\MockObject\MockObject */
+    private IOServiceInterface $ioService;
+
+    /** @var \Ibexa\Core\Helper\TranslationHelper&\PHPUnit\Framework\MockObject\MockObject */
+    private TranslationHelper $translationHelper;
+
+    protected static function getKernelClass(): string
+    {
+        return InternalRoutingTestKernel::class;
+    }
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        self::bootKernel();
+
+        $this->contentService = $this->createMock(ContentService::class);
+        $this->ioService = $this->createMock(IOServiceInterface::class);
+        $this->translationHelper = $this->createMock(TranslationHelper::class);
+    }
+
+    public function testDownloadsFileWithUrlEncodedFilename(): void
+    {
+        $routes = $this->createRouteCollection();
+        $context = new RequestContext();
+        $url = (new UrlGenerator($routes, $context))->generate(
+            'ibexa.content.download',
+            [
+                'contentId' => 42,
+                'fieldIdentifier' => 'file',
+                'filename' => self::FILENAME,
+                'inLanguage' => 'eng-GB',
+            ]
+        );
+
+        self::assertStringContainsString('Q1%20report%20%231%20+%20100%25.jpg', $url);
+
+        $content = $this->createContent();
+        $field = $content->getField('file', 'eng-GB');
+        self::assertInstanceOf(Field::class, $field);
+
+        $binaryFile = new BinaryFile([
+            'id' => 'binary-file-id',
+            'mtime' => new DateTime(),
+            'size' => 123,
+            'uri' => 'binary-file-uri',
+        ]);
+
+        $this->contentService
+            ->expects(self::once())
+            ->method('loadContent')
+            ->with(42)
+            ->willReturn($content);
+        $this->translationHelper
+            ->expects(self::once())
+            ->method('getTranslatedField')
+            ->with($content, 'file', 'eng-GB')
+            ->willReturn($field);
+        $this->ioService
+            ->expects(self::once())
+            ->method('loadBinaryFile')
+            ->with('binary-file-id')
+            ->willReturn($binaryFile);
+
+        $this->configureDownloadController($routes);
+
+        $response = $this->createHttpKernel($routes, $context)->handle(
+            Request::create($url),
+            HttpKernelInterface::MAIN_REQUEST,
+            false
+        );
+
+        self::assertSame(Response::HTTP_OK, $response->getStatusCode());
+        self::assertInstanceOf(BinaryStreamResponse::class, $response);
+    }
+
+    private function configureDownloadController(RouteCollection $routes): void
+    {
+        $route = $routes->get('ibexa.content.download');
+        self::assertNotNull($route);
+        $route->setDefault('_controller', [$this->createController(), 'downloadBinaryFileAction']);
+    }
+
+    private function createController(): DownloadController
+    {
+        return new DownloadController(
+            $this->contentService,
+            $this->ioService,
+            $this->translationHelper
+        );
+    }
+
+    private function createHttpKernel(RouteCollection $routes, RequestContext $context): HttpKernel
+    {
+        $requestStack = new RequestStack();
+        $dispatcher = new EventDispatcher();
+        $dispatcher->addSubscriber(new RouterListener(
+            new UrlMatcher($routes, $context),
+            $requestStack,
+            $context
+        ));
+
+        $controllerResolver = self::getContainer()->get('controller_resolver');
+        self::assertInstanceOf(ControllerResolverInterface::class, $controllerResolver);
+
+        return new HttpKernel(
+            $dispatcher,
+            $controllerResolver,
+            $requestStack,
+            new ArgumentResolver()
+        );
+    }
+
+    private function createRouteCollection(): RouteCollection
+    {
+        $routes = new RouteCollection();
+        $routes->add('ibexa.content.download', new Route(
+            '/content/download/{contentId}/{fieldIdentifier}/{filename}',
+            [],
+            ['contentId' => '\d+']
+        ));
+
+        return $routes;
+    }
+
+    private function createContent(): Content
+    {
+        return new Content([
+            'internalFields' => [
+                new Field([
+                    'fieldDefIdentifier' => 'file',
+                    'languageCode' => 'eng-GB',
+                    'value' => new BinaryFileValue([
+                        'id' => 'binary-file-id',
+                        'fileName' => self::FILENAME,
+                    ]),
+                ]),
+            ],
+            'versionInfo' => new VersionInfo([
+                'contentInfo' => new ContentInfo([
+                    'id' => 42,
+                    'mainLanguageCode' => 'eng-GB',
+                    'name' => 'Test content',
+                    'status' => ContentInfo::STATUS_PUBLISHED,
+                ]),
+            ]),
+        ]);
+    }
+}

tests/integration/Core/MVC/Symfony/InternalRoutingTestKernel.php

@@ -0,0 +1,34 @@
+<?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\Integration\Core\MVC\Symfony;
+
+use Ibexa\Contracts\Core\Test\IbexaTestKernel;
+use Symfony\Component\Config\Loader\LoaderInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+final class InternalRoutingTestKernel extends IbexaTestKernel
+{
+    public function registerContainerConfiguration(LoaderInterface $loader): void
+    {
+        parent::registerContainerConfiguration($loader);
+
+        $loader->load(static function (ContainerBuilder $container): void {
+            self::loadRouting($container);
+        });
+    }
+
+    private static function loadRouting(ContainerBuilder $container): void
+    {
+        $container->loadFromExtension('framework', [
+            'router' => [
+                'resource' => '@IbexaCoreBundle/Resources/config/routing/internal.yml',
+            ],
+        ]);
+    }
+}