! updates to how the settings.php file is saved. Move db_last_error to its own file (yes ugly) to avoid any possible race conditions. Added lock_ex to acquire an exclusive lock on the file while proceeding to the writing, added check for proper witting, and if failure, attempt to restore the settings_bak.php to recover

Sources/ManageServer.php

@@ -104,6 +104,9 @@ function ModifySettings()
 	// By default we're editing the core settings
 	$_REQUEST['sa'] = isset($_REQUEST['sa']) && isset($subActions[$_REQUEST['sa']]) ? $_REQUEST['sa'] : 'general';
 	$context['sub_action'] = $_REQUEST['sa'];
+	
+	// Any messages to speak of?
+	$context['settings_message'] = (isset($_REQUEST['msg']) && isset($txt[$_REQUEST['msg']])) ? $txt[$_REQUEST['msg']] : '';
 
 	// Warn the user if there's any relevant information regarding Settings.php.
 	if ($_REQUEST['sa'] != 'cache')
@@ -174,7 +177,7 @@ function ModifyGeneralSettings($return_config = false)
 		call_integration_hook('integrate_save_general_settings');
 
 		saveSettings($config_vars);
-		redirectexit('action=admin;area=serversettings;sa=general;' . $context['session_var'] . '=' . $context['session_id']);
+		redirectexit('action=admin;area=serversettings;sa=general;' . $context['session_var'] . '=' . $context['session_id']. ';msg=' . (!empty($context['settings_message']) ? $context['settings_message'] : 'core_settings_saved'));
 	}
 
 	// Fill the config array.
@@ -238,7 +241,7 @@ function ModifyDatabaseSettings($return_config = false)
 		call_integration_hook('integrate_save_database_settings');
 
 		saveSettings($config_vars);
-		redirectexit('action=admin;area=serversettings;sa=database;' . $context['session_var'] . '=' . $context['session_id']);
+		redirectexit('action=admin;area=serversettings;sa=database;' . $context['session_var'] . '=' . $context['session_id'] . ';msg=' . (!empty($context['settings_message']) ? $context['settings_message'] : 'core_settings_saved'));
 	}
 
 	// Fill the config array.
@@ -305,7 +308,7 @@ function ModifyCookieSettings($return_config = false)
 			redirectexit('action=admin;area=serversettings;sa=cookie;' . $context['session_var'] . '=' . $original_session_id, $context['server']['needs_login_fix']);
 		}
 
-		redirectexit('action=admin;area=serversettings;sa=cookie;' . $context['session_var'] . '=' . $context['session_id']);
+		redirectexit('action=admin;area=serversettings;sa=cookie;' . $context['session_var'] . '=' . $context['session_id']. ';msg=' . (!empty($context['settings_message']) ? $context['settings_message'] : 'core_settings_saved'));
 	}
 
 	// Fill the config array.
@@ -505,6 +508,7 @@ function ModifyLoadBalancingSettings($return_config = false)
 	}
 	
 	createToken('admin-ssc');
+	createToken('admin-dbsc');
 	prepareDBSettingContext($config_vars);
 }
 

Sources/Subs-Admin.php

@@ -77,13 +77,13 @@ function getServerVersions($checkFor)
 
 /**
  * Search through source, theme and language files to determine their version.
- * Get detailed version information about the physical SMF files on the
- * server.
- * the input parameter allows to set whether to include SSI.php and
- * whether the results should be sorted.
- * returns an array containing information on source files, templates
- * and language files found in the default theme directory (grouped by
- * language).
+ * Get detailed version information about the physical SMF files on the server.
+ * 
+ * - the input parameter allows to set whether to include SSI.php and whether 
+ *   the results should be sorted.
+ * - returns an array containing information on source files, templates and
+ *   language files found in the default theme directory (grouped by language).
+ *
  * @param array &$versionOptions
  */
 function getFileVersions(&$versionOptions)
@@ -223,25 +223,38 @@ function getFileVersions(&$versionOptions)
  *
  * The most important function in this file for mod makers happens to be the
  * updateSettingsFile() function, but it shouldn't be used often anyway.
