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 <opensource@fthiessen.de>
This commit is contained in:
Ferdinand Thiessen 2026-01-26 02:01:52 +01:00
parent 6c9418880f
commit 6eddda147b
No known key found for this signature in database
GPG key ID: 7E849AE05218500F
2 changed files with 133 additions and 23 deletions

View file

@ -19,8 +19,7 @@ use OCP\Files\SimpleFS\ISimpleFile;
* @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 @@ class FileDisplayResponse extends Response implements ICallbackResponse {
*/
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);
}
}
}

View file

@ -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 @@ namespace Test\AppFramework\Http;
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 @@ class FileDisplayResponseTest extends \Test\TestCase {
}
public function test304(): void {
$output = $this->getMockBuilder('OCP\AppFramework\Http\IOutput')
$output = $this->getMockBuilder(IOutput::class)
->disableOriginalConstructor()
->getMock();
@ -69,26 +71,125 @@ class FileDisplayResponseTest extends \Test\TestCase {
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'));
$this->file->expects($this->once())
->method('getContent')
->willReturn('my data');
$this->file->expects($this->any())
->method('getSize')
->willReturn(42);
->with($this->equalTo('Content-Length: 7'));
$this->response->callback($output);
}
public function testFileNotFound(): void {
$this->file->expects($this->once())
->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);
}
}