From 3af74cfeccc4443fad53cba5c9aba6b2453ff73a Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 22 Jan 2021 15:44:24 +0100 Subject: [PATCH] Set umask before operations that create local files this solves issues where "other php stuff" is messing with the umask Signed-off-by: Robin Appelman --- lib/private/Files/Storage/Local.php | 17 +++++++++--- tests/lib/Files/Storage/LocalTest.php | 37 ++++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/lib/private/Files/Storage/Local.php b/lib/private/Files/Storage/Local.php index 26687f96081e3..3b30034c1b59d 100644 --- a/lib/private/Files/Storage/Local.php +++ b/lib/private/Files/Storage/Local.php @@ -256,11 +256,13 @@ public function touch($path, $mtime = null) { if ($this->file_exists($path) and !$this->isUpdatable($path)) { return false; } + $oldMask = umask(022); if (!is_null($mtime)) { $result = @touch($this->getSourcePath($path), $mtime); } else { $result = @touch($this->getSourcePath($path)); } + umask($oldMask); if ($result) { clearstatcache(true, $this->getSourcePath($path)); } @@ -273,7 +275,10 @@ public function file_get_contents($path) { } public function file_put_contents($path, $data) { - return file_put_contents($this->getSourcePath($path), $data); + $oldMask = umask(022); + $result = file_put_contents($this->getSourcePath($path), $data); + umask($oldMask); + return $result; } public function unlink($path) { @@ -343,12 +348,18 @@ public function copy($path1, $path2) { if ($this->is_dir($path1)) { return parent::copy($path1, $path2); } else { - return copy($this->getSourcePath($path1), $this->getSourcePath($path2)); + $oldMask = umask(022); + $result = copy($this->getSourcePath($path1), $this->getSourcePath($path2)); + umask($oldMask); + return $result; } } public function fopen($path, $mode) { - return fopen($this->getSourcePath($path), $mode); + $oldMask = umask(022); + $result = fopen($this->getSourcePath($path), $mode); + umask($oldMask); + return $result; } public function hash($type, $path, $raw = false) { diff --git a/tests/lib/Files/Storage/LocalTest.php b/tests/lib/Files/Storage/LocalTest.php index 467848112488c..e324d2b28db06 100644 --- a/tests/lib/Files/Storage/LocalTest.php +++ b/tests/lib/Files/Storage/LocalTest.php @@ -63,21 +63,21 @@ public function testEtagChange() { $this->assertNotEquals($etag1, $etag2); } - + public function testInvalidArgumentsEmptyArray() { $this->expectException(\InvalidArgumentException::class); new \OC\Files\Storage\Local([]); } - + public function testInvalidArgumentsNoArray() { $this->expectException(\InvalidArgumentException::class); new \OC\Files\Storage\Local(null); } - + public function testDisallowSymlinksOutsideDatadir() { $this->expectException(\OCP\Files\ForbiddenException::class); @@ -108,4 +108,35 @@ public function testDisallowSymlinksInsideDatadir() { $storage->file_put_contents('sym/foo', 'bar'); $this->addToAssertionCount(1); } + + public function testWriteUmaskFilePutContents() { + $oldMask = umask(0333); + $this->instance->file_put_contents('test.txt', 'sad'); + umask($oldMask); + $this->assertTrue($this->instance->isUpdatable('test.txt')); + } + + public function testWriteUmaskMkdir() { + $oldMask = umask(0333); + $this->instance->mkdir('test.txt'); + umask($oldMask); + $this->assertTrue($this->instance->isUpdatable('test.txt')); + } + + public function testWriteUmaskFopen() { + $oldMask = umask(0333); + $handle = $this->instance->fopen('test.txt', 'w'); + fwrite($handle, 'foo'); + fclose($handle); + umask($oldMask); + $this->assertTrue($this->instance->isUpdatable('test.txt')); + } + + public function testWriteUmaskCopy() { + $this->instance->file_put_contents('source.txt', 'sad'); + $oldMask = umask(0333); + $this->instance->copy('source.txt', 'test.txt'); + umask($oldMask); + $this->assertTrue($this->instance->isUpdatable('test.txt')); + } }