diff --git a/lib/AppConfig.php b/lib/AppConfig.php index 3e0ee1ddb0..5e5f1e1cfb 100644 --- a/lib/AppConfig.php +++ b/lib/AppConfig.php @@ -30,9 +30,14 @@ class AppConfig { // Default: 'no', set to 'yes' to enable public const USE_SECURE_VIEW_ADDITIONAL_MIMES = 'use_secure_view_additional_mimes'; + public const PREVIEW_CONVERSION_TIMEOUT = 'preview_conversion_timeout'; + public const PREVIEW_CONVERSION_MAX_FILESIZE = 'preview_conversion_max_filesize'; + private array $defaults = [ 'wopi_url' => '', 'timeout' => 15, + 'preview_conversion_timeout' => 5, + 'preview_conversion_max_filesize' => 104857600, // 100 MB 'watermark_text' => '{userId}', 'watermark_allGroupsList' => [], 'watermark_allTagsList' => [], diff --git a/lib/Preview/Office.php b/lib/Preview/Office.php index 8b8caa8390..7b90da76ae 100644 --- a/lib/Preview/Office.php +++ b/lib/Preview/Office.php @@ -33,12 +33,22 @@ public function isAvailable(FileInfo $file): bool { } public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage { - if ($file->getSize() === 0) { + $fileSize = $file->getSize(); + if ($fileSize === 0) { + return null; + } + + $maxFileSize = $this->appConfig->getPreviewConversionMaxFileSize(); + if ($fileSize > $maxFileSize) { + $this->logger->debug('Skipping preview conversion: file size {size} exceeds limit {limit}', [ + 'size' => $fileSize, + 'limit' => $maxFileSize, + ]); return null; } try { - $response = $this->remoteService->convertFileTo($file, 'png'); + $response = $this->remoteService->convertFileTo($file, 'png', $this->appConfig->getPreviewConversionTimeout()); $image = new Image(); $image->loadFromData($response); diff --git a/lib/Service/RemoteService.php b/lib/Service/RemoteService.php index ac2df4d102..ffd24d9b07 100644 --- a/lib/Service/RemoteService.php +++ b/lib/Service/RemoteService.php @@ -57,14 +57,19 @@ public function fetchTargetThumbnail(File $file, string $target): ?string { /** * @return resource|string */ - public function convertFileTo(File $file, string $format) { + public function convertFileTo(File $file, string $format, int $timeout = RemoteOptionsService::REMOTE_TIMEOUT_DEFAULT) { $fileName = $file->getStorage()->getLocalFile($file->getInternalPath()); $stream = fopen($fileName, 'rb'); if ($stream === false) { throw new Exception('Failed to open stream'); } - return $this->convertTo($file->getName(), $stream, $format); + + try { + return $this->convertTo($file->getName(), $stream, $format, [], $timeout); + } finally { + fclose($stream); + } } /** @@ -73,7 +78,7 @@ public function convertFileTo(File $file, string $format) { */ public function convertTo(string $filename, $stream, string $format) { $client = $this->clientService->newClient(); - $options = RemoteOptionsService::getDefaultOptions(); + $options = RemoteOptionsService::getDefaultOptions($timeout); // FIXME: can be removed once https://github.com/CollaboraOnline/online/issues/6983 is fixed upstream $options['expect'] = false; diff --git a/tests/lib/AppConfigTest.php b/tests/lib/AppConfigTest.php index 1f9296427f..c9722f1fff 100644 --- a/tests/lib/AppConfigTest.php +++ b/tests/lib/AppConfigTest.php @@ -58,4 +58,40 @@ public function testGetAppValueArrayWithNoneValue() { $this->assertSame([], $result); } + + public function testGetPreviewConversionTimeoutReturnsDefault(): void { + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('richdocuments', 'preview_conversion_timeout', 5) + ->willReturn(5); + + $this->assertSame(5, $this->appConfig->getPreviewConversionTimeout()); + } + + public function testGetPreviewConversionTimeoutReturnsConfiguredValue(): void { + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('richdocuments', 'preview_conversion_timeout', 5) + ->willReturn(30); + + $this->assertSame(30, $this->appConfig->getPreviewConversionTimeout()); + } + + public function testGetPreviewConversionMaxFileSizeReturnsDefault(): void { + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('richdocuments', 'preview_conversion_max_filesize', 104857600) + ->willReturn(104857600); + + $this->assertSame(104857600, $this->appConfig->getPreviewConversionMaxFileSize()); + } + + public function testGetPreviewConversionMaxFileSizeReturnsConfiguredValue(): void { + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('richdocuments', 'preview_conversion_max_filesize', 104857600) + ->willReturn(52428800); + + $this->assertSame(52428800, $this->appConfig->getPreviewConversionMaxFileSize()); + } } diff --git a/tests/lib/Preview/OfficeTest.php b/tests/lib/Preview/OfficeTest.php new file mode 100644 index 0000000000..f3bb6d19cc --- /dev/null +++ b/tests/lib/Preview/OfficeTest.php @@ -0,0 +1,103 @@ +remoteService = $this->createMock(RemoteService::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->appConfig = $this->createMock(AppConfig::class); + $this->capabilities = $this->createMock(Capabilities::class); + $this->capabilities->method('getCapabilities')->willReturn(['richdocuments' => []]); + + $this->provider = new class($this->remoteService, $this->logger, $this->appConfig, $this->capabilities) extends Office { + #[\Override] + public function getMimeType(): string { + return '/application\\/test/'; + } + }; + } + + public function testGetThumbnailSkipsConversionWhenFileIsTooLarge(): void { + $file = $this->createMock(File::class); + $file->expects($this->once())->method('getSize')->willReturn(101 * 1024 * 1024); + + $this->appConfig->expects($this->once()) + ->method('getPreviewConversionMaxFileSize') + ->willReturn(100 * 1024 * 1024); + $this->remoteService->expects($this->never())->method('convertFileTo'); + + $result = $this->provider->getThumbnail($file, 64, 64); + + $this->assertNull($result); + } + + public function testGetThumbnailReturnsNullForEmptyFile(): void { + $file = $this->createMock(File::class); + $file->expects($this->once())->method('getSize')->willReturn(0); + + $this->appConfig->expects($this->never())->method('getPreviewConversionMaxFileSize'); + $this->remoteService->expects($this->never())->method('convertFileTo'); + + $result = $this->provider->getThumbnail($file, 64, 64); + + $this->assertNull($result); + } + + public function testGetThumbnailAttemptsConversionWhenFileSizeIsExactlyAtLimit(): void { + $file = $this->createMock(File::class); + $file->expects($this->once())->method('getSize')->willReturn(100 * 1024 * 1024); + + $this->appConfig->expects($this->once()) + ->method('getPreviewConversionMaxFileSize') + ->willReturn(100 * 1024 * 1024); + // Conversion is attempted; throw to keep the test simple (image loading is not unit-tested here) + $this->remoteService->expects($this->once()) + ->method('convertFileTo') + ->with($file, 'png') + ->willThrowException(new \Exception('conversion failed')); + + $result = $this->provider->getThumbnail($file, 64, 64); + + $this->assertNull($result); + } + + public function testGetThumbnailReturnsNullWhenConversionFails(): void { + $file = $this->createMock(File::class); + $file->expects($this->once())->method('getSize')->willReturn(1024); + + $this->appConfig->expects($this->once()) + ->method('getPreviewConversionMaxFileSize') + ->willReturn(100 * 1024 * 1024); + $this->remoteService->expects($this->once()) + ->method('convertFileTo') + ->with($file, 'png') + ->willThrowException(new \Exception('conversion failed')); + + $result = $this->provider->getThumbnail($file, 64, 64); + + $this->assertNull($result); + } +}