Make config file saving safe against write failures

In case of disk space depletion, writing a new version of config fails,
leaving the config file empty.

This patch improves safety of updates to config.php by saving the new
version of the config into a randomly-named temporary file in the same
directory, and then renaming it to config.php, which renaming is atomic
in POSIX-compliant filesystems and shouldn't involve disk space
allocation. If writing the new config version is impossible, the current
config remains unchanged.

This patch drops the use of file locking as unnecessary. File locking
merely established order in which the concurrent independent processes
attempted file writing. In the end, the process which was last to update
the file "wins" - their changes persist and the changes of previous
writers are dropped as there's no conflict detection or change merging
mechanism anyway. With the current change, there is still some resulting
ordering, and the last writer still wins in the same way. Readers don't
need file locking as well, as opening config.php for reading always
provides a certain consistent version of the file.

Signed-off-by: Andriy Utkin <dev@autkin.net>

1 files changed, 29 insertions, 25 deletions

diff --git a/lib/private/Config.php b/lib/private/Config.php
index 37708357339..e743b26b4e1 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();
@@ -268,30 +261,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);