From 155034c3ba089ca0d36cfbc94511dc3ca1961e3b Mon Sep 17 00:00:00 2001 From: David Dreschner Date: Tue, 28 Apr 2026 18:12:16 +0200 Subject: [PATCH] fix(SimpleFile): Catch exception and call checkFile() to sanitize broken appdata structure Signed-off-by: David Dreschner --- lib/private/Files/Node/File.php | 4 +++- lib/private/Files/SimpleFS/SimpleFile.php | 14 ++++++-------- lib/public/Files/SimpleFS/ISimpleFile.php | 2 +- tests/lib/Files/SimpleFS/SimpleFileTest.php | 17 ++++++++++++++++- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/lib/private/Files/Node/File.php b/lib/private/Files/Node/File.php index de22ccc9f758d..86898ebff3e9c 100644 --- a/lib/private/Files/Node/File.php +++ b/lib/private/Files/Node/File.php @@ -9,6 +9,7 @@ use OCP\Constants; use OCP\Files\GenericFileException; +use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\Lock\LockedException; @@ -35,7 +36,7 @@ public function getContent() { if ($this->checkPermissions(Constants::PERMISSION_READ)) { $content = $this->view->file_get_contents($this->path); if ($content === false) { - throw new GenericFileException(); + throw new GenericFileException('file_get_contents failed'); } return $content; } else { @@ -66,6 +67,7 @@ public function putContent($data) { /** * @param string $mode * @return resource|false + * @throws NotFoundException * @throws NotPermittedException * @throws LockedException */ diff --git a/lib/private/Files/SimpleFS/SimpleFile.php b/lib/private/Files/SimpleFS/SimpleFile.php index b28a415f57e28..cdc58c94b36b5 100644 --- a/lib/private/Files/SimpleFS/SimpleFile.php +++ b/lib/private/Files/SimpleFS/SimpleFile.php @@ -54,27 +54,25 @@ public function getMTime(): int { /** * Get the content * - * @throws GenericFileException * @throws LockedException * @throws NotFoundException * @throws NotPermittedException */ #[\Override] - public function getContent(): string { - $result = $this->file->getContent(); - - if ($result === false) { + public function getContent(): string|bool { + try { + return $this->file->getContent(); + } catch (GenericFileException) { $this->checkFile(); } - return $result; + return false; } /** * Overwrite the file * * @param string|resource $data - * @throws GenericFileException * @throws LockedException * @throws NotFoundException * @throws NotPermittedException @@ -83,7 +81,7 @@ public function getContent(): string { public function putContent($data): void { try { $this->file->putContent($data); - } catch (NotFoundException $e) { + } catch (GenericFileException) { $this->checkFile(); } } diff --git a/lib/public/Files/SimpleFS/ISimpleFile.php b/lib/public/Files/SimpleFS/ISimpleFile.php index 4e77299ab00da..20f261842367b 100644 --- a/lib/public/Files/SimpleFS/ISimpleFile.php +++ b/lib/public/Files/SimpleFS/ISimpleFile.php @@ -58,7 +58,7 @@ public function getMTime(): int; * @throws NotPermittedException * @since 11.0.0 */ - public function getContent(): string; + public function getContent(): string|bool; /** * Overwrite the file diff --git a/tests/lib/Files/SimpleFS/SimpleFileTest.php b/tests/lib/Files/SimpleFS/SimpleFileTest.php index 6f6746e7cda5a..17c524ab557a2 100644 --- a/tests/lib/Files/SimpleFS/SimpleFileTest.php +++ b/tests/lib/Files/SimpleFS/SimpleFileTest.php @@ -10,6 +10,7 @@ use OC\Files\SimpleFS\SimpleFile; use OCP\Files\File; use OCP\Files\Folder; +use OCP\Files\GenericFileException; use OCP\Files\NotFoundException; class SimpleFileTest extends \Test\TestCase { @@ -92,7 +93,7 @@ public function testGetMimeType(): void { public function testGetContentInvalidAppData(): void { $this->file->method('getContent') - ->willReturn(false); + ->willThrowException(new GenericFileException()); $this->file->method('stat')->willReturn(false); $parent = $this->createMock(Folder::class); @@ -109,6 +110,20 @@ public function testGetContentInvalidAppData(): void { $this->simpleFile->getContent(); } + public function testGetContentReturnsFalseOnFailure(): void { + $this->file->expects($this->once()) + ->method('getContent') + ->willThrowException(new GenericFileException()); + + // We don't want to test the `checkFile()` + // method, so, just return an empty array. + $this->file->method('stat')->willReturn([]); + + $result = $this->simpleFile->getContent(); + + $this->assertFalse($result); + } + public function testRead(): void { $this->file->expects($this->once()) ->method('fopen')