Refactor the Core::getenv() method

Renames it to getEnv() and adds unit tests for it. Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
phpmyadmin · Sep 28, 2023 · 1ae995b · 1ae995b
1 parent b338c18
commit 1ae995b
Show file tree

Hide file tree

Showing 13 changed files with 85 additions and 83 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -8315,14 +8315,9 @@ parameters:
 			count: 1
 			path: src/Core.php
 
-		-
-			message: "#^Cannot cast mixed to string\\.$#"
-			count: 2
-			path: src/Core.php
-
 		-
 			message: "#^Casting to string something that's already string\\.$#"
-			count: 3
+			count: 1
 			path: src/Core.php
 
 		-
@@ -8335,11 +8330,6 @@ parameters:
 			count: 1
 			path: src/Core.php
 
-		-
-			message: "#^Only booleans are allowed in &&, string\\|false given on the right side\\.$#"
-			count: 1
-			path: src/Core.php
-
 		-
 			message: "#^Only booleans are allowed in a negated boolean, int\\|false given\\.$#"
 			count: 2
@@ -8360,11 +8350,6 @@ parameters:
 			count: 1
 			path: src/Core.php
 
-		-
-			message: "#^Only booleans are allowed in an if condition, string\\|false given\\.$#"
-			count: 1
-			path: src/Core.php
-
 		-
 			message: "#^Parameter \\#1 \\$name of method Symfony\\\\Component\\\\DependencyInjection\\\\Container\\:\\:setParameter\\(\\) expects string, \\(int\\|string\\) given\\.$#"
 			count: 1
@@ -8385,6 +8370,11 @@ parameters:
 			count: 1
 			path: src/Core.php
 
+		-
+			message: "#^Parameter \\#1 \\$variableName of static method PhpMyAdmin\\\\Core\\:\\:getEnv\\(\\) expects non\\-empty\\-string, string given\\.$#"
+			count: 1
+			path: src/Core.php
+
 		-
 			message: "#^Parameter \\#2 \\$subject of function preg_match expects string, \\(int\\|string\\) given\\.$#"
 			count: 1

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -4481,6 +4481,9 @@
     </PossiblyUnusedMethod>
   </file>
   <file src="src/Core.php">
+    <ArgumentTypeCoercion>
+      <code><![CDATA[$config->settings['TrustedProxies'][$directIp]]]></code>
+    </ArgumentTypeCoercion>
     <DeprecatedMethod>
       <code>Config::getInstance()</code>
       <code>Config::getInstance()</code>
@@ -4536,9 +4539,6 @@
     <PossiblyInvalidArrayOffset>
       <code>$path[$depth - 1]</code>
     </PossiblyInvalidArrayOffset>
-    <PossiblyInvalidCast>
-      <code>$_SERVER[$varName]</code>
-    </PossiblyInvalidCast>
     <PossiblyUndefinedArrayOffset>
       <code><![CDATA[$_SERVER['SERVER_NAME']]]></code>
     </PossiblyUndefinedArrayOffset>

