Merge pull request #113 from julienloizelet/feat/various-fix-and-update

Feat/various fix and update

Author: julienloizelet

CHANGELOG.md

@@ -5,6 +5,22 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 
+## [0.34.0](https://github.com/crowdsecurity/php-cs-bouncer/releases/tag/v0.34.0) - 2022-11-24
+[_Compare with previous release_](https://github.com/crowdsecurity/php-cs-bouncer/compare/v0.33.0...v0.34.0)
+
+### Changed
+- Do not cache bypass decision in stream mode
+- Replace unauthorized chars by underscore `_` in cache key
+
+### Added
+- Add compatibility with PHP 8.2
+
+### Fixed
+- Fix decision duration parsing when it uses milliseconds
+
+---
+
+
 ## [0.33.0](https://github.com/crowdsecurity/php-cs-bouncer/releases/tag/v0.33.0) - 2022-11-10
 [_Compare with previous release_](https://github.com/crowdsecurity/php-cs-bouncer/compare/v0.32.0...v0.33.0)
 

composer.json

@@ -50,7 +50,7 @@
         "ext-json": "*"
     },
     "require-dev": {
-        "phpunit/phpunit": "8.5.27",
+        "phpunit/phpunit": "^8.5.30 || ^9.3",
         "ext-curl": "*"
     },
     "suggest": {

src/AbstractCache.php

@@ -145,17 +145,24 @@ protected function getCacheKey(string $scope, string $value): string
         if (!isset($this->cacheKeys[$scope][$value])) {
             switch ($scope) {
                 case Constants::SCOPE_RANGE:
-                    $this->cacheKeys[$scope][$value] = Constants::SCOPE_IP . self::CACHE_SEP . $value;
+                    $result = Constants::SCOPE_IP . self::CACHE_SEP . $value;
                     break;
                 case Constants::SCOPE_IP:
                 case Constants::CACHE_TAG_GEO . self::CACHE_SEP . Constants::SCOPE_IP:
                 case Constants::CACHE_TAG_CAPTCHA . self::CACHE_SEP . Constants::SCOPE_IP:
                 case Constants::SCOPE_COUNTRY:
-                    $this->cacheKeys[$scope][$value] = $scope . self::CACHE_SEP . $value;
+                    $result = $scope . self::CACHE_SEP . $value;
                     break;
                 default:
                     throw new BouncerException('Unknown scope:' . $scope);
             }
+
+            /**
+             * Replace unauthorized symbols.
+             *
+             * @see https://symfony.com/doc/current/components/cache/cache_items.html#cache-item-keys-and-values
+             */
+            $this->cacheKeys[$scope][$value] = preg_replace('/[^A-Za-z0-9_.]/', self::CACHE_SEP, $result);
         }
 
         return $this->cacheKeys[$scope][$value];
@@ -285,7 +292,8 @@ protected function addRemediationToCacheItem(
         ]; // erase previous decision with the same id
 
         // Build the item lifetime in cache and sort remediations by priority
-        $maxLifetime = max(array_column($remediations, 1));
+        $exps = array_column($remediations, 1);
+        $maxLifetime = $exps ? max($exps) : 0;
         $prioritizedRemediations = Remediation::sortRemediationByPriority($remediations);
 
         $item->set($prioritizedRemediations);
@@ -341,20 +349,14 @@ protected function defferUpdateCacheConfig(array $config): void
     protected function formatRemediationFromDecision(?array $decision): array
     {
         if (!$decision) {
-            /**
-             * In stream mode we consider a clean IP forever... until the next resync.
-             * in this case, forever is 10 years as PHP_INT_MAX will cause trouble with the Memcached Adapter
-             * (int to float unwanted conversion)
-             *
-             */
-            $duration = $this->streamMode ? 315360000 : $this->cacheExpirationForCleanIp;
+            $duration = $this->cacheExpirationForCleanIp;
 
             return [Constants::REMEDIATION_BYPASS, time() + $duration, 0];
         }
 
         $duration = self::parseDurationToSeconds($decision['duration']);
 
-        // Don't set a max duration in stream mode to avoid bugs. Only the stream update has to change the cache state.
+        // In stream mode, only the stream update has to change the cache state.
         if (!$this->streamMode) {
             $duration = min($this->cacheExpirationForBadIp, $duration);
         }
@@ -425,6 +427,10 @@ protected function miss(string $value, string $cacheScope): string
                 ]);
             }
         }
+        // In stream mode, we do not save bypass decision in cache
+        if ($this->streamMode && !$decisions) {
+            return Constants::REMEDIATION_BYPASS;
+        }
 
         return $this->saveRemediationsForCacheKey($decisions, $cacheKey);
     }
