Merge pull request #27349 from nextcloud/backport/25280/stable20

[stable20] Set umask before operations that create local files

lib/private/Avatar/Avatar.php

@@ -156,8 +156,9 @@ protected function generateAvatarFromSvg(int $size) {
 			$avatar->readImageBlob($svg);
 			$avatar->setImageFormat('png');
 			$image = new OC_Image();
-			$image->loadFromData($avatar);
-			return $image->data();
+			$image->loadFromData((string)$avatar);
+			$data = $image->data();
+			return $data === null ? false : $data;
 		} catch (\Exception $e) {
 			return false;
 		}

lib/private/Files/Storage/Local.php

@@ -87,7 +87,11 @@ public function getId() {
 	}
 
 	public function mkdir($path) {
-		return @mkdir($this->getSourcePath($path), 0777, true);
+		$sourcePath = $this->getSourcePath($path);
+		$oldMask = umask(022);
+		$result = @mkdir($sourcePath, 0777, true);
+		umask($oldMask);
+		return $result;
 	}
 
 	public function rmdir($path) {
@@ -256,11 +260,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 +279,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 +352,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) {

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'));
+	}
 }

tests/lib/Preview/Provider.php

@@ -141,6 +141,10 @@ private function getPreview($provider) {
 		$file = new File(\OC::$server->getRootFolder(), $this->rootView, $this->imgPath);
 		$preview = $provider->getThumbnail($file, $this->maxWidth, $this->maxHeight, $this->scalingUp);
 
+		if (get_class($this) === BitmapTest::class && $preview === null) {
+			$this->markTestSkipped('An error occured while operating with Imagick.');
+		}
+
 		$this->assertNotEquals(false, $preview);
 		$this->assertEquals(true, $preview->valid());