From 8b5997483c852bcee6b44188982073c8213de25f Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 9 Feb 2018 20:15:29 +0100 Subject: [PATCH] Hardening of SimpleFile getContent if file_get_contents fails remove the file. And traverse up the tree checking if the other folders are there. Signed-off-by: Roeland Jago Douma --- lib/private/Files/SimpleFS/SimpleFile.php | 36 ++++++++++++++++++++- lib/public/Files/SimpleFS/ISimpleFile.php | 3 ++ tests/lib/Files/SimpleFS/SimpleFileTest.php | 22 +++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/SimpleFS/SimpleFile.php b/lib/private/Files/SimpleFS/SimpleFile.php index 5eadfd98b6..1f2b497a19 100644 --- a/lib/private/Files/SimpleFS/SimpleFile.php +++ b/lib/private/Files/SimpleFS/SimpleFile.php @@ -23,6 +23,7 @@ namespace OC\Files\SimpleFS; use OCP\Files\File; +use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\Files\SimpleFS\ISimpleFile; @@ -79,10 +80,18 @@ class SimpleFile implements ISimpleFile { /** * Get the content * + * @throws NotPermittedException + * @throws NotFoundException * @return string */ public function getContent() { - return $this->file->getContent(); + $result = $this->file->getContent(); + + if ($result === false) { + $this->checkFile(); + } + + return $result; } /** @@ -95,6 +104,31 @@ class SimpleFile implements ISimpleFile { $this->file->putContent($data); } + /** + * Sometimes there are some issues with the AppData. Most of them are from + * user error. But we should handle them gracefull anyway. + * + * If for some reason the current file can't be found. We remove it. + * Then traverse up and check all folders if they exists. This so that the + * next request will have a valid appdata structure again. + * + * @throws NotFoundException + */ + private function checkFile() { + $cur = $this->file; + + while ($cur->stat() === false) { + $parent = $cur->getParent(); + $cur->delete(); + $cur = $parent; + } + + if ($cur !== $this->file) { + throw new NotFoundException('File does not exist'); + } + } + + /** * Delete the file * diff --git a/lib/public/Files/SimpleFS/ISimpleFile.php b/lib/public/Files/SimpleFS/ISimpleFile.php index e9182377cb..e03509d7ab 100644 --- a/lib/public/Files/SimpleFS/ISimpleFile.php +++ b/lib/public/Files/SimpleFS/ISimpleFile.php @@ -22,6 +22,7 @@ */ namespace OCP\Files\SimpleFS; +use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; /** @@ -67,6 +68,8 @@ interface ISimpleFile { /** * Get the content * + * @throws NotPermittedException + * @throws NotFoundException * @return string * @since 11.0.0 */ diff --git a/tests/lib/Files/SimpleFS/SimpleFileTest.php b/tests/lib/Files/SimpleFS/SimpleFileTest.php index 4e623eafa2..ab4970804a 100644 --- a/tests/lib/Files/SimpleFS/SimpleFileTest.php +++ b/tests/lib/Files/SimpleFS/SimpleFileTest.php @@ -24,6 +24,9 @@ namespace Test\File\SimpleFS; use OC\Files\SimpleFS\SimpleFile; use OCP\Files\File; +use OCP\Files\Folder; +use OCP\Files\NotFoundException; +use OCP\Files\NotPermittedException; class SimpleFileTest extends \Test\TestCase { /** @var File|\PHPUnit_Framework_MockObject_MockObject */ @@ -101,4 +104,23 @@ class SimpleFileTest extends \Test\TestCase { $this->assertEquals('app/awesome', $this->simpleFile->getMimeType()); } + + public function testGetContentInvalidAppData() { + $this->file->method('getContent') + ->willReturn(false); + $this->file->method('stat')->willReturn(false); + + $parent = $this->createMock(Folder::class); + $parent->method('stat')->willReturn(false); + + $root = $this->createMock(Folder::class); + $root->method('stat')->willReturn([]); + + $this->file->method('getParent')->willReturn($parent); + $parent->method('getParent')->willReturn($root); + + $this->expectException(NotFoundException::class); + + $this->simpleFile->getContent(); + } }