[3.0]: Config::updateModSettings() never removes a setting passed as null

[albertlast]

Basic Information

Config::updateModSettings() never removes a setting. Passing null as the value is the documented way to delete one, and every caller that does so silently leaves the row in place.

Sources/Config.php:1293-1304:

// 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);

// Proceed with the deletion.
if (!empty($to_remove)) {
    Db\DatabaseApi::$db->query(
        'DELETE FROM {db_prefix}settings
        WHERE variable IN ({array_string:remove})',
        [
            'remove' => $to_remove,
        ],
    );
}

array_filter() keeps the keys, so $to_remove is ['some_setting' => null, ...] — the names are the keys and every value is null. {array_string:remove} binds the values, so the statement that reaches the database is:

DELETE FROM smf_settings WHERE variable IN ('')

It matches nothing, and nothing is deleted. array_keys($to_remove) is what the placeholder wants.

The removal half of $change_array is filtered out at line 1294, too, so the setting is not written either — a caller passing null gets neither the delete it asked for nor an update, and no error.

Where it shows. v2_1\SettingsUpdate is the largest caller: it lists 22 settings in $removedSettings, sets each to null, and hands them to Config::updateModSettings(). None of them are removed. Upgrading a 2.1 forum that carries time_offset leaves that setting in smf_settings afterwards, which is how I noticed.

time_offset is the visible one because a 2.1 install writes none of the other 21 — they are 2.0-era settings, so a 2.0 → 3.0 upgrade is where the rest of them survive.

Steps to reproduce

1. From any SMF 3.0 install, write a setting and then ask for it back:

Config::updateModSettings(['probe_removeme' => 'x']);
Config::updateModSettings(['probe_removeme' => null]);

2. SELECT value FROM smf_settings WHERE variable = 'probe_removeme';

Or, on the upgrade path: upgrade a 2.1 database whose smf_settings contains time_offset, then look for time_offset in smf_settings afterwards.

Expected result

The row is gone.

Actual result

The row is still there, with its original value.

after writing it:  present ('x')
after nulling it:  present ('x')
the bound array:   array (
  'probe_removeme' => NULL,
)
the statement:     DELETE FROM `smf`.smf_settings WHERE variable IN ('')

Version/Git revision

3.0 Alpha 4, release-3.0 at 7a934c8

Database Engine

All

Database Version

Reproduced on MySQL 8.4. The defect is in Config.php, which is shared, so nothing about it is engine-specific.

PHP Version

8.4.24

Logs

Nothing is logged. The query is valid SQL and succeeds, having matched no rows.

Additional Information

Found while widening the time zone coverage of the 2.1 upgrade baseline in #9330, which is what put time_offset in front of that migration in the first place.