Introduce new type-safe methods for receiving config section values

core/Common.php

@@ -28,6 +28,8 @@
  */
 class Common
 {
+    private const FLOAT_REGEX = "/^[-+]?((([0-9]+(_[0-9]+)*)|(([0-9]+(_[0-9]+)*)?\.([0-9]+(_[0-9]+)*))|(([0-9]+(_[0-9]+)*)\.([0-9]+(_[0-9]+)*)?))([eE][+-]?([0-9]+(_[0-9]+)*))?)$/";
+
     // constants used to map the referrer type to an integer in the log_visit table
     public const REFERRER_TYPE_DIRECT_ENTRY = 1;
     public const REFERRER_TYPE_SEARCH_ENGINE = 2;
@@ -1093,6 +1095,26 @@ public static function forceDotAsSeparatorForDecimalPoint($value)
         return str_replace(',', '.', $value);
     }
 
+    /**
+     * Parses the given value as float and returns null if it cannot be represented as a PHP float.
+     *
+     * Supports the same string notations as PHP floats, including underscore notation.
+     *
+     * @param mixed $value
+     */
+    public static function parseFloat($value): ?float
+    {
+        if (is_float($value) || is_int($value)) {
+            return (float)$value;
+        }
+
+        if (is_string($value) && preg_match(self::FLOAT_REGEX, $value)) {
+            return (float)str_replace('_', '', $value);
+        }
+
+        return null;
+    }
+
     /**
      * Sets outgoing header.
      *

core/Config/SectionConfig.php

@@ -10,6 +10,9 @@
 namespace Piwik\Config;
 
 use Piwik\Config;
+use Piwik\Common;
+use Piwik\Container\StaticContainer;
+use Piwik\Log\LoggerInterface;
 
 abstract class SectionConfig
 {
@@ -31,39 +34,248 @@ public static function setConfigValue(string $name, $value): void
     /**
      * Get a setting value
      *
+     * Prefer one of the type-safe getters if a specific type is expected:
+     * @see getIntegerConfigValue()
+     * @see getFloatConfigValue()
+     * @see getBoolConfigValue()
+     * @see getStringConfigValue()
+     * @see getArrayConfigValue()
+     *
      * @param string    $name     Setting name
      * @param int|null  $idSite   Optional site Id
      *
      * @return mixed|null
      */
     public static function getConfigValue(string $name, ?int $idSite = null)
     {
-        $config = self::getConfig();
-        if (!empty($idSite)) {
-            $siteSpecificConfig = self::getSiteSpecificConfig($idSite);
-            $config = array_merge($config, $siteSpecificConfig);
-        }
+        return static::getRawConfigValue($name, $idSite);
+    }
+
+    /**
+     * @phpstan-return ($default is null ? int|null : int)
+     */
+    public static function getIntegerConfigValue(string $name, ?int $default = null, ?int $idSite = null): ?int
+    {
+        return self::castIntConfigValue($name, static::getRawConfigValue($name, $idSite), $default);
+    }
+
+    /**
+     * @phpstan-return ($default is null ? float|null : float)
+     */
+    public static function getFloatConfigValue(string $name, ?float $default = null, ?int $idSite = null): ?float
+    {
+        return self::castFloatConfigValue($name, static::getRawConfigValue($name, $idSite), $default);
+    }
+
+    /**
+     * @phpstan-return ($default is null ? bool|null : bool)
+     */
+    public static function getBoolConfigValue(string $name, ?bool $default = null, ?int $idSite = null): ?bool
+    {
+        return self::castBoolConfigValue($name, static::getRawConfigValue($name, $idSite), $default);
+    }
+
+    /**
+     * @phpstan-return ($default is null ? string|null : string)
+     */
+    public static function getStringConfigValue(string $name, ?string $default = null, ?int $idSite = null): ?string
+    {
+        return self::castStringConfigValue($name, static::getRawConfigValue($name, $idSite), $default);
+    }
+
+    /**
+     * @return array<mixed>|null
+     * @phpstan-return ($default is null ? array<mixed>|null : array<mixed>)
+     */
+    public static function getArrayConfigValue(string $name, ?array $default = null, ?int $idSite = null): ?array
+    {
+        return self::castArrayConfigValue($name, static::getRawConfigValue($name, $idSite), $default);
+    }
+
+    /**
+     * @return mixed|null
+     */
+    protected static function getRawConfigValue(string $name, ?int $idSite = null)
+    {
+        $config = self::getMergedConfig($idSite);
         return $config[$name] ?? null;
     }
 
     /**
      * Get the section config as an array
      *
-     * @return array|string
+     * @return array<string, mixed>
      */
