From 781401e3c8ff93c8754c4ba3750ca3034839c143 Mon Sep 17 00:00:00 2001 From: datorik Date: Wed, 9 Sep 2026 22:12:13 +0300 Subject: [PATCH 1/2] Udp.Code.Unit tests --- lib/Cleantalk/ApbctWP/RemoteCalls.php | 36 +++++-- tests/ApbctWP/TestRemoteCalls.php | 132 ++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 8 deletions(-) diff --git a/lib/Cleantalk/ApbctWP/RemoteCalls.php b/lib/Cleantalk/ApbctWP/RemoteCalls.php index 98b682cab..eccdff050 100644 --- a/lib/Cleantalk/ApbctWP/RemoteCalls.php +++ b/lib/Cleantalk/ApbctWP/RemoteCalls.php @@ -116,6 +116,31 @@ private static function isAllowedWithoutToken($rc) return in_array($rc, self::$allowedActionsWithoutToken, true); } + /** + * Delay to wait before the remote call is passed to the host. + * + * Only the whitelisted actions are allowed to delay, and the delay itself is limited + * by self::MAX_DELAY. + * + * @param string $action Remote call action name, without the "action__" prefix + * @param mixed $delay Delay as it came in the request + * + * @return int|null Seconds to sleep, null when the call must be performed without a delay + * @psalm-return int<0, max>|null + */ + private static function getDelayForAction($action, $delay) + { + if ( ! $delay || ! in_array(strtolower($action), self::$allowedActionsWithDelay, true) ) { + return null; + } + + $delay = TT::toInt($delay); + $delay = max($delay, 0); + $delay = min($delay, self::MAX_DELAY); + + return $delay; + } + public static function checkWithoutToken() { global $apbct; @@ -215,18 +240,13 @@ public static function perform() // Flag to let plugin know that Remote Call is running. $apbct->rc_running = true; + $raw_action = $action; $action = 'action__' . $action; if ( method_exists(__CLASS__, $action) ) { // Delay before perform action - only for whitelisted actions - $current_action = strtolower(Request::getString('spbc_remote_call_action')); - if ( - Request::get('delay') && - in_array($current_action, self::$allowedActionsWithDelay, true) - ) { - $delay = Request::getInt('delay'); - $delay = max($delay, 0); - $delay = min($delay, self::MAX_DELAY); + $delay = self::getDelayForAction($raw_action, Request::get('delay')); + if ( ! is_null($delay) ) { sleep($delay); $params = $_REQUEST; unset($params['delay']); diff --git a/tests/ApbctWP/TestRemoteCalls.php b/tests/ApbctWP/TestRemoteCalls.php index d821e179e..3a1c499eb 100644 --- a/tests/ApbctWP/TestRemoteCalls.php +++ b/tests/ApbctWP/TestRemoteCalls.php @@ -361,6 +361,138 @@ public function itOnlyAllowsSfwUpdateWorkerForDelay() $this->assertEquals(['sfw_update__worker'], $allowedActions); } + /** @test */ + public function itAllowsDelayOnlyForWhitelistedAction() + { + $method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction'); + $method->setAccessible(true); + + $this->assertSame(5, $method->invoke(null, 'sfw_update__worker', 5)); + + $this->assertNull($method->invoke(null, 'sfw_update', 5)); + $this->assertNull($method->invoke(null, 'debug', 5)); + $this->assertNull($method->invoke(null, 'get_fresh_wpnonce', 5)); + $this->assertNull($method->invoke(null, 'post_api_key', 5)); + $this->assertNull($method->invoke(null, 'install_plugin', 5)); + $this->assertNull($method->invoke(null, '', 5)); + } + + /** @test */ + public function itIgnoresActionCaseWhenCheckingDelayPermission() + { + $method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction'); + $method->setAccessible(true); + + $this->assertSame(5, $method->invoke(null, 'SFW_UPDATE__WORKER', 5)); + $this->assertSame(5, $method->invoke(null, 'Sfw_Update__Worker', 5)); + } + + /** @test */ + public function itDoesNotAllowDelayForPrefixedActionName() + { + // perform() must pass the raw action name, not the one prefixed with 'action__' + $method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction'); + $method->setAccessible(true); + + $this->assertNull($method->invoke(null, 'action__sfw_update__worker', 5)); + } + + /** @test */ + public function itReturnsNullWhenDelayParamIsEmpty() + { + // No delay param — the action must be performed right here, not passed to the host + $method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction'); + $method->setAccessible(true); + + $this->assertNull($method->invoke(null, 'sfw_update__worker', null)); + $this->assertNull($method->invoke(null, 'sfw_update__worker', false)); + $this->assertNull($method->invoke(null, 'sfw_update__worker', '')); + $this->assertNull($method->invoke(null, 'sfw_update__worker', '0')); + $this->assertNull($method->invoke(null, 'sfw_update__worker', 0)); + } + + /** @test */ + public function itKeepsAllowedDelayValueAsIs() + { + $method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction'); + $method->setAccessible(true); + + $this->assertSame(1, $method->invoke(null, 'sfw_update__worker', 1)); + $this->assertSame(3, $method->invoke(null, 'sfw_update__worker', 3)); + $this->assertSame( + RemoteCalls::MAX_DELAY, + $method->invoke(null, 'sfw_update__worker', RemoteCalls::MAX_DELAY) + ); + } + + /** @test */ + public function itCapsDelayAtMaxDelay() + { + $method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction'); + $method->setAccessible(true); + + $this->assertSame( + RemoteCalls::MAX_DELAY, + $method->invoke(null, 'sfw_update__worker', RemoteCalls::MAX_DELAY + 1) + ); + $this->assertSame(RemoteCalls::MAX_DELAY, $method->invoke(null, 'sfw_update__worker', 999)); + $this->assertSame(RemoteCalls::MAX_DELAY, $method->invoke(null, 'sfw_update__worker', PHP_INT_MAX)); + } + + /** @test */ + public function itTurnsNegativeDelayIntoZeroButStillDelegatesToHost() + { + // Zero, not null: the call is still passed to the host, it just does not sleep + $method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction'); + $method->setAccessible(true); + + $this->assertSame(0, $method->invoke(null, 'sfw_update__worker', -1)); + $this->assertSame(0, $method->invoke(null, 'sfw_update__worker', -999)); + $this->assertSame(0, $method->invoke(null, 'sfw_update__worker', PHP_INT_MIN)); + } + + /** @test */ + public function itCastsNonIntegerDelayToInt() + { + $method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction'); + $method->setAccessible(true); + + $this->assertSame(7, $method->invoke(null, 'sfw_update__worker', '7')); + $this->assertSame(7, $method->invoke(null, 'sfw_update__worker', 7.9)); + $this->assertSame(0, $method->invoke(null, 'sfw_update__worker', 'abc')); + $this->assertSame(0, $method->invoke(null, 'sfw_update__worker', array(5))); + } + + /** @test */ + public function performDelegatesDelayDecisionToGetDelayForAction() + { + $method = new ReflectionMethod(RemoteCalls::class, 'perform'); + $source = implode('', array_slice( + file($method->getFileName()), + $method->getStartLine() - 1, + $method->getEndLine() - $method->getStartLine() + 1 + )); + + $this->assertStringContainsString( + "self::getDelayForAction(\$raw_action, Request::get('delay'))", + $source, + 'perform() must take the delay decision from getDelayForAction()' + ); + + // The branch is chosen by null, not by the value: a negative delay is still + // passed to the host instead of running the action locally + $this->assertStringContainsString( + 'if ( ! is_null($delay) ) {', + $source, + 'perform() must branch on null, not on the delay value' + ); + + $this->assertStringNotContainsString( + 'allowedActionsWithDelay', + $source, + 'The whitelist check belongs to getDelayForAction(), not to perform()' + ); + } // ========================================================================= // APBCT-W07: 'api_key' added to $sensitiveData list // ========================================================================= From 44a56a195391779cdf7f05cfa4ff4d88230f740f Mon Sep 17 00:00:00 2001 From: datorik Date: Wed, 9 Sep 2026 22:48:54 +0300 Subject: [PATCH 2/2] Fix errors psalm --- lib/Cleantalk/Common/Helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cleantalk/Common/Helper.php b/lib/Cleantalk/Common/Helper.php index c592f1efe..2e777c58b 100644 --- a/lib/Cleantalk/Common/Helper.php +++ b/lib/Cleantalk/Common/Helper.php @@ -595,7 +595,7 @@ public static function ipResolve($ip) * @param $host * @param bool $out * - * @return bool + * @return bool|string Resolved IP, the host itself when it is already an IP, or $out on failure * @psalm-suppress PossiblyUnusedMethod */ public static function dnsResolve($host, $out = false)