diff --git a/src/Core/Environment.php b/src/Core/Environment.php index 84bf2cfe7..a6b0fc7ef 100644 --- a/src/Core/Environment.php +++ b/src/Core/Environment.php @@ -187,16 +187,22 @@ class Environment */ public static function getEnv($name) { - switch (true) { - case is_array(static::$env) && array_key_exists($name, static::$env): - return static::$env[$name]; - case is_array($_ENV) && array_key_exists($name, $_ENV): - return $_ENV[$name]; - case is_array($_SERVER) && array_key_exists($name, $_SERVER): - return $_SERVER[$name]; - default: - return getenv($name); + if (array_key_exists($name, static::$env)) { + return static::$env[$name]; } + // isset() is used for $_ENV and $_SERVER instead of array_key_exists() to fix a very strange issue that + // occured in CI running silverstripe/recipe-kitchen-sink where PHP would timeout due apparently due to an + // excessively high number of array method calls. isset() is not used for static::$env above because + // values there may be null, and isset() will return false for null values + // Symfony also uses isset() for reading $_ENV and $_SERVER values + // https://github.com/symfony/dependency-injection/blob/6.2/EnvVarProcessor.php#L148 + if (isset($_ENV[$name])) { + return $_ENV[$name]; + } + if (isset($_SERVER[$name])) { + return $_SERVER[$name]; + } + return getenv($name); } /** @@ -231,9 +237,10 @@ class Environment */ public static function hasEnv(string $name): bool { + // See getEnv() for an explanation of why isset() is used for $_ENV and $_SERVER return array_key_exists($name, static::$env) - || is_array($_ENV) && array_key_exists($name, $_ENV) - || is_array($_SERVER) && array_key_exists($name, $_SERVER) + || isset($_ENV[$name]) + || isset($_SERVER[$name]) || getenv($name) !== false; } diff --git a/src/Dev/Deprecation.php b/src/Dev/Deprecation.php index 7e3a545dd..d74b7bf45 100644 --- a/src/Dev/Deprecation.php +++ b/src/Dev/Deprecation.php @@ -237,8 +237,8 @@ class Deprecation if (!Director::isDev()) { return false; } - $envVar = Environment::getEnv('SS_DEPRECATION_ENABLED'); if (Environment::hasEnv('SS_DEPRECATION_ENABLED')) { + $envVar = Environment::getEnv('SS_DEPRECATION_ENABLED'); return self::varAsBoolean($envVar); } return static::$currentlyEnabled; @@ -293,8 +293,8 @@ class Deprecation */ public static function shouldShowForHttp(): bool { - $envVar = Environment::getEnv('SS_DEPRECATION_SHOW_HTTP'); if (Environment::hasEnv('SS_DEPRECATION_SHOW_HTTP')) { + $envVar = Environment::getEnv('SS_DEPRECATION_SHOW_HTTP'); return self::varAsBoolean($envVar); } return self::$shouldShowForHttp; @@ -306,8 +306,8 @@ class Deprecation */ public static function shouldShowForCli(): bool { - $envVar = Environment::getEnv('SS_DEPRECATION_SHOW_CLI'); if (Environment::hasEnv('SS_DEPRECATION_SHOW_CLI')) { + $envVar = Environment::getEnv('SS_DEPRECATION_SHOW_CLI'); return self::varAsBoolean($envVar); } return self::$shouldShowForCli; diff --git a/tests/php/Core/EnvironmentTest.php b/tests/php/Core/EnvironmentTest.php index 221b2563a..7f864710b 100644 --- a/tests/php/Core/EnvironmentTest.php +++ b/tests/php/Core/EnvironmentTest.php @@ -79,7 +79,6 @@ class EnvironmentTest extends SapphireTest $valueOptions = [ true, false, - null, 0, 1, 1.75, @@ -97,6 +96,12 @@ class EnvironmentTest extends SapphireTest ]; } } + // `null` isn't a supported value outside of using the `.env` option. + $scenarios[] = [ + 'setAs' => '.env', + 'value' => null, + 'expected' => true, + ]; $scenarios[] = [ 'setAs' => null, 'value' => null,