-    private static function getConfig()
+    private static function getConfig(): array
     {
-        return Config::getInstance()->{static::getSectionName()};
+        $config = Config::getInstance()->{static::getSectionName()};
+        return is_array($config) ? $config : [];
     }
 
     /**
      * Get the site specific config (if any) as an array
      *
-     * @return array|string
+     * @return array<string, mixed>
      */
-    private static function getSiteSpecificConfig(int $idSite)
+    private static function getSiteSpecificConfig(int $idSite): array
     {
         $key = static::getSectionName() . '_' . $idSite;
-        return Config::getInstance()->$key;
+        $config = Config::getInstance()->$key;
+        return is_array($config) ? $config : [];
+    }
+
+    /**
+     * @return array<string, mixed>
+     */
+    private static function getMergedConfig(?int $idSite = null): array
+    {
+        $config = self::getConfig();
+        if (!empty($idSite)) {
+            $config = array_merge($config, self::getSiteSpecificConfig($idSite));
+        }
+        return $config;
+    }
+
+    /**
+     * @param mixed $value
+     */
+    private static function castIntConfigValue(string $name, $value, ?int $default): ?int
+    {
+        if ($value === null) {
+            return $default;
+        }
+
+        if ((is_string($value) || is_numeric($value)) && (string) $value === (string) (int) $value) {
+            return (int) $value;
+        }
+
+        self::logTypeCastWarning($name, 'int', $value);
+        return $default;
+    }
+
+    /**
+     * @param mixed $value
+     */
+    private static function castFloatConfigValue(string $name, $value, ?float $default): ?float
+    {
+        if ($value === null) {
+            return $default;
+        }
+
+        $parsedFloat = Common::parseFloat($value);
+        if ($parsedFloat !== null) {
+            return $parsedFloat;
+        }
+
+        self::logTypeCastWarning($name, 'float', $value);
+        return $default;
+    }
+
+    /**
+     * @param mixed $value
+     */
+    private static function castBoolConfigValue(string $name, $value, ?bool $default): ?bool
+    {
+        if ($value === null) {
+            return $default;
+        }
+
+        if ($value === false || $value === true) {
+            return $value;
+        }
+
+        if ((is_string($value) && strtolower($value) === 'false') || $value === '0' || $value === 0) {
+            return false;
+        }
+
+        if ((is_string($value) && strtolower($value) === 'true') || $value === '1' || $value === 1) {
+            return true;
+        }
+
+        self::logTypeCastWarning($name, 'bool', $value);
+        return $default;
+    }
+
+    /**
+     * @param mixed $value
+     */
+    private static function castStringConfigValue(string $name, $value, ?string $default): ?string
+    {
+        if ($value === null) {
+            return $default;
+        }
+
+        if (is_string($value) || is_numeric($value)) {
+            return Common::sanitizeNullBytes((string) $value);
+        }
+
+        self::logTypeCastWarning($name, 'string', $value);
+        return $default;
+    }
+
+    /**
+     * @param mixed $value
+     * @return array<mixed>|null
+     */
+    private static function castArrayConfigValue(string $name, $value, ?array $default): ?array
+    {
+        if ($value === null) {
+            return $default;
+        }
+
+        if (is_array($value)) {
+            /** @var array<mixed> $sanitizedValue */
+            $sanitizedValue = self::filterNullBytes($value);
+            return $sanitizedValue;
+        }
+
+        self::logTypeCastWarning($name, 'array', $value);
+        return $default;
+    }
+
+    /**
+     * @param mixed $value
+     */
+    private static function logTypeCastWarning(string $name, string $type, $value): void
+    {
+        StaticContainer::get(LoggerInterface::class)->warning(
+            'Failed to cast config value {section}.{name} to {type}; actual type was {actualType}.',
+            [
+                'section' => static::getSectionName(),
+                'name' => $name,
+                'type' => $type,
+                'actualType' => self::getValueType($value),
+            ]
+        );
+    }
+
+    /**
+     * @param mixed $value
+     */
+    private static function getValueType($value): string
+    {
+        if (is_object($value)) {
+            return get_class($value);
+        }
+
+        return gettype($value);
+    }
+
+    /**
+     * @param mixed $value
+     * @return mixed
+     */
+    private static function filterNullBytes($value)
+    {
+        if (is_array($value)) {
+            $result = [];
+            foreach ($value as $key => $arrayValue) {
+                $result[$key] = self::filterNullBytes($arrayValue);
+            }
+
+            return $result;
+        }
+
+        return is_string($value) ? Common::sanitizeNullBytes($value) : $value;
     }
 }

