2.0.0: lower-priority fixes (canonicalization lock, installer script, settings save tests)

CHANGELOG

@@ -1,7 +1,9 @@
 2.0.0 (2026-08-24)
   - Migrate to Joomla 5/6: PSR-4 namespaced classes (Codeling\Plugin\System\Bfstop),
     service provider, SubscriberInterface event handling
-  - Drop Joomla 3/4 compatibility (requires Joomla 5 or 6)
+  - Drop Joomla 3/4 compatibility (requires Joomla 5 or 6 and PHP 8.1); the
+    installer refuses older Joomla and PHP versions, and replacing a newer
+    version of BFStop with an older one
   - Modernize token generation to use random_bytes()/random_int()
   - Consolidate all configuration into the component's Settings view
     (Components -> Brute Force Stop -> Settings); the plugin's Options tab
@@ -180,7 +182,9 @@
     purge age to 0 for the old behaviour) and limited to 200000 rows
   - Security: a response is delayed by 60 seconds at most, however the delay
     settings add up, and the account throttle no longer delays an address
-    that was just blocked
+    that was just blocked. "Delay Duration" and "Maximum Adaptive Delay" go
+    up to 60 seconds (before: 120); a larger value saved by an earlier
+    version has to be lowered the next time the settings are saved
   - Security: unblock tokens are stored as hashes; links sent before the
     update stop working (they are valid for 3 days only)
   - Security: usernames in log entries and in mails can no longer contain

src/Extension/Bfstop.php

@@ -39,6 +39,13 @@ class Bfstop extends CMSPlugin implements SubscriberInterface
 	 * compares them). The plugin is enabled by the installation, so it runs
 	 * with these until an administrator opens the settings.
 	 */
+	/**
+	 * Values the plugin keeps among its settings for itself: they are not on
+	 * the settings page, so saving that page must keep them (see com_bfstop's
+	 * SettingsModel).
+	 */
+	public const InternalSettings = array('addressesCanonical', 'lastPurge');
+
 	public const DefaultSettings = array(
 		'blockNumber' => 10,
 		'checkInterval' => 10080,
@@ -791,9 +798,7 @@ public function onAfterInitialise($event)
 		// version may be loaded still
 		if (!$this->getBoolParam('addressesCanonical', false))
 		{
-			$this->mydb->canonicalizeStoredAddresses();
-			$this->mydb->savePluginSetting('addressesCanonical', 1);
-			$this->params->set('addressesCanonical', 1);
+			$this->canonicalizeStoredAddressesOnce();
 		}
 		// periodic maintenance, at most once a day
 		$purgeInterval = 86400; // = 24*60*60 => one day
@@ -821,6 +826,51 @@ public function onAfterInitialise($event)
 		}
 	}
 
+	/**
+	 * Brings the stored addresses into canonical form, if no other request
+	 * did or does: every request starts out with the settings as they were
+	 * when it began, so after the update all requests arriving before the
+	 * first of them is done would see the setting unset, and each would go
+	 * through the whole block and allow list (and update the same rows) - on
+	 * a busy site with a long list, many at once. The first takes a lock; the
+	 * others go on without waiting (their lookups may miss an entry which
+	 * isn't converted yet, as they would have before the update). Once it has
+	 * the lock, a request checks the stored setting again, as another one may
+	 * have finished in the meantime. If no lock can be taken (no writable
+	 * temporary directory), it is done without one: it can be run more than
+	 * once, it only costs time.
+	 */
+	private function canonicalizeStoredAddressesOnce()
+	{
+		$lockFile = HtaccessHelper::lockDirectory().'/bfstop-canonicalize-'.
+			md5((string) $this->getApplication()->get('db').(string) $this->getApplication()->get('dbprefix')).'.lock';
+		$handle = @fopen($lockFile, 'c');
+		if ($handle !== false && !flock($handle, LOCK_EX | LOCK_NB))
+		{
+			fclose($handle);
+			$this->logger->log('Another request is converting the stored addresses, not waiting for it', Log::DEBUG);
+			return;
+		}
+		try
+		{
+			if (!$this->mydb->getPluginSetting('addressesCanonical', false))
+			{
+				$changed = $this->mydb->canonicalizeStoredAddresses();
+				$this->mydb->savePluginSetting('addressesCanonical', 1);
+				$this->logger->log('Converted '.$changed.' stored address(es) to canonical form', Log::INFO);
+			}
+			$this->params->set('addressesCanonical', 1);
+		}
+		finally
+		{
+			if ($handle !== false)
+			{
+				flock($handle, LOCK_UN);
+				fclose($handle);
+			}
+		}
+	}
+
 	/**
 	 * The status code of the page shown to a blocked client, or null for none
 	 * (which makes it 200). Set with http_response_code(), not as a raw

src/Helper/DatabaseHelper.php

@@ -915,6 +915,31 @@ public function saveLastPurge($timestamp)
 	 * request was started with would undo whatever an administrator saved in
 	 * the meantime.
 	 */
