From 6eddda147bf6200917a39871c529e6bfaff972b5 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Mon, 26 Jan 2026 02:01:52 +0100 Subject: [PATCH 1/2] fix(FileDisplayResponse): return 404 if not found If the linked file is not found (anymore) return proper 404 status code. Signed-off-by: Ferdinand Thiessen --- .../AppFramework/Http/FileDisplayResponse.php | 15 +- .../Http/FileDisplayResponseTest.php | 139 +++++++++++++++--- 2 files changed, 132 insertions(+), 22 deletions(-) diff --git a/lib/public/AppFramework/Http/FileDisplayResponse.php b/lib/public/AppFramework/Http/FileDisplayResponse.php index c18404b7d9161..4b15f2efaa77b 100644 --- a/lib/public/AppFramework/Http/FileDisplayResponse.php +++ b/lib/public/AppFramework/Http/FileDisplayResponse.php @@ -19,8 +19,7 @@ * @template-extends Response> */ class FileDisplayResponse extends Response implements ICallbackResponse { - /** @var File|ISimpleFile */ - private $file; + private File|ISimpleFile $file; /** * FileDisplayResponse constructor. @@ -48,8 +47,18 @@ public function __construct(File|ISimpleFile $file, int $statusCode = Http::STAT */ public function callback(IOutput $output) { if ($output->getHttpResponseCode() !== Http::STATUS_NOT_MODIFIED) { + $file = $this->file instanceof File + ? $this->file->fopen('rb') + : $this->file->read(); + + if ($file === false) { + $output->setHttpResponseCode(Http::STATUS_NOT_FOUND); + $output->setOutput(''); + return; + } + $output->setHeader('Content-Length: ' . $this->file->getSize()); - $output->setOutput($this->file->getContent()); + $output->setReadfile($file); } } } diff --git a/tests/lib/AppFramework/Http/FileDisplayResponseTest.php b/tests/lib/AppFramework/Http/FileDisplayResponseTest.php index 029ddaad7125b..ae9228562a078 100644 --- a/tests/lib/AppFramework/Http/FileDisplayResponseTest.php +++ b/tests/lib/AppFramework/Http/FileDisplayResponseTest.php @@ -1,5 +1,7 @@ file = $this->getMockBuilder('OCP\Files\File') - ->getMock(); + parent::setUp(); + $this->file = $this->createMock(File::class); $this->file->expects($this->once()) ->method('getETag') ->willReturn('myETag'); @@ -52,7 +54,7 @@ public function testLastModified(): void { } public function test304(): void { - $output = $this->getMockBuilder('OCP\AppFramework\Http\IOutput') + $output = $this->getMockBuilder(IOutput::class) ->disableOriginalConstructor() ->getMock(); @@ -69,26 +71,125 @@ public function test304(): void { public function testNon304(): void { - $output = $this->getMockBuilder('OCP\AppFramework\Http\IOutput') + $resource = fopen('php://memory', 'w+b'); + fwrite($resource, 'my data'); + rewind($resource); + + $this->file->expects($this->once()) + ->method('fopen') + ->willReturn($resource); + $this->file->expects($this->any()) + ->method('getSize') + ->willReturn(7); + + $output = $this->getMockBuilder(IOutput::class) ->disableOriginalConstructor() ->getMock(); - - $output->expects($this->any()) + $output->expects($this->once()) ->method('getHttpResponseCode') ->willReturn(Http::STATUS_OK); $output->expects($this->once()) - ->method('setOutput') - ->with($this->equalTo('my data')); + ->method('setReadFile') + ->with($this->equalTo($resource)); $output->expects($this->once()) ->method('setHeader') - ->with($this->equalTo('Content-Length: 42')); + ->with($this->equalTo('Content-Length: 7')); + + $this->response->callback($output); + } + + public function testFileNotFound(): void { $this->file->expects($this->once()) - ->method('getContent') - ->willReturn('my data'); - $this->file->expects($this->any()) - ->method('getSize') - ->willReturn(42); + ->method('fopen') + ->willReturn(false); + + $output = $this->getMockBuilder(IOutput::class) + ->disableOriginalConstructor() + ->getMock(); + $output->expects($this->once()) + ->method('getHttpResponseCode') + ->willReturn(Http::STATUS_OK); + $output->expects($this->once()) + ->method('setHttpResponseCode') + ->with($this->equalTo(Http::STATUS_NOT_FOUND)); + $output->expects($this->once()) + ->method('setOutput') + ->with($this->equalTo('')); $this->response->callback($output); } + + public function testSimpleFileNotFound(): void { + $file = $this->createMock(ISimpleFile::class); + $file->expects($this->once()) + ->method('getETag') + ->willReturn('myETag'); + $file->expects($this->once()) + ->method('getName') + ->willReturn('myFileName'); + $file->expects($this->once()) + ->method('getMTime') + ->willReturn(1464825600); + $file->expects($this->once()) + ->method('read') + ->willReturn(false); + + $response = new FileDisplayResponse($file); + + $output = $this->getMockBuilder(IOutput::class) + ->disableOriginalConstructor() + ->getMock(); + $output->expects($this->once()) + ->method('getHttpResponseCode') + ->willReturn(Http::STATUS_OK); + $output->expects($this->once()) + ->method('setHttpResponseCode') + ->with($this->equalTo(Http::STATUS_NOT_FOUND)); + $output->expects($this->once()) + ->method('setOutput') + ->with($this->equalTo('')); + + $response->callback($output); + } + + public function testSimpleFile(): void { + $file = $this->createMock(ISimpleFile::class); + $file->expects($this->once()) + ->method('getETag') + ->willReturn('myETag'); + $file->expects($this->once()) + ->method('getName') + ->willReturn('myFileName'); + $file->expects($this->once()) + ->method('getMTime') + ->willReturn(1464825600); + + $resource = fopen('php://memory', 'w+b'); + fwrite($resource, 'my data'); + rewind($resource); + + $file->expects($this->once()) + ->method('read') + ->willReturn($resource); + $file->expects($this->any()) + ->method('getSize') + ->willReturn(7); + + $response = new FileDisplayResponse($file); + + $output = $this->getMockBuilder(IOutput::class) + ->disableOriginalConstructor() + ->getMock(); + $output->expects($this->once()) + ->method('getHttpResponseCode') + ->willReturn(Http::STATUS_OK); + $output->expects($this->once()) + ->method('setReadFile') + ->with($this->equalTo($resource)); + $output->expects($this->once()) + ->method('setHeader') + ->with($this->equalTo('Content-Length: 7')); + + $response->callback($output); + } } From 762ae4520aa9eb49009e060300ef6d227eaf8fcc Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Fri, 6 Feb 2026 16:54:43 +0100 Subject: [PATCH 2/2] fix(InMemoryFile): allow to stream read the contents Signed-off-by: Ferdinand Thiessen --- lib/public/Files/SimpleFS/InMemoryFile.php | 14 +++++++------- tests/lib/Files/SimpleFS/InMemoryFileTest.php | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/public/Files/SimpleFS/InMemoryFile.php b/lib/public/Files/SimpleFS/InMemoryFile.php index 7849b8c879a79..d5a0fa160cbc9 100644 --- a/lib/public/Files/SimpleFS/InMemoryFile.php +++ b/lib/public/Files/SimpleFS/InMemoryFile.php @@ -105,7 +105,7 @@ public function getMimeType(): string { } /** - * {@inheritDoc} + * @inheritDoc * @since 24.0.0 */ public function getExtension(): string { @@ -113,15 +113,15 @@ public function getExtension(): string { } /** - * Stream reading is unsupported for in memory files. - * - * @throws NotPermittedException + * @inheritDoc * @since 16.0.0 + * @since 34.0.0 - return in-memory stream of contents */ public function read() { - throw new NotPermittedException( - 'Stream reading is unsupported for in memory files' - ); + $stream = fopen('php://memory', 'r+'); + fwrite($stream, $this->contents); + rewind($stream); + return $stream; } /** diff --git a/tests/lib/Files/SimpleFS/InMemoryFileTest.php b/tests/lib/Files/SimpleFS/InMemoryFileTest.php index 62f3c86bb786a..25f0a4a4d03e3 100644 --- a/tests/lib/Files/SimpleFS/InMemoryFileTest.php +++ b/tests/lib/Files/SimpleFS/InMemoryFileTest.php @@ -4,7 +4,7 @@ /** * SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-only + * SPDX-License-Identifier: AGPL-3.0-or-later */ namespace Test\File\SimpleFS; @@ -14,7 +14,7 @@ use Test\TestCase; /** - * This class provide test casesf or the InMemoryFile. + * This class provide test cases for the InMemoryFile. * * @package Test\File\SimpleFS */ @@ -106,13 +106,13 @@ public function testGetMimeType(): void { /** - * Asserts that read() raises an NotPermittedException. - * - * @return void + * Ensure that read() returns a stream with the same contents than the original file. */ public function testRead(): void { - self::expectException(NotPermittedException::class); - $this->testPdf->read(); + self::assertEquals( + file_get_contents(__DIR__ . '/../../../data/test.pdf'), + stream_get_contents($this->testPdf->read()), + ); } /**