- * updates the Settings.php file with the changes in config_vars.
- * expects config_vars to be an associative array, with the keys as the
- * variable names in Settings.php, and the values the varaible values.
- * does not escape or quote values.
- * preserves case, formatting, and additional options in file.
- * writes nothing if the resulting file would be less than 10 lines
- * in length (sanity check for read lock.)
+ * 
+ * - updates the Settings.php file with the changes supplied in config_vars.
+ * - expects config_vars to be an associative array, with the keys as the
+ *   variable names in Settings.php, and the values the variable values.
+ * - does not escape or quote values.
+ * - preserves case, formatting, and additional options in file.
+ * - writes nothing if the resulting file would be less than 10 lines
+ *   in length (sanity check for read lock.)
+ * - check for changes to db_last_error and passes those off to a separate handler
+ * - attempts to create a backup file and will use it should the writing of the 
+ *   new settings file fail
  *
  * @param array $config_vars
  */
 function updateSettingsFile($config_vars)
 {
-	global $boarddir, $cachedir;
+	global $boarddir, $cachedir, $context;
+	
+	// Updating the db_last_error, then don't mess around with Settings.php
+	if (count($config_vars) === 1 && isset($config_vars['db_last_error']))
+	{
+		updateDbLastError($config_vars['db_last_error']);
+		return;
+	}
 
-	// When is Settings.php last changed?
+	// When was Settings.php last changed?
 	$last_settings_change = filemtime($boarddir . '/Settings.php');
 
-	// Load the file.  Break it up based on \r or \n, and then clean out extra characters.
+	// Load the settings file.
 	$settingsArray = trim(file_get_contents($boarddir . '/Settings.php'));
+	
+	// Break it up based on \r or \n, and then clean out extra characters.
 	if (strpos($settingsArray, "\n") !== false)
 		$settingsArray = explode("\n", $settingsArray);
 	elseif (strpos($settingsArray, "\r") !== false)
@@ -249,23 +262,15 @@ function updateSettingsFile($config_vars)
 	else
 		return;
 
-	// Make sure we got a good file.
-	if (count($config_vars) == 1 && isset($config_vars['db_last_error']))
-	{
-		$temp = trim(implode("\n", $settingsArray));
-		if (substr($temp, 0, 5) != '<?php' || substr($temp, -2) != '?' . '>')
-			return;
-		if (strpos($temp, 'sourcedir') === false || strpos($temp, 'boarddir') === false)
-			return;
-	}
-
 	// Presumably, the file has to have stuff in it for this function to be called :P.
 	if (count($settingsArray) < 10)
 		return;
 
+	// remove any /r's that made there way in here
 	foreach ($settingsArray as $k => $dummy)
 		$settingsArray[$k] = strtr($dummy, array("\r" => '')) . "\n";
 
+	// go line by line and see whats changing
 	for ($i = 0, $n = count($settingsArray); $i < $n; $i++)
 	{
 		// Don't trim or bother with it if it's not a variable.
@@ -277,7 +282,13 @@ function updateSettingsFile($config_vars)
 		// Look through the variables to set....
 		foreach ($config_vars as $var => $val)
 		{
-			if (strncasecmp($settingsArray[$i], '$' . $var, 1 + strlen($var)) == 0)
+			// be sure someone is not updating db_last_error this with a group
+			if ($var === 'db_last_error')
+			{
+				updateDbLastError($val);
+				unset($config_vars[$var]);
+			}
+			elseif (strncasecmp($settingsArray[$i], '$' . $var, 1 + strlen($var)) == 0)
 			{
 				$comment = strstr(substr($settingsArray[$i], strpos($settingsArray[$i], ';')), '#');
 				$settingsArray[$i] = '$' . $var . ' = ' . $val . ';' . ($comment == '' ? '' : "\t\t" . rtrim($comment)) . "\n";
@@ -287,6 +298,7 @@ function updateSettingsFile($config_vars)
 			}
 		}
 
+		// End of the file ... maybe
 		if (substr(trim($settingsArray[$i]), 0, 2) == '?' . '>')
 			$end = $i;
 	}
@@ -295,16 +307,18 @@ function updateSettingsFile($config_vars)
 	if (empty($end) || $end < 10)
 		$end = count($settingsArray) - 1;
 
-	// Still more?  Add them at the end.
+	// Still more variables to go?  Then lets add them at the end.
 	if (!empty($config_vars))
 	{
 		if (trim($settingsArray[$end]) == '?' . '>')
 			$settingsArray[$end++] = '';
 		else
 			$end++;
-
+		
+		// Add in any newly defined vars that were passed
 		foreach ($config_vars as $var => $val)
 			$settingsArray[$end++] = '$' . $var . ' = ' . $val . ';' . "\n";
+		
 		$settingsArray[$end] = '?' . '>';
 	}
 	else
@@ -315,24 +329,22 @@ function updateSettingsFile($config_vars)
 		return;
 
 	// Try to avoid a few pitfalls:
-	// like a possible race condition,
-	// or a failure to write at low diskspace
-
-	// Check before you act: if cache is enabled, we can do a simple test
-	// Can we even write things on this filesystem?
+	//  - like a possible race condition,
+	//  - or a failure to write at low diskspace
+	//
+	// Check before you act: if cache is enabled, we can do a simple write test
+	// to validate that we even write things on this filesystem.
 	if ((empty($cachedir) || !file_exists($cachedir)) && file_exists($boarddir . '/cache'))
 		$cachedir = $boarddir . '/cache';
+	
 	$test_fp = @fopen($cachedir . '/settings_update.tmp', "w+");
 	if ($test_fp)
 	{
 		fclose($test_fp);
-
-		$test_fp = @fopen($cachedir . '/settings_update.tmp', 'r+');
-		$written_bytes = fwrite($test_fp, "test");
-		fclose($test_fp);
+		$written_bytes = file_put_contents($cachedir . '/settings_update.tmp', 'test', LOCK_EX);
 		@unlink($cachedir . '/settings_update.tmp');
 
-		if ($written_bytes !== strlen("test"))
+		if ($written_bytes !== 4)
 		{
 			// Oops. Low disk space, perhaps. Don't mess with Settings.php then.
 			// No means no. :P
@@ -344,23 +356,39 @@ function updateSettingsFile($config_vars)
 	clearstatcache();
 	if (filemtime($boarddir . '/Settings.php') === $last_settings_change)
 	{
-		// You asked for it...
-		// Blank out the file - done to fix a oddity with some servers.
-		$fp = @fopen($boarddir . '/Settings.php', 'w');
-
-		// Is it even writable, though?
-		if ($fp)
+		// save the old before we do anything
+		$settings_backup_fail = !@is_writable($boarddir . '/Settings_bak.php') || !@copy($boarddir . '/Settings.php', $boarddir . '/Settings_bak.php');
+
+		// write out the new
+		$write_settings = implode('', $settingsArray);
+		$written_bytes = file_put_contents($boarddir . '/Settings.php', $write_settings, LOCK_EX);
+		
+		// survey says ...
+		if ($written_bytes !== strlen($write_settings) && !$settings_backup_fail)
 		{
-			fclose($fp);
+			// Well this is not good at all, lets see if we can save this
+			$context['settings_message'] = 'settings_error';
 
-			$fp = fopen($boarddir . '/Settings.php', 'r+');
-			foreach ($settingsArray as $line)
-				fwrite($fp, strtr($line, "\r", ''));
-			fclose($fp);
+			if (file_exists($boarddir . '/Settings_bak.php'))
+				@copy($boarddir . '/Settings_bak.php', $boarddir . '/Settings.php');
 		}
 	}
 }
 
+/**
+ * Saves the time of the last db error for the error log
+ * - Done separately from updateSettingsFile to avoid race conditions 
+ *   which can occur during a db error
+ * - If it fails Settings.php will assume 0
+ */
+function updateDbLastError($time) 
+{
+	global $boarddir; 
+	
+	// Write out the db_last_error file with the error timestamp 
+	file_put_contents($boarddir . '/db_last_error.php', "<?php\n$db_last_error = " . $time . ";\n?" . ">\n", LOCK_EX);
+	@touch($boarddir . '/' . 'Settings.php');
+}
 /**
  * Saves the admins current preferences to the database.
  */
@@ -400,9 +428,9 @@ function updateAdminPreferences()
 
 /**
  * Send all the administrators a lovely email.
- * loads all users who are admins or have the admin forum permission.
- * uses the email template and replacements passed in the parameters.
- * sends them an email.
+ * - loads all users who are admins or have the admin forum permission.
+ * - uses the email template and replacements passed in the parameters.
+ * - sends them an email.
  */
 function emailAdmins($template, $replacements = array(), $additional_recipients = array())
 {

Themes/default/Admin.template.php

@@ -781,7 +781,7 @@ function template_show_settings()
 				</h3>
 			</div>';
 
-	// Have we got some custom code to insert?
+	// Have we got a message to display?
 	if (!empty($context['settings_message']))
 		echo '
 			<div class="information">', $context['settings_message'], '</div>';

Themes/default/languages/ManageSettings.english.php

@@ -98,6 +98,8 @@ $txt['who_enabled'] = 'Enable who\'s online list';
 $txt['make_email_viewable'] = 'Allow viewable email addresses';
 $txt['meta_keywords'] = 'Meta keywords associated with forum';
 $txt['meta_keywords_note'] = 'For search engines. Leave blank for default.';
+$txt['settings_error'] = 'Warning: Updating of Settings.php failed, the settings cannot be saved.';
+$txt['core_settings_saved'] = 'The settings were successfully saved';
 
 $txt['karmaMode'] = 'Karma mode';
 $txt['karma_options'] = 'Disable karma|Enable karma total|Enable karma positive/negative';

other/Settings.php

@@ -156,7 +156,15 @@ $sourcedir = dirname(__FILE__) . '/Sources';
 
 ########## Error-Catching ##########
 # Note: You shouldn't touch these settings.
-$db_last_error = 0;
+if (file_exists(dirname(__FILE__) . '/db_last_error.php')); 
+	include(dirname(__FILE__) . '/db_last_error.php'); 
+
+if (!isset($db_last_error))
+{
+	// File does not exist so lets try to create it
+	updateDbLastError(0); 
+	$db_last_error = 0; 
+}
 
 if (file_exists(dirname(__FILE__) . '/install.php'))
 {

other/install.php

@@ -1966,6 +1966,14 @@ function updateSettingsFile($vars)
 	return true;
 }
 
+function updateDbLastError() 
+{
+	// Write out the db_last_error file with the error timestamp 
+	file_put_contents(dirname(__FILE__) . '/db_last_error.php', "<?php\n$db_last_error = 0;\n?" . ">\n");
+	
+	return true;
+}
+
 // Create an .htaccess file to prevent mod_security. SMF has filtering built-in.
 function fixModSecurity()
 {

other/upgrade.php

@@ -689,6 +689,7 @@ function upgradeExit($fallThrough = false)
 		$upgradeData = base64_encode(serialize($upcontext['user']));
 		copy($boarddir . '/Settings.php', $boarddir . '/Settings_bak.php');
 		changeSettings(array('upgradeData' => '"' . $upgradeData . '"'));
+		updateLastError();
 	}
 
 	// Handle the progress of the step, if any.
@@ -2167,6 +2168,11 @@ function changeSettings($config_vars)
 	fwrite($fp, rtrim($settingsArray[$i]));
 	fclose($fp);
 }
+function updateLastError() 
+{
+	// clear out the db_last_error file
+	file_put_contents(dirname(__FILE__) . '/db_last_error.php', "<?php\n$db_last_error = 0;\n?" . ">\n");
+}
 
 function php_version_check()
 {