@@ -464,7 +470,8 @@ protected function removeDecisionFromRemediationItem(string $cacheKey, int $deci
             return true;
         }
         // Build the item lifetime in cache and sort remediations by priority
-        $maxLifetime = max(array_column($remediations, 1));
+        $exps = array_column($remediations, 1);
+        $maxLifetime = $exps ? max($exps) : 0;
         $cacheContent = Remediation::sortRemediationByPriority($remediations);
         $item->expiresAt(new DateTime('@' . $maxLifetime));
         $item->set($cacheContent);
@@ -576,7 +583,8 @@ private function configureAdapter(): void
                     $this->adapter = new RedisTagAwareAdapter((RedisAdapter::createConnection($redisDsn)));
                 } catch (Exception $e) {
                     throw new BouncerException('Error when connecting to Redis.' .
-                                               ' Please fix the Redis DSN or select another cache technology.');
+                                               ' Please fix the Redis DSN or select another cache technology.' .
+                                                ' Initial error was: ' . $e->getMessage());
                 }
                 break;
 
@@ -612,7 +620,7 @@ private static function parseDurationToSeconds(string $duration): int
         $re = '/(-?)(?:(?:(\d+)h)?(\d+)m)?(\d+).\d+(m?)s/m';
         preg_match($re, $duration, $matches);
         if (!\count($matches)) {
-            throw new BouncerException("Unable to parse the following duration: ${$duration}.");
+            throw new BouncerException('Unable to parse the following duration: ' . $duration);
         }
         $seconds = 0;
         if (isset($matches[2])) {
@@ -621,12 +629,14 @@ private static function parseDurationToSeconds(string $duration): int
         if (isset($matches[3])) {
             $seconds += ((int)$matches[3]) * 60; // minutes
         }
+        $secondsPart = 0;
         if (isset($matches[4])) {
-            $seconds += ((int)$matches[4]); // seconds
+            $secondsPart += ((int)$matches[4]); // seconds
         }
         if ('m' === ($matches[5])) { // units in milliseconds
-            $seconds *= 0.001;
+            $secondsPart *= 0.001;
         }
+        $seconds += $secondsPart;
         if ('-' === ($matches[1])) { // negative
             $seconds *= -1;
         }

src/Constants.php

@@ -75,7 +75,7 @@ class Constants
     /** @var string Path for html templates folder (e.g. ban and captcha wall) */
     public const TEMPLATES_DIR = __DIR__ . "/templates";
     /** @var string The last version of this library */
-    public const VERSION = 'v0.33.0';
+    public const VERSION = 'v0.34.0';
     /** @var string The "disabled" x-forwarded-for setting */
     public const X_FORWARDED_DISABLED = 'no_forward';
 }

src/Fixes/Gregwar/Captcha/CaptchaBuilder.php

@@ -5,14 +5,22 @@
 use Gregwar\Captcha\CaptchaBuilder as GregwarCaptchaBuilder;
 
 /**
- * Override to fix "implicit conversion error on PHP  8.1"
+ * Override to :
+ * - fix "implicit conversion error on PHP  8.1"
+ * - fix "creation of dynamic property $background error on PHP 8.2"
+ *
+ *
  * @see https://github.com/crowdsecurity/php-cs-bouncer/issues/62 and
  * @see https://github.com/Gregwar/Captcha/pull/101/files
  * @SuppressWarnings(PHPMD.ElseExpression)
  *
  */
 class CaptchaBuilder extends GregwarCaptchaBuilder
 {
+    /**
+     * @var false|int
+     */
+    protected $background = false;
     /**
      * Writes the phrase on the image
      */

src/Remediation.php

@@ -57,8 +57,8 @@ public static function sortRemediationByPriority(array $remediations): array
         }
 
         // Sort by priorities.
-        /** @var callable $compareFunction */
-        $compareFunction = 'self::comparePriorities';
+        /** @var callable $compareFunction  */
+        $compareFunction = self::class . '::comparePriorities';
         usort($remediationsWithPriorities, $compareFunction);
 
         return $remediationsWithPriorities;

src/RestClient/Curl.php

@@ -51,8 +51,6 @@ public function request(
     }
 
     /**
-     * @param $handle
-     *
      * @return bool|string
      */
     protected function exec($handle)
@@ -61,8 +59,6 @@ protected function exec($handle)
     }
 
     /**
-     * @param $handle
-     *
      * @return mixed
      */
     protected function getResponseHttpCode($handle)

tests/end-to-end/package.json

@@ -23,8 +23,7 @@
     "jest-playwright-preset": "^1.4.3",
     "jest-runner": "^26.6.3",
     "lodash": "^4.17.21",
-    "playwright-chromium": "1.22.2",
-    "soap": "^0.42.0",
+    "playwright-chromium": "^1.27.1",
     "ws": "^7.4.6"
   }
 }