+	/**
+	 * A setting as it is stored now - the plugin's $params are what was stored
+	 * when the request began.
+	 */
+	public function getPluginSetting($name, $default = null)
+	{
+		try
+		{
+			$query = $this->db->getQuery(true)
+				->select($this->db->quoteName('params'))
+				->from($this->db->quoteName('#__extensions'))
+				->where($this->db->quoteName('type').' = '.$this->db->quote('plugin'))
+				->where($this->db->quoteName('folder').' = '.$this->db->quote('system'))
+				->where($this->db->quoteName('element').' = '.$this->db->quote('bfstop'));
+			$this->db->setQuery($query);
+			$params = json_decode((string) $this->db->loadResult(), true);
+			return (is_array($params) && array_key_exists($name, $params)) ? $params[$name] : $default;
+		}
+		catch (\Exception $e)
+		{
+			$this->logger->log("Database exception occured: ".$e->getMessage(), Log::ERROR);
+			return $default;
+		}
+	}
+
 	public function savePluginSetting($name, $value)
 	{
 		try

src/Helper/HtaccessHelper.php

@@ -223,7 +223,7 @@ public function remove403Message()
 	 */
 	private function locked(callable $change)
 	{
-		$lockFile = $this->lockDirectory().'/bfstop-htaccess-'.md5($this->path).'.lock';
+		$lockFile = self::lockDirectory().'/bfstop-htaccess-'.md5($this->path).'.lock';
 		$handle = @fopen($lockFile, 'c');
 		if ($handle === false || !flock($handle, LOCK_EX))
 		{
@@ -249,11 +249,12 @@ private function locked(callable $change)
 	}
 
 	/**
-	 * Joomla's own temporary directory if there is one we can use, not the
-	 * system's: that one is shared with other users of the machine on many
-	 * hosts, who could then hold the lock (or plant a link under its name).
+	 * Where lock files go (also those of the plugin, see Bfstop): Joomla's own
+	 * temporary directory if there is one we can use, not the system's: that
+	 * one is shared with other users of the machine on many hosts, who could
+	 * then hold the lock (or plant a link under its name).
 	 */
-	private function lockDirectory()
+	public static function lockDirectory()
 	{
 		try
 		{

tests/Integration/BlockedRequestTest.php

@@ -92,6 +92,34 @@ public function testFullModeBlocksEverything()
 		$this->assertNotBlocked('', '203.0.113.42');
 	}
 
+	public function testStoredAddressesAreConvertedByOneRequestAtATime()
+	{
+		// after the update, the first request converts the stored addresses;
+		// the ones arriving while it is at it must not all do the same
+		$this->configure();
+		$id = $this->block('2001:DB8::41');
+		$app = Factory::getApplication();
+		$lockFile = HtaccessHelper::lockDirectory().'/bfstop-canonicalize-'.
+			md5((string) $app->get('db').(string) $app->get('dbprefix')).'.lock';
+		$handle = fopen($lockFile, 'c');
+		$this->assertTrue(flock($handle, LOCK_EX), 'the test takes the lock of the "first request"');
+		try
+		{
+			$this->request('', '203.0.113.99');
+			$this->assertSame('2001:DB8::41', $this->queryValue('SELECT ipaddress FROM #__bfstop_bannedip WHERE id='.$id),
+				'converted although another request holds the lock');
+			$this->assertArrayNotHasKey('addressesCanonical', json_decode($this->getPluginParams(), true));
+		}
+		finally
+		{
+			flock($handle, LOCK_UN);
+			fclose($handle);
+		}
+		$this->request('', '203.0.113.99');
+		$this->assertSame('2001:db8::41', $this->queryValue('SELECT ipaddress FROM #__bfstop_bannedip WHERE id='.$id));
+		$this->assertSame(1, json_decode($this->getPluginParams(), true)['addressesCanonical']);
+	}
+
 	public function testBlockIsEnforcedOnlyAfterRouting()
 	{
 		// with SEF URLs option/view are empty until the router ran, so the

tests/Integration/ComponentTest.php

@@ -15,6 +15,7 @@
 use Codeling\Component\Bfstop\Administrator\Model\BlockModel;
 use Codeling\Component\Bfstop\Administrator\Model\BlocklistModel;
 use Codeling\Component\Bfstop\Administrator\Model\FailedloginlistModel;
+use Codeling\Component\Bfstop\Administrator\Model\SettingsModel;
 use Codeling\Component\Bfstop\Administrator\Model\UsernamestatsModel;
 use Codeling\Component\Bfstop\Site\Model\TokenunblockModel;
 use Codeling\Plugin\System\Bfstop\Helper\DatabaseHelper;
@@ -394,6 +395,72 @@ public function testSettingsAndLogViewsNeedTheAdminPermission()
 		}
 	}
 
+	public function testSavingTheSettingsKeepsWhatThePluginStoresForItself()
+	{
+		// the form only has the settings; the plugin's own values must survive
+		// a save, or it would e.g. convert all stored addresses again
+		$original = $this->getPluginParams();
+		try
+		{
+			$this->setPluginParams(json_encode(array('blockNumber' => 7, 'lastPurge' => 1700000000,
+				'addressesCanonical' => 1)));
+			$model = $this->model(SettingsModel::class);
+			$model->getState();
+			$this->assertTrue($model->save(array('params' => array('blockNumber' => 8, 'logLevel' => 8))),
+				implode(', ', $model->getErrors()));
+			$saved = json_decode($this->getPluginParams(), true);
+			$this->assertSame(8, (int) $saved['blockNumber']);
+			$this->assertSame(1700000000, $saved['lastPurge']);
+			$this->assertSame(1, $saved['addressesCanonical']);
+			$this->assertSame(\Codeling\Plugin\System\Bfstop\Extension\Bfstop::InternalSettings,
+				array_values(array_intersect(array_keys($saved), \Codeling\Plugin\System\Bfstop\Extension\Bfstop::InternalSettings)));
+			// a value given in the data wins (not that the form has them)
+			$this->assertSame(array('lastPurge' => 5), SettingsModel::keepInternalSettings(array('lastPurge' => 5),
+				array('lastPurge' => 1700000000)));
+		}
+		finally
+		{
+			$this->setPluginParams($original);
+		}
+	}
+
+	public function testInstallerScriptHintAndVersionCheck()
+	{
+		require_once getenv('COM_BFSTOP_ROOT').'/installscript.php';
+		$app = \Joomla\CMS\Factory::getApplication();
+		$adapter = $this->createMock(\Joomla\CMS\Installer\InstallerAdapter::class);
+		$adapter->method('getName')->willReturn('com_bfstop');
+		$adapter->method('getManifest')->willReturn(simplexml_load_file(getenv('COM_BFSTOP_ROOT').'/bfstop.xml'));
+		$script = new \com_bfstopInstallerScript($adapter);
+		$app->getMessageQueue(true);
+
+		// the hint about the settings is for new installations; on an update,
+		// the plugin's script says what to look at, if anything
+		$script->postflight('update', $adapter);
+		$this->assertSame(array(), $app->getMessageQueue(true));
+		$script->postflight('install', $adapter);
+		$this->assertCount(1, $app->getMessageQueue(true));
+
+		$cache = $this->queryValue("SELECT manifest_cache FROM #__extensions WHERE type='component' AND element='com_bfstop'");
+		try
+		{
+			$this->assertTrue($script->preflight('update', $adapter));
+			$newer = json_decode((string) $cache, true) ?: array();
+			$newer['version'] = '99.0.0';
+			$this->db->setQuery('UPDATE #__extensions SET manifest_cache='.$this->db->quote(json_encode($newer)).
+				" WHERE type='component' AND element='com_bfstop'");
+			$this->db->execute();
+			$this->assertFalse($script->preflight('update', $adapter), 'replaced a newer version');
+		}
+		finally
+		{
+			$this->db->setQuery('UPDATE #__extensions SET manifest_cache='.$this->db->quote($cache).
+				" WHERE type='component' AND element='com_bfstop'");
+			$this->db->execute();
+			$app->getMessageQueue(true);
+		}
+	}
+
 	public function testUnsavedSettingsAreWhatTheSettingsPageShows()
 	{
 		// the plugin is enabled by the installation: until the settings are

tests/Integration/InstallerScriptTest.php

@@ -0,0 +1,125 @@
+<?php
+/*
+ * @package BFStop Plugin (bfstop) for Joomla!
+ * @author Bernhard Froehler
+ * @copyright (C) Bernhard Froehler
+ * @license GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
+**/
+namespace Codeling\Bfstop\Tests\Integration;
+
+use Joomla\CMS\Factory;
+use Joomla\CMS\Installer\InstallerAdapter;
+
+/**
+ * The plugin's installer script (updatescript.php), called the way Joomla's
+ * installer calls it, against the installed plugin's row: what it does
+ * depends on the version being replaced, which is in its manifest cache.
+ */
+class InstallerScriptTest extends IntegrationTestCase
+{
+	private $originalManifestCache;
+
+	protected function setUp(): void
+	{
+		parent::setUp();
+		require_once dirname(__DIR__, 2).'/updatescript.php';
+		$this->originalManifestCache = $this->queryValue(
+			"SELECT manifest_cache FROM #__extensions WHERE type='plugin' AND element='bfstop'");
+		Factory::getApplication()->getMessageQueue(true);
+	}
+
+	protected function tearDown(): void
+	{
+		$this->setInstalledVersion(null);
+		Factory::getApplication()->getMessageQueue(true);
+	}
+
+	private function setInstalledVersion($version)
+	{
+		$cache = $this->originalManifestCache;
+		if ($version !== null)
+		{
+			$decoded = json_decode((string) $cache, true) ?: array();
+			$decoded['version'] = $version;
+			$cache = json_encode($decoded);
+		}
+		$this->db->setQuery('UPDATE #__extensions SET manifest_cache='.$this->db->quote($cache).
+			" WHERE type='plugin' AND element='bfstop'");
+		$this->db->execute();
+	}
+
+	private function script()
+	{
+		$adapter = $this->createMock(InstallerAdapter::class);
+		$adapter->method('getName')->willReturn('plg_system_bfstop');
+		$adapter->method('getManifest')->willReturn(simplexml_load_file(dirname(__DIR__, 2).'/bfstop.xml'));
+		return array(new \PlgsystembfstopInstallerScript($adapter), $adapter);
+	}
+
+	private function messages()
+	{
+		return array_map(function ($message) { return $message['message']; },
+			Factory::getApplication()->getMessageQueue(true));
+	}
+
+	private function updateHintShown($fromVersion)
+	{
+		$this->setInstalledVersion($fromVersion);
+		list($script, $adapter) = $this->script();
+		$this->assertTrue($script->preflight('update', $adapter));
+		$script->postflight('update', $adapter);
+		$hint = strip_tags(\Joomla\CMS\Language\Text::sprintf('PLG_SYSTEM_BFSTOP_UPDATE_2_0_0_HINT', ''));
+		foreach ($this->messages() as $message)
+		{
+			if (strip_tags($message) === $hint)
+			{
+				return true;
+			}
+		}
+		return false;
+	}
+
+	public function testSettingsMovedHintOnlyForUpdatesFromBefore200()
+	{
+		Factory::getApplication()->getLanguage()->load('plg_system_bfstop', JPATH_ADMINISTRATOR);
+		$this->assertTrue($this->updateHintShown('1.5.2'));
+		$this->assertFalse($this->updateHintShown('2.0.0'), 'the settings did not move again');
+	}
+
+	public function testRefusesToReplaceANewerVersion()
+	{
+		$this->setInstalledVersion('99.0.0');
+		list($script, $adapter) = $this->script();
+		$this->assertFalse($script->preflight('update', $adapter));
+	}
+
+	public function testWhitelistTableIsRenamedBeforeTheUpdateSql()
+	{
+		// new installations of 1.4.2 kept the old table name; the update SQL
+		// of 2.0.0 (run between preflight and update) changes the allowlist
+		if ($this->db->getServerType() !== 'mysql')
+		{
+			$this->markTestSkipped('only MySQL installations can be that old');
+		}
+		$this->db->setQuery('RENAME TABLE #__bfstop_allowlist TO #__bfstop_whitelist');
+		$this->db->execute();
+		try
+		{
+			$this->setInstalledVersion('1.4.2');
+			list($script, $adapter) = $this->script();
+			$this->assertTrue($script->preflight('update', $adapter));
+			$tables = array_map('strtolower', $this->db->getTableList());
+			$this->assertContains(strtolower($this->db->getPrefix().'bfstop_allowlist'), $tables);
+			$this->assertNotContains(strtolower($this->db->getPrefix().'bfstop_whitelist'), $tables);
+		}
+		finally
+		{
+			$tables = array_map('strtolower', $this->db->getTableList());
+			if (in_array(strtolower($this->db->getPrefix().'bfstop_whitelist'), $tables, true))
+			{
+				$this->db->setQuery('RENAME TABLE #__bfstop_whitelist TO #__bfstop_allowlist');
+				$this->db->execute();
+			}
+		}
+	}
+}