Revert "Set umask before operations that create local files"

apps/settings/lib/Controller/CheckSetupController.php

@@ -26,6 +26,7 @@
  * @author Timo Förster <tfoerster@webfoersterei.de>
  * @author Valdnet <47037905+Valdnet@users.noreply.github.com>
  * @author MichaIng <micha@dietpi.com>
+ * @author Björn Gottschall <github.mail@bgottschall.de>
  *
  * @license AGPL-3.0
  *
@@ -828,6 +829,14 @@ protected function imageMagickLacksSVGSupport(): bool {
 		return extension_loaded('imagick') && count(\Imagick::queryFormats('SVG')) === 0;
 	}
 
+	/**
+	 * Checks if the process umask has owner read/write/execute permissions
+	 * @return bool
+	 */
+	protected function hasRecommendedUMask(): bool {
+		return ((umask() & 0700) === 0000);
+	}
+
 	/**
 	 * @return DataResponse
 	 * @AuthorizedAdminSetting(settings=OCA\Settings\Settings\Admin\Overview)
@@ -890,6 +899,7 @@ public function check() {
 				SupportedDatabase::class => ['pass' => $supportedDatabases->run(), 'description' => $supportedDatabases->description(), 'severity' => $supportedDatabases->severity()],
 				'temporaryDirectoryWritable' => $this->isTemporaryDirectoryWritable(),
 				LdapInvalidUuids::class => ['pass' => $ldapInvalidUuids->run(), 'description' => $ldapInvalidUuids->description(), 'severity' => $ldapInvalidUuids->severity()],
+				'hasRecommendedUMask' => $this->hasRecommendedUMask(),
 			]
 		);
 	}

apps/settings/tests/Controller/CheckSetupControllerTest.php

@@ -671,6 +671,7 @@ public function testCheck() {
 				'isFairUseOfFreePushService' => false,
 				'temporaryDirectoryWritable' => false,
 				\OCA\Settings\SetupChecks\LdapInvalidUuids::class => ['pass' => true, 'description' =>  'Invalid UUIDs of LDAP users or groups have been found. Please review your "Override UUID detection" settings in the Expert part of the LDAP configuration and use "occ ldap:update-uuid" to update them.', 'severity' => 'warning'],
+				'hasRecommendedUMask' => true,
 			]
 		);
 		$this->assertEquals($expected, $this->checkSetupController->check());

lib/private/Files/Storage/Local.php

@@ -95,9 +95,7 @@ public function getId() {
 
 	public function mkdir($path) {
 		$sourcePath = $this->getSourcePath($path);
-		$oldMask = umask(022);
 		$result = @mkdir($sourcePath, 0777, true);
-		umask($oldMask);
 		return $result;
 	}
 
@@ -269,13 +267,11 @@ 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));
 		}
@@ -288,10 +284,7 @@ public function file_get_contents($path) {
 	}
 
 	public function file_put_contents($path, $data) {
-		$oldMask = umask(022);
-		$result = file_put_contents($this->getSourcePath($path), $data);
-		umask($oldMask);
-		return $result;
+		return file_put_contents($this->getSourcePath($path), $data);
 	}
 
 	public function unlink($path) {
@@ -361,18 +354,12 @@ public function copy($path1, $path2) {
 		if ($this->is_dir($path1)) {
 			return parent::copy($path1, $path2);
 		} else {
-			$oldMask = umask(022);
-			$result = copy($this->getSourcePath($path1), $this->getSourcePath($path2));
-			umask($oldMask);
-			return $result;
+			return copy($this->getSourcePath($path1), $this->getSourcePath($path2));
 		}
 	}
 
 	public function fopen($path, $mode) {
-		$oldMask = umask(022);
-		$result = fopen($this->getSourcePath($path), $mode);
-		umask($oldMask);
-		return $result;
+		return fopen($this->getSourcePath($path), $mode);
 	}
 
 	public function hash($type, $path, $raw = false) {

tests/lib/Files/Storage/LocalTest.php

@@ -108,35 +108,4 @@ 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'));
-	}
 }