fix(FileDisplayResponse): return 404 if not found

lib/public/AppFramework/Http/FileDisplayResponse.php

@@ -19,8 +19,7 @@
  * @template-extends Response<Http::STATUS_*, array<string, mixed>>
  */
 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);
 		}
 	}
 }

lib/public/Files/SimpleFS/InMemoryFile.php

@@ -105,23 +105,23 @@ public function getMimeType(): string {
 	}
 
 	/**
-	 * {@inheritDoc}
+	 * @inheritDoc
 	 * @since 24.0.0
 	 */
 	public function getExtension(): string {
 		return \pathinfo($this->name, PATHINFO_EXTENSION);
 	}
 
 	/**
-	 * 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;
 	}
 
 	/**

tests/lib/AppFramework/Http/FileDisplayResponseTest.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -9,19 +11,19 @@
 
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\FileDisplayResponse;
+use OCP\AppFramework\Http\IOutput;
 use OCP\Files\File;
+use OCP\Files\SimpleFS\ISimpleFile;
+use PHPUnit\Framework\MockObject\MockObject;
 
 class FileDisplayResponseTest extends \Test\TestCase {
-	/** @var File|\PHPUnit\Framework\MockObject\MockObject */
-	private $file;
-
-	/** @var FileDisplayResponse */
-	private $response;
+	private File&MockObject $file;
+	private FileDisplayResponse $response;
 
 	protected function setUp(): void {
-		$this->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);
+	}
 }

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()),
+		);
 	}
 
 	/**