diff --git a/src/Config.php b/src/Config.php
@@ -210,8 +210,8 @@ private function setClientPlatform(string $userAgent): void
     public function checkClient(): void
     {
         $httpUserAgent = '';
-        if (Core::getenv('HTTP_USER_AGENT') !== '') {
-            $httpUserAgent = Core::getenv('HTTP_USER_AGENT');
+        if (Core::getEnv('HTTP_USER_AGENT') !== '') {
+            $httpUserAgent = Core::getEnv('HTTP_USER_AGENT');
         }
 
         // 1. Platform
@@ -771,26 +771,26 @@ public function isHttps(): bool
         $isHttps = false;
         if (! empty($url) && parse_url($url, PHP_URL_SCHEME) === 'https') {
             $isHttps = true;
-        } elseif (strtolower(Core::getenv('HTTP_SCHEME')) === 'https') {
+        } elseif (strtolower(Core::getEnv('HTTP_SCHEME')) === 'https') {
             $isHttps = true;
-        } elseif (strtolower(Core::getenv('HTTPS')) === 'on') {
+        } elseif (strtolower(Core::getEnv('HTTPS')) === 'on') {
             $isHttps = true;
-        } elseif (strtolower(substr(Core::getenv('REQUEST_URI'), 0, 6)) === 'https:') {
+        } elseif (strtolower(substr(Core::getEnv('REQUEST_URI'), 0, 6)) === 'https:') {
             $isHttps = true;
-        } elseif (strtolower(Core::getenv('HTTP_HTTPS_FROM_LB')) === 'on') {
+        } elseif (strtolower(Core::getEnv('HTTP_HTTPS_FROM_LB')) === 'on') {
             // A10 Networks load balancer
             $isHttps = true;
-        } elseif (strtolower(Core::getenv('HTTP_FRONT_END_HTTPS')) === 'on') {
+        } elseif (strtolower(Core::getEnv('HTTP_FRONT_END_HTTPS')) === 'on') {
             $isHttps = true;
-        } elseif (strtolower(Core::getenv('HTTP_X_FORWARDED_PROTO')) === 'https') {
+        } elseif (strtolower(Core::getEnv('HTTP_X_FORWARDED_PROTO')) === 'https') {
             $isHttps = true;
-        } elseif (strtolower(Core::getenv('HTTP_CLOUDFRONT_FORWARDED_PROTO')) === 'https') {
+        } elseif (strtolower(Core::getEnv('HTTP_CLOUDFRONT_FORWARDED_PROTO')) === 'https') {
             // Amazon CloudFront, issue #15621
             $isHttps = true;
-        } elseif (Util::getProtoFromForwardedHeader(Core::getenv('HTTP_FORWARDED')) === 'https') {
+        } elseif (Util::getProtoFromForwardedHeader(Core::getEnv('HTTP_FORWARDED')) === 'https') {
             // RFC 7239 Forwarded header
             $isHttps = true;
-        } elseif (Core::getenv('SERVER_PORT') == 443) {
+        } elseif (Core::getEnv('SERVER_PORT') == 443) {
             $isHttps = true;
         }
 

diff --git a/src/Controllers/Preferences/ManageController.php b/src/Controllers/Preferences/ManageController.php
@@ -72,7 +72,7 @@ public function __invoke(ServerRequest $request): void
         if ($request->hasBodyParam('submit_export') && $request->getParsedBodyParam('export_type') === 'text_file') {
             // export to JSON file
             $this->response->disable();
-            $filename = 'phpMyAdmin-config-' . urlencode(Core::getenv('HTTP_HOST')) . '.json';
+            $filename = 'phpMyAdmin-config-' . urlencode(Core::getEnv('HTTP_HOST')) . '.json';
             Core::downloadHeader($filename, 'application/json');
             $settings = $this->userPreferences->load();
             echo json_encode($settings['config_data'], JSON_PRETTY_PRINT);
@@ -83,7 +83,7 @@ public function __invoke(ServerRequest $request): void
         if ($request->hasBodyParam('submit_export') && $request->getParsedBodyParam('export_type') === 'php_file') {
             // export to JSON file
             $this->response->disable();
-            $filename = 'phpMyAdmin-config-' . urlencode(Core::getenv('HTTP_HOST')) . '.php';
+            $filename = 'phpMyAdmin-config-' . urlencode(Core::getEnv('HTTP_HOST')) . '.php';
             Core::downloadHeader($filename, 'application/php');
             $settings = $this->userPreferences->load();
             echo '/* ' . __('phpMyAdmin configuration snippet') . " */\n\n";

diff --git a/src/Core.php b/src/Core.php
@@ -30,6 +30,7 @@
 use function in_array;
 use function intval;
 use function is_array;
+use function is_scalar;
 use function is_string;
 use function json_decode;
 use function mb_strpos;
@@ -203,31 +204,21 @@ public static function checkPageValidity(string $page, array $allowList = [], bo
     }
 
     /**
-     * tries to find the value for the given environment variable name
+     * Tries to find the value for the given environment variable name
      *
-     * searches in $_SERVER, $_ENV then tries getenv() and apache_getenv()
-     * in this order
+     * Searches in $_SERVER, $_ENV then tries getenv() and apache_getenv() in this order.
      *
-     * @param string $varName variable name
-     *
-     * @return string  value of $var or empty string
+     * @psalm-param non-empty-string $variableName
      */
-    public static function getenv(string $varName): string
+    public static function getEnv(string $variableName): string
     {
-        if (isset($_SERVER[$varName])) {
-            return (string) $_SERVER[$varName];
-        }
-
-        if (isset($_ENV[$varName])) {
-            return (string) $_ENV[$varName];
-        }
-
-        if (getenv($varName)) {
-            return (string) getenv($varName);
+        $value = $_SERVER[$variableName] ?? $_ENV[$variableName] ?? getenv($variableName);
+        if (is_scalar($value) && (string) $value !== '') {
+            return (string) $value;
         }
 
-        if (function_exists('apache_getenv') && apache_getenv($varName, true)) {
-            return (string) apache_getenv($varName, true);
+        if (function_exists('apache_getenv')) {
+            return (string) apache_getenv($variableName, true); // @codeCoverageIgnore
         }
 
         return '';
@@ -609,7 +600,7 @@ public static function getIp(): string|bool
          * X-Forwarded-For: client, proxy1, proxy2
          */
         // Get header content
-        $value = self::getenv($config->settings['TrustedProxies'][$directIp]);
+        $value = self::getEnv($config->settings['TrustedProxies'][$directIp]);
         // Grab first element what is client adddress
         $value = explode(',', $value)[0];
         // checks that the header contains only one IP address,

diff --git a/src/Footer.php b/src/Footer.php
@@ -145,7 +145,7 @@ public function getSelfUrl(): string
             $params['single_table'] = $_REQUEST['single_table'];
         }
 
-        return basename(Core::getenv('SCRIPT_NAME')) . Url::getCommonRaw($params);
+        return basename(Core::getEnv('SCRIPT_NAME')) . Url::getCommonRaw($params);
     }
 
     /**
@@ -239,7 +239,7 @@ public function getDisplay(): string
         if ($this->isEnabled) {
             $config = Config::getInstance();
             if (! $this->isAjax && ! $this->isMinimal) {
-                if (Core::getenv('SCRIPT_NAME') !== '') {
+                if (Core::getEnv('SCRIPT_NAME') !== '') {
                     $url = $this->getSelfUrl();
                 }
 

diff --git a/src/IpAllowDeny.php b/src/IpAllowDeny.php
@@ -252,10 +252,10 @@ private function allowDeny(string $type): bool
         $shortcuts = ['all' => '0.0.0.0/0', 'localhost' => '127.0.0.1/8'];
 
         // Provide some useful shortcuts if server gives us address:
-        if (Core::getenv('SERVER_ADDR') !== '') {
-            $shortcuts['localnetA'] = Core::getenv('SERVER_ADDR') . '/8';
-            $shortcuts['localnetB'] = Core::getenv('SERVER_ADDR') . '/16';
-            $shortcuts['localnetC'] = Core::getenv('SERVER_ADDR') . '/24';
+        if (Core::getEnv('SERVER_ADDR') !== '') {
+            $shortcuts['localnetA'] = Core::getEnv('SERVER_ADDR') . '/8';
+            $shortcuts['localnetB'] = Core::getEnv('SERVER_ADDR') . '/16';
+            $shortcuts['localnetC'] = Core::getEnv('SERVER_ADDR') . '/24';
         }
 
         foreach ($rules as $rule) {

diff --git a/src/LanguageManager.php b/src/LanguageManager.php
@@ -924,7 +924,7 @@ public function selectLanguage(): Language
         $langs = $this->availableLanguages();
 
         // try to find out user's language by checking its HTTP_ACCEPT_LANGUAGE variable;
-        $acceptedLanguages = Core::getenv('HTTP_ACCEPT_LANGUAGE');
+        $acceptedLanguages = Core::getEnv('HTTP_ACCEPT_LANGUAGE');
         if ($acceptedLanguages !== '') {
             foreach (explode(',', $acceptedLanguages) as $header) {
                 foreach ($langs as $language) {
@@ -936,7 +936,7 @@ public function selectLanguage(): Language
         }
 
         // try to find out user's language by checking its HTTP_USER_AGENT variable
-        $userAgent = Core::getenv('HTTP_USER_AGENT');
+        $userAgent = Core::getEnv('HTTP_USER_AGENT');
         if ($userAgent !== '') {
             foreach ($langs as $language) {
                 if ($language->matchesUserAgent($userAgent)) {

diff --git a/src/Plugins/Auth/AuthenticationHttp.php b/src/Plugins/Auth/AuthenticationHttp.php
@@ -105,23 +105,23 @@ public function readCredentials(): bool
         }
 
         if ($this->user === '') {
-            if (Core::getenv('PHP_AUTH_USER') !== '') {
-                $this->user = Core::getenv('PHP_AUTH_USER');
-            } elseif (Core::getenv('REMOTE_USER') !== '') {
+            if (Core::getEnv('PHP_AUTH_USER') !== '') {
+                $this->user = Core::getEnv('PHP_AUTH_USER');
+            } elseif (Core::getEnv('REMOTE_USER') !== '') {
                 // CGI, might be encoded, see below
-                $this->user = Core::getenv('REMOTE_USER');
-            } elseif (Core::getenv('REDIRECT_REMOTE_USER') !== '') {
+                $this->user = Core::getEnv('REMOTE_USER');
+            } elseif (Core::getEnv('REDIRECT_REMOTE_USER') !== '') {
                 // CGI, might be encoded, see below
-                $this->user = Core::getenv('REDIRECT_REMOTE_USER');
-            } elseif (Core::getenv('AUTH_USER') !== '') {
+                $this->user = Core::getEnv('REDIRECT_REMOTE_USER');
+            } elseif (Core::getEnv('AUTH_USER') !== '') {
                 // WebSite Professional
-                $this->user = Core::getenv('AUTH_USER');
-            } elseif (Core::getenv('HTTP_AUTHORIZATION') !== '') {
+                $this->user = Core::getEnv('AUTH_USER');
+            } elseif (Core::getEnv('HTTP_AUTHORIZATION') !== '') {
                 // IIS, might be encoded, see below
-                $this->user = Core::getenv('HTTP_AUTHORIZATION');
-            } elseif (Core::getenv('Authorization') !== '') {
+                $this->user = Core::getEnv('HTTP_AUTHORIZATION');
+            } elseif (Core::getEnv('Authorization') !== '') {
                 // FastCGI, might be encoded, see below
-                $this->user = Core::getenv('Authorization');
+                $this->user = Core::getEnv('Authorization');
             }
         }
 
@@ -131,14 +131,14 @@ public function readCredentials(): bool
         }
 
         if ($this->password === '') {
-            if (Core::getenv('PHP_AUTH_PW') !== '') {
-                $this->password = Core::getenv('PHP_AUTH_PW');
-            } elseif (Core::getenv('REMOTE_PASSWORD') !== '') {
+            if (Core::getEnv('PHP_AUTH_PW') !== '') {
+                $this->password = Core::getEnv('PHP_AUTH_PW');
+            } elseif (Core::getEnv('REMOTE_PASSWORD') !== '') {
                 // Apache/CGI
-                $this->password = Core::getenv('REMOTE_PASSWORD');
-            } elseif (Core::getenv('AUTH_PASSWORD') !== '') {
+                $this->password = Core::getEnv('REMOTE_PASSWORD');
+            } elseif (Core::getEnv('AUTH_PASSWORD') !== '') {
                 // WebSite Professional
-                $this->password = Core::getenv('AUTH_PASSWORD');
+                $this->password = Core::getEnv('AUTH_PASSWORD');
             }
         }
 

diff --git a/src/Plugins/TwoFactorPlugin.php b/src/Plugins/TwoFactorPlugin.php
@@ -143,7 +143,7 @@ public function getAppId(bool $returnUrl): string
         }
 
         if (! isset($parsed['host']) || $parsed['host'] === '') {
-            $parsed['host'] = Core::getenv('HTTP_HOST');
+            $parsed['host'] = Core::getEnv('HTTP_HOST');
         }
 
         if ($returnUrl) {

diff --git a/src/Routing/Routing.php b/src/Routing/Routing.php
@@ -222,12 +222,12 @@ public static function callSetupController(ServerRequest $request, ResponseFacto
      */
     public static function getCleanPathInfo(): string
     {
-        $pmaPhpSelf = Core::getenv('PHP_SELF');
+        $pmaPhpSelf = Core::getEnv('PHP_SELF');
         if ($pmaPhpSelf === '') {
-            $pmaPhpSelf = urldecode(Core::getenv('REQUEST_URI'));
+            $pmaPhpSelf = urldecode(Core::getEnv('REQUEST_URI'));
         }
 
-        $pathInfo = Core::getenv('PATH_INFO');
+        $pathInfo = Core::getEnv('PATH_INFO');
         if ($pathInfo !== '' && $pmaPhpSelf !== '') {
             $questionPos = mb_strpos($pmaPhpSelf, '?');
             if ($questionPos != false) {

diff --git a/src/Util.php b/src/Util.php
@@ -1385,7 +1385,7 @@ public static function expandUserString(
     ): string {
         /* Content */
         $vars = [];
-        $vars['http_host'] = Core::getenv('HTTP_HOST');
+        $vars['http_host'] = Core::getEnv('HTTP_HOST');
         $config = Config::getInstance();
         $vars['server_name'] = $config->selectedServer['host'];
         $vars['server_verbose'] = $config->selectedServer['verbose'];

diff --git a/test/classes/CoreTest.php b/test/classes/CoreTest.php
@@ -21,6 +21,7 @@
 use function _pgettext;
 use function hash;
 use function header;
+use function putenv;
 use function serialize;
 use function str_repeat;
 
@@ -716,4 +717,24 @@ public function testDownloadHeader2(): void
         $this->assertContains('Content-Transfer-Encoding: binary', $headersList);
         $this->assertNotContains('Content-Length: 0', $headersList);
     }
+
+    public function testGetEnv(): void
+    {
+        self::assertSame('', Core::getEnv('PHPMYADMIN_GET_ENV_TEST'));
+
+        $_SERVER['PHPMYADMIN_GET_ENV_TEST'] = 'value_from_server_global';
+        $_ENV['PHPMYADMIN_GET_ENV_TEST'] = 'value_from_env_global';
+        putenv('PHPMYADMIN_GET_ENV_TEST=value_from_getenv');
+
+        self::assertSame('value_from_server_global', Core::getEnv('PHPMYADMIN_GET_ENV_TEST'));
+        unset($_SERVER['PHPMYADMIN_GET_ENV_TEST']);
+
+        self::assertSame('value_from_env_global', Core::getEnv('PHPMYADMIN_GET_ENV_TEST'));
+        unset($_ENV['PHPMYADMIN_GET_ENV_TEST']);
+
+        self::assertSame('value_from_getenv', Core::getEnv('PHPMYADMIN_GET_ENV_TEST'));
+        putenv('PHPMYADMIN_GET_ENV_TEST');
+
+        self::assertSame('', Core::getEnv('PHPMYADMIN_GET_ENV_TEST'));
+    }
 }