Merge pull request #34009 from andrey-utkin/safe-config-update

Make config file saving safe against write failures

1 files changed, 29 insertions, 25 deletions

diff --git a/lib/private/Config.php b/lib/private/Config.php
index ba3b8c6fe4d..e338d3f2332 100644
--- a/lib/private/Config.php
+++ b/lib/private/Config.php
@@ -224,11 +224,6 @@ class Config {
 				continue;
 			}
 
-			// Try to acquire a file lock
-			if (!flock($filePointer, LOCK_SH)) {
-				throw new \Exception(sprintf('Could not acquire a shared lock on the config file %s', $file));
-			}
-
 			unset($CONFIG);
 			include $file;
 			if (!defined('PHPUNIT_RUN') && headers_sent()) {
@@ -244,7 +239,6 @@ class Config {
 			}
 
 			// Close the file pointer and release the lock
-			flock($filePointer, LOCK_UN);
 			fclose($filePointer);
 		}
 
@@ -256,8 +250,7 @@ class Config {
 	 *
 	 * Saves the config to the config file.
 	 *
-	 * @throws HintException If the config file cannot be written to
-	 * @throws \Exception If no file lock can be acquired
+	 * @throws HintException If the config file cannot be written to, etc
 	 */
 	private function writeData() {
 		$this->checkReadOnly();
@@ -272,30 +265,41 @@ class Config {
 		$content .= var_export($this->cache, true);
 		$content .= ";\n";
 
-		touch($this->configFilePath);
-		$filePointer = fopen($this->configFilePath, 'r+');
-
-		// Prevent others not to read the config
-		chmod($this->configFilePath, 0640);
+		// tmpfile must be in the same filesystem for the rename() to be atomic
+		$tmpfile = tempnam(dirname($this->configFilePath), 'config.php.tmp.');
+		// dirname check is for PHP's fallback quirk
+		if (!$tmpfile || dirname($tmpfile) != dirname($this->configFilePath)) {
+			if ($tmpfile) {
+				unlink($tmpfile);
+			}
+			throw new HintException(
+				"Can't create temporary file in config directory!",
+				'This can usually be fixed by giving the webserver write access to the config directory.');
+		}
 
-		// File does not exist, this can happen when doing a fresh install
+		chmod($tmpfile, 0640);
+		$filePointer = fopen($tmpfile, 'w');
 		if (!is_resource($filePointer)) {
 			throw new HintException(
-				"Can't write into config directory!",
-				'This can usually be fixed by giving the webserver write access to the config directory.');
+				"Failed to open temporary file in config directory for writing",
+				'Please report this to Nextcloud developers.');
 		}
 
-		// Try to acquire a file lock
-		if (!flock($filePointer, LOCK_EX)) {
-			throw new \Exception(sprintf('Could not acquire an exclusive lock on the config file %s', $this->configFilePath));
+		$write_ok = fwrite($filePointer, $content);
+		$close_ok = fclose($filePointer);
+		if (!$write_ok || !$close_ok) {
+			unlink($tmpfile);
+			throw new HintException(
+				"Failed to save temporary file in config directory",
+				'Please report this to Nextcloud developers.');
 		}
 
-		// Write the config and release the lock
-		ftruncate($filePointer, 0);
-		fwrite($filePointer, $content);
-		fflush($filePointer);
-		flock($filePointer, LOCK_UN);
-		fclose($filePointer);
+		if (!rename($tmpfile, $this->configFilePath)) {
+			unlink($tmpfile);
+			throw new HintException(
+				"Failed to replace the config file with the new copy",
+				'Please report this to Nextcloud developers.');
+		}
 
 		if (function_exists('opcache_invalidate')) {
 			@opcache_invalidate($this->configFilePath, true);