From bf19db5f9a69c0ca2746d980ee48b28f1b0752b4 Mon Sep 17 00:00:00 2001 From: svfcode Date: Sun, 9 Aug 2026 15:14:40 +0300 Subject: [PATCH 1/2] Upd. FW. Improve statement to check update --- inc/fw-update.php | 9 +- inc/spbc-firewall.php | 5 +- lib/CleantalkSP/SpbctWP/Firewall/FW.php | 5 + .../SpbctWP/Firewall/FirewallUpdateLock.php | 190 ++++++++++++++++++ .../Firewall/FirewallUpdateLockTest.php | 77 +++++++ 5 files changed, 283 insertions(+), 3 deletions(-) create mode 100644 lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLock.php create mode 100644 tests/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLockTest.php diff --git a/inc/fw-update.php b/inc/fw-update.php index a7d03fc5e..9e3218677 100644 --- a/inc/fw-update.php +++ b/inc/fw-update.php @@ -3,6 +3,7 @@ use CleantalkSP\Common\DNS; use CleantalkSP\SpbctWP\DB; use CleantalkSP\SpbctWP\Firewall\FW; +use CleantalkSP\SpbctWP\Firewall\FirewallUpdateLock; use CleantalkSP\SpbctWP\Cron; use CleantalkSP\SpbctWP\Queue; use CleantalkSP\Variables\Request; @@ -638,10 +639,12 @@ function spbc_security_firewall_update__end_of_update() $spbc->update_logger->writeLog('STAGE: END OF UPDATE START'); - // Put in maintenance mode + // Put in maintenance mode + cross-request lock (re-read by FW checks; not only in-memory $spbc) + FirewallUpdateLock::set(); $spbc->fw_stats['is_on_maintenance'] = true; $spbc->save('fw_stats', true, false); - usleep(100000); + // Give in-flight requests a moment to finish before DROP + usleep(200000); //Increment firewall entries @@ -661,6 +664,7 @@ function spbc_security_firewall_update__end_of_update() } } if ( ! empty($result['error']) ) { + FirewallUpdateLock::clear(false); $spbc->fw_stats['is_on_maintenance'] = false; $spbc->save('fw_stats', true, false); $spbc->update_logger->writeLog('STAGE: END OF UPDATE ERROR', $result['error']); @@ -674,6 +678,7 @@ function spbc_security_firewall_update__end_of_update() $spbc->fw_stats['updating_last_start'] = 0; $spbc->fw_stats['last_updated'] = current_time('timestamp'); $spbc->fw_stats['is_on_maintenance'] = false; // Remove maintenance mode + FirewallUpdateLock::clear(); $sql_count_networks = "SELECT SUM(cnt) FROM ( SELECT COUNT(*) as cnt FROM " . SPBC_TBL_FIREWALL_DATA_V4 . " diff --git a/inc/spbc-firewall.php b/inc/spbc-firewall.php index cc272ed63..fe0c3809d 100644 --- a/inc/spbc-firewall.php +++ b/inc/spbc-firewall.php @@ -8,6 +8,7 @@ use CleantalkSP\SpbctWP\Firewall; use CleantalkSP\SpbctWP\Firewall\BFP; use CleantalkSP\SpbctWP\Firewall\FW; +use CleantalkSP\SpbctWP\Firewall\FirewallUpdateLock; use CleantalkSP\SpbctWP\Firewall\TC; use CleantalkSP\SpbctWP\Firewall\WAF; use CleantalkSP\SpbctWP\Firewall\WafBlocker; @@ -236,7 +237,9 @@ function spbc_firewall_skip_check() global $spbc, $apbct; // General skip - if ( $spbc->fw_stats['is_on_maintenance'] + // FirewallUpdateLock::isLocked() re-reads transient/file — do not rely on in-memory $spbc->fw_stats alone + if ( FirewallUpdateLock::isLocked() + || $spbc->fw_stats['is_on_maintenance'] || ! $spbc->feature_restrictions->getState($spbc, 'firewall_log')->is_active || ! isset($spbc->fw_stats['last_updated'], $spbc->fw_stats['entries']) // Plugin's FW base is updated || CleantalkSP\SpbctWP\Firewall::isException() diff --git a/lib/CleantalkSP/SpbctWP/Firewall/FW.php b/lib/CleantalkSP/SpbctWP/Firewall/FW.php index c4f96eed4..4d8173deb 100644 --- a/lib/CleantalkSP/SpbctWP/Firewall/FW.php +++ b/lib/CleantalkSP/SpbctWP/Firewall/FW.php @@ -61,6 +61,11 @@ public function check() { $results = array(); + // Skip DB UNION while tables are swapped (lock) or missing — avoids wpdb "table doesn't exist" log spam + if ( FirewallUpdateLock::shouldSkipFwDbQueries($this->db) ) { + return $results; + } + foreach ( $this->ip_array as $_ip_origin => $current_ip ) { try { $version = IP::validate($current_ip); diff --git a/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLock.php b/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLock.php new file mode 100644 index 000000000..e83a78631 --- /dev/null +++ b/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLock.php @@ -0,0 +1,190 @@ +fw_stats): + * concurrent requests may have loaded State before maintenance started. + * + * Uses a WP transient (TTL safety net) and a filesystem flag (always fresh across workers). + */ +class FirewallUpdateLock +{ + const TRANSIENT_KEY = 'spbc_fw_update_lock'; + + /** + * Positive cache: FW data tables are present (deleted when lock is acquired). + */ + const TABLES_READY_KEY = 'spbc_fw_tables_ready'; + + /** + * Safety TTL if clear() never runs (fatal during rename, etc.). + */ + const TTL = 300; + + const FILE_NAME = 'spbc_fw_update.lock'; + + /** + * @var bool|null Request-level cache for table existence only. + * Lock is never cached — always re-read. + */ + private static $tables_exist_cache = null; + + /** + * Acquire lock before DROP/RENAME of FW data tables. + * + * @return void + */ + public static function set() + { + set_transient(self::TRANSIENT_KEY, time(), self::TTL); + delete_transient(self::TABLES_READY_KEY); + self::bustTransientCaches(); + + $path = self::filePath(); + if ( $path ) { + $dir = dirname($path); + if ( ! is_dir($dir) ) { + // phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.directory_mkdir + @mkdir($dir, 0755, true); + } + @file_put_contents($path, (string) time()); + } + + self::$tables_exist_cache = null; + } + + /** + * Release lock after tables are permanent again. + * + * @param bool $mark_tables_ready Set false on failed swap so callers re-check SHOW TABLES. + * + * @return void + */ + public static function clear($mark_tables_ready = true) + { + delete_transient(self::TRANSIENT_KEY); + + if ( $mark_tables_ready ) { + set_transient(self::TABLES_READY_KEY, 1, defined('DAY_IN_SECONDS') ? DAY_IN_SECONDS : 86400); + self::$tables_exist_cache = true; + } else { + delete_transient(self::TABLES_READY_KEY); + self::$tables_exist_cache = null; + } + + self::bustTransientCaches(); + + $path = self::filePath(); + if ( $path && is_file($path) ) { + @unlink($path); + } + } + + /** + * Fresh lock check for every FW skip/check call. + * + * @return bool + */ + public static function isLocked() + { + $path = self::filePath(); + if ( $path && is_file($path) ) { + return true; + } + + return (bool) get_transient(self::TRANSIENT_KEY); + } + + /** + * Whether FW list queries should be skipped (lock or missing data tables). + * + * @param DB|null $db + * + * @return bool + */ + public static function shouldSkipFwDbQueries($db = null) + { + if ( self::isLocked() ) { + return true; + } + + // Hot path: marker set after successful swap — no SHOW TABLES + if ( get_transient(self::TABLES_READY_KEY) ) { + return false; + } + + $exist = self::fwDataTablesExist($db); + if ( $exist ) { + set_transient(self::TABLES_READY_KEY, 1, defined('DAY_IN_SECONDS') ? DAY_IN_SECONDS : 86400); + } + + return ! $exist; + } + + /** + * True when tables used by ipv4/ipv6 FW UNION queries exist. + * + * @param DB|null $db + * + * @return bool + */ + public static function fwDataTablesExist($db = null) + { + if ( self::$tables_exist_cache !== null ) { + return self::$tables_exist_cache; + } + + if ( ! defined('SPBC_TBL_FIREWALL_DATA_V4') || ! defined('SPBC_TBL_FIREWALL_DATA__COUNTRIES') ) { + self::$tables_exist_cache = false; + + return false; + } + + $db = $db instanceof DB ? $db : DB::getInstance(); + + // During swap all FW data tables are dropped together — checking the two used in the reported UNION is enough. + self::$tables_exist_cache = $db->isTableExists(SPBC_TBL_FIREWALL_DATA_V4) + && $db->isTableExists(SPBC_TBL_FIREWALL_DATA__COUNTRIES); + + return self::$tables_exist_cache; + } + + /** + * @return string Absolute path to lock file, or empty string if uploads dir unavailable. + */ + public static function filePath() + { + if ( ! function_exists('wp_upload_dir') ) { + return ''; + } + + $upload = wp_upload_dir(); + if ( ! empty($upload['error']) || empty($upload['basedir']) ) { + return ''; + } + + return rtrim($upload['basedir'], '/\\') . DIRECTORY_SEPARATOR . self::FILE_NAME; + } + + /** + * @return void + */ + private static function bustTransientCaches() + { + if ( ! function_exists('wp_cache_delete') ) { + return; + } + + wp_cache_delete(self::TRANSIENT_KEY, 'transient'); + wp_cache_delete('_transient_' . self::TRANSIENT_KEY, 'options'); + wp_cache_delete('_transient_timeout_' . self::TRANSIENT_KEY, 'options'); + wp_cache_delete(self::TABLES_READY_KEY, 'transient'); + wp_cache_delete('_transient_' . self::TABLES_READY_KEY, 'options'); + wp_cache_delete('_transient_timeout_' . self::TABLES_READY_KEY, 'options'); + } +} diff --git a/tests/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLockTest.php b/tests/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLockTest.php new file mode 100644 index 000000000..8fca470cf --- /dev/null +++ b/tests/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLockTest.php @@ -0,0 +1,77 @@ +assertTrue(FirewallUpdateLock::isLocked()); + $this->assertNotEmpty(get_transient(FirewallUpdateLock::TRANSIENT_KEY)); + $path = FirewallUpdateLock::filePath(); + $this->assertNotEmpty($path); + $this->assertFileExists($path); + $this->assertFalse((bool) get_transient(FirewallUpdateLock::TABLES_READY_KEY)); + } + + public function testClearRemovesLockAndMarksTablesReady() + { + FirewallUpdateLock::set(); + FirewallUpdateLock::clear(); + + $this->assertFalse(FirewallUpdateLock::isLocked()); + $this->assertFalse((bool) get_transient(FirewallUpdateLock::TRANSIENT_KEY)); + $this->assertTrue((bool) get_transient(FirewallUpdateLock::TABLES_READY_KEY)); + $path = FirewallUpdateLock::filePath(); + if ( $path ) { + $this->assertFileNotExists($path); + } + } + + public function testClearWithoutReadyMarker() + { + FirewallUpdateLock::set(); + FirewallUpdateLock::clear(false); + + $this->assertFalse(FirewallUpdateLock::isLocked()); + $this->assertFalse((bool) get_transient(FirewallUpdateLock::TABLES_READY_KEY)); + } + + public function testShouldSkipWhenLocked() + { + set_transient(FirewallUpdateLock::TABLES_READY_KEY, 1, 3600); + FirewallUpdateLock::set(); + + $this->assertTrue(FirewallUpdateLock::shouldSkipFwDbQueries()); + } + + public function testShouldNotSkipWhenReadyMarkerSet() + { + FirewallUpdateLock::clear(); + + $this->assertFalse(FirewallUpdateLock::isLocked()); + $this->assertFalse(FirewallUpdateLock::shouldSkipFwDbQueries()); + } +} From 61a10e0546d77a514b031cf305bf6ac5d5ca023e Mon Sep 17 00:00:00 2001 From: svfcode Date: Sun, 9 Aug 2026 15:23:20 +0300 Subject: [PATCH 2/2] fix cp --- .../SpbctWP/Firewall/FirewallUpdateLock.php | 93 +++++++++++++++++-- .../Firewall/FirewallUpdateLockTest.php | 30 ++++++ 2 files changed, 114 insertions(+), 9 deletions(-) diff --git a/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLock.php b/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLock.php index e83a78631..091787fae 100644 --- a/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLock.php +++ b/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLock.php @@ -11,6 +11,7 @@ * concurrent requests may have loaded State before maintenance started. * * Uses a WP transient (TTL safety net) and a filesystem flag (always fresh across workers). + * Both expire after TTL so a fatal mid-swap cannot disable FW forever. */ class FirewallUpdateLock { @@ -41,7 +42,8 @@ class FirewallUpdateLock */ public static function set() { - set_transient(self::TRANSIENT_KEY, time(), self::TTL); + $now = time(); + set_transient(self::TRANSIENT_KEY, $now, self::TTL); delete_transient(self::TABLES_READY_KEY); self::bustTransientCaches(); @@ -52,7 +54,7 @@ public static function set() // phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.directory_mkdir @mkdir($dir, 0755, true); } - @file_put_contents($path, (string) time()); + @file_put_contents($path, (string) $now); } self::$tables_exist_cache = null; @@ -92,8 +94,7 @@ public static function clear($mark_tables_ready = true) */ public static function isLocked() { - $path = self::filePath(); - if ( $path && is_file($path) ) { + if ( self::isFileLockActive() ) { return true; } @@ -139,7 +140,8 @@ public static function fwDataTablesExist($db = null) return self::$tables_exist_cache; } - if ( ! defined('SPBC_TBL_FIREWALL_DATA_V4') || ! defined('SPBC_TBL_FIREWALL_DATA__COUNTRIES') ) { + $tables = self::requiredFwDataTables(); + if ( empty($tables) ) { self::$tables_exist_cache = false; return false; @@ -147,11 +149,43 @@ public static function fwDataTablesExist($db = null) $db = $db instanceof DB ? $db : DB::getInstance(); - // During swap all FW data tables are dropped together — checking the two used in the reported UNION is enough. - self::$tables_exist_cache = $db->isTableExists(SPBC_TBL_FIREWALL_DATA_V4) - && $db->isTableExists(SPBC_TBL_FIREWALL_DATA__COUNTRIES); + foreach ( $tables as $table ) { + if ( ! $db->isTableExists($table) ) { + self::$tables_exist_cache = false; + + return false; + } + } + + self::$tables_exist_cache = true; - return self::$tables_exist_cache; + return true; + } + + /** + * Tables swapped in end_of_update / used by FW UNION queries. + * + * @return string[] + */ + public static function requiredFwDataTables() + { + $constants = array( + 'SPBC_TBL_FIREWALL_DATA_V4', + 'SPBC_TBL_FIREWALL_DATA_V6', + 'SPBC_TBL_FIREWALL_DATA__IPS_V4', + 'SPBC_TBL_FIREWALL_DATA__IPS_V6', + 'SPBC_TBL_FIREWALL_DATA__COUNTRIES', + ); + + $tables = array(); + foreach ( $constants as $constant ) { + if ( ! defined($constant) ) { + return array(); + } + $tables[] = constant($constant); + } + + return $tables; } /** @@ -171,6 +205,47 @@ public static function filePath() return rtrim($upload['basedir'], '/\\') . DIRECTORY_SEPARATOR . self::FILE_NAME; } + /** + * File lock with the same TTL as the transient — stale files are removed. + * + * @return bool + */ + private static function isFileLockActive() + { + $path = self::filePath(); + if ( ! $path || ! is_file($path) ) { + return false; + } + + $started_at = self::readLockTimestamp($path); + if ( $started_at === null || ( time() - $started_at ) > self::TTL ) { + @unlink($path); + + return false; + } + + return true; + } + + /** + * Prefer timestamp written by set(); fall back to mtime. + * + * @param string $path + * + * @return int|null + */ + private static function readLockTimestamp($path) + { + $raw = @file_get_contents($path); + if ( is_string($raw) && preg_match('/^\d+$/', trim($raw)) ) { + return (int) trim($raw); + } + + $mtime = @filemtime($path); + + return $mtime ? (int) $mtime : null; + } + /** * @return void */ diff --git a/tests/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLockTest.php b/tests/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLockTest.php index 8fca470cf..caf776665 100644 --- a/tests/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLockTest.php +++ b/tests/lib/CleantalkSP/SpbctWP/Firewall/FirewallUpdateLockTest.php @@ -74,4 +74,34 @@ public function testShouldNotSkipWhenReadyMarkerSet() $this->assertFalse(FirewallUpdateLock::isLocked()); $this->assertFalse(FirewallUpdateLock::shouldSkipFwDbQueries()); } + + public function testStaleFileLockIsIgnoredAndRemoved() + { + $path = FirewallUpdateLock::filePath(); + $this->assertNotEmpty($path); + + $dir = dirname($path); + if ( ! is_dir($dir) ) { + mkdir($dir, 0755, true); + } + + // Simulate fatal after set(): file left behind, transient already expired + delete_transient(FirewallUpdateLock::TRANSIENT_KEY); + file_put_contents($path, (string) ( time() - FirewallUpdateLock::TTL - 10 )); + + $this->assertFalse(FirewallUpdateLock::isLocked()); + $this->assertFileNotExists($path); + } + + public function testRequiredFwDataTablesListsAllSwapTables() + { + $tables = FirewallUpdateLock::requiredFwDataTables(); + + $this->assertCount(5, $tables); + $this->assertContains(SPBC_TBL_FIREWALL_DATA_V4, $tables); + $this->assertContains(SPBC_TBL_FIREWALL_DATA_V6, $tables); + $this->assertContains(SPBC_TBL_FIREWALL_DATA__IPS_V4, $tables); + $this->assertContains(SPBC_TBL_FIREWALL_DATA__IPS_V6, $tables); + $this->assertContains(SPBC_TBL_FIREWALL_DATA__COUNTRIES, $tables); + } }