From 4a6f75f0f40fe2f566e28dec9146bd7b92d7b29f Mon Sep 17 00:00:00 2001 From: albertlast Date: Mon, 17 Aug 2026 00:26:32 +0200 Subject: [PATCH] Removes the settings that updateModSettings() is asked to remove Passing null as a value is how a setting is deleted, and it never deleted anything. array_filter() keeps the keys, so $to_remove is a list of names mapped to nulls, and {array_string:remove} binds the values -- so the statement that ran was: DELETE FROM smf_settings WHERE variable IN ('') which matches nothing and reports no error. The names are what the placeholder wants, and array_keys($change_array, null, true) is a shorter way of finding them than filtering and then taking the keys. Two more places had to lose them or the setting comes straight back. The copy in Config::$modSettings was never touched, so the removed setting was still readable for the rest of the request, while every other path through this method keeps that copy in step. And a call that only removes settings returns before either of the existing cache invalidations, so with caching on the next request read the setting back out of the cache. The callers are three migrations whose entire job is to drop a setting -- v2_1\SettingsUpdate with 22 of them, v3_0\RemoveCookieTime, and v3_0\LanguageDirectory renaming one -- plus Search\APIs\Parsed clearing its status. None of them have ever worked. Upgrading a 2.1 forum leaves time_offset behind in smf_settings, which is what led here. Fixes #9530 Signed-off-by: albertlast --- Sources/Config.php | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/Sources/Config.php b/Sources/Config.php index f2082e4f8f..501e1afb08 100644 --- a/Sources/Config.php +++ b/Sources/Config.php @@ -1289,9 +1289,12 @@ public static function updateModSettings(array $change_array, bool $update = fal return; } - // Go check if there is any setting to be removed. - $to_remove = array_filter($change_array, fn($setting) => $setting === null); - $change_array = array_diff_key($change_array, $to_remove); + // Go check if there is any setting to be removed. The names are what we + // want here, not the nulls they are set to: {array_string:remove} + // binds the values, so handing it the whole array asks the database to + // delete every setting called ''. + $to_remove = array_keys($change_array, null, true); + $change_array = array_diff_key($change_array, array_flip($to_remove)); // Proceed with the deletion. if (!empty($to_remove)) { @@ -1302,6 +1305,15 @@ public static function updateModSettings(array $change_array, bool $update = fal 'remove' => $to_remove, ], ); + + // The copy we are holding and the cached one both have to lose them + // as well. Otherwise the setting is still here for the rest of this + // request, and back again on the next one. + foreach ($to_remove as $variable) { + unset(self::$modSettings[$variable]); + } + + Cache\CacheApi::put('modSettings', null, 90); } // In some cases, this may be better and faster, but for large sets we don't want so many UPDATEs.