core/DataTable/Filter/PivotByDimension.php

@@ -581,7 +581,7 @@ public static function isPivotingReportBySubtableSupported(Report $report)
      */
     public static function isSegmentFetchingEnabledInConfig()
     {
-        return (bool)GeneralConfig::getConfigValue('pivot_by_filter_enable_fetch_by_segment');
+        return GeneralConfig::getBoolConfigValue('pivot_by_filter_enable_fetch_by_segment', false);
     }
 
     /**
@@ -592,7 +592,7 @@ public static function isSegmentFetchingEnabledInConfig()
      */
     public static function getDefaultColumnLimit()
     {
-        return (int)GeneralConfig::getConfigValue('pivot_by_filter_default_column_limit');
+        return GeneralConfig::getIntegerConfigValue('pivot_by_filter_default_column_limit', 0);
     }
 
     /**

core/Request.php

@@ -156,17 +156,9 @@ public function getIntegerParameter(string $name, ?int $default = null): int
     public function getFloatParameter(string $name, ?float $default = null): float
     {
         $parameter = $this->getParameter($name, $default);
-
-        if (is_float($parameter) || is_int($parameter)) {
-            return (float)$parameter;
-        }
-
-        // Regex for all supported float notations in PHP (see https://www.php.net/manual/en/language.types.float.php)
-        $floatRegex = "/^[-+]?((([0-9]+(_[0-9]+)*)|(([0-9]+(_[0-9]+)*)?\.([0-9]+(_[0-9]+)*))|(([0-9]+(_[0-9]+)*)\.([0-9]+(_[0-9]+)*)?))([eE][+-]?([0-9]+(_[0-9]+)*))?)$/";
-
-        if (is_string($parameter) && preg_match($floatRegex, $parameter)) {
-            // underscores would break numbers if not removed before
-            return (float) str_replace('_', '', $parameter);
+        $parsedFloat = Common::parseFloat($parameter);
+        if ($parsedFloat !== null) {
+            return $parsedFloat;
         }
 
         if (null !== $default) {

core/SiteContentDetector.php

@@ -347,7 +347,7 @@ private function requestSiteResponse(string $url, int $timeOut): array
         }
 
         // If internet features are disabled, we don't try to fetch any site content
-        if (0 === (int) GeneralConfig::getConfigValue('enable_internet_features')) {
+        if (0 === GeneralConfig::getIntegerConfigValue('enable_internet_features', 0)) {
             return [];
         }
 

core/Tracker.php

@@ -377,12 +377,12 @@ private function handleFatalErrors()
     private static function isDebugEnabled()
     {
         try {
-            $debug = (bool) TrackerConfig::getConfigValue('debug');
+            $debug = TrackerConfig::getBoolConfigValue('debug', false);
             if ($debug) {
                 return true;
             }
 
-            $debugOnDemand = (bool) TrackerConfig::getConfigValue('debug_on_demand');
+            $debugOnDemand = TrackerConfig::getBoolConfigValue('debug_on_demand', false);
             if ($debugOnDemand) {
                 return (bool) Common::getRequestVar('debug', false);
             }

core/Tracker/Request.php

@@ -100,8 +100,9 @@ public function __construct(
         // check for 4byte utf8 characters in all tracking params and replace them with � if not support by database
         $this->params = $this->replaceUnsupportedUtf8Chars($this->params);
 
-        $this->customTimestampDoesNotRequireTokenauthWhenNewerThan = (int) TrackerConfig::getConfigValue(
+        $this->customTimestampDoesNotRequireTokenauthWhenNewerThan = TrackerConfig::getIntegerConfigValue(
             'tracking_requests_require_authentication_when_custom_timestamp_newer_than',
+            0,
             $this->getIdSiteIfExists()
         );
     }