diff --git a/libraries/classes/Url.php b/libraries/classes/Url.php index 6c3cc45a54..ed573cfdc6 100644 --- a/libraries/classes/Url.php +++ b/libraries/classes/Url.php @@ -11,13 +11,14 @@ use PhpMyAdmin\Crypto\Crypto; use function base64_decode; use function base64_encode; -use function htmlentities; use function htmlspecialchars; use function http_build_query; use function in_array; use function ini_get; use function is_array; +use function is_string; use function json_encode; +use function method_exists; use function str_contains; use function strlen; use function strtr; @@ -27,6 +28,9 @@ use function strtr; */ class Url { + /** @var string|null */ + private static $inputArgSeparator = null; + /** * Generates text with hidden inputs. * @@ -230,7 +234,7 @@ class Url $query = self::buildHttpQuery($params, $encrypt); - if (($divider !== '?' && $divider !== '&') || strlen($query) > 0) { + if (($divider !== '?' && $divider !== self::getArgSeparator()) || strlen($query) > 0) { return $divider . $query; } @@ -303,47 +307,42 @@ class Url } /** - * Returns url separator + * Returns url separator character used for separating url parts. * - * extracted from arg_separator.input as set in php.ini - * we do not use arg_separator.output to avoid problems with & and & + * Extracted from 'arg_separator.input' as set in php.ini, but prefers '&' and ';'. * - * @param string $encode whether to encode separator or not, - * currently 'none' or 'html' - * - * @return string character used for separating url parts usually ; or & + * @see https://www.php.net/manual/en/ini.core.php#ini.arg-separator.input + * @see https://www.w3.org/TR/1999/REC-html401-19991224/appendix/notes.html#h-B.2.2 */ - public static function getArgSeparator($encode = 'none') + public static function getArgSeparator(): string { - static $separator = null; - static $html_separator = null; - - if ($separator === null) { - // use separators defined by php, but prefer ';' - // as recommended by W3C - // (see https://www.w3.org/TR/1999/REC-html401-19991224/appendix - // /notes.html#h-B.2.2) - $arg_separator = (string) ini_get('arg_separator.input'); - if (str_contains($arg_separator, ';')) { - $separator = ';'; - } elseif (strlen($arg_separator) > 0) { - $separator = $arg_separator[0]; - } else { - $separator = '&'; - } - - $html_separator = htmlentities($separator); + if (is_string(self::$inputArgSeparator)) { + return self::$inputArgSeparator; } - switch ($encode) { - case 'html': - return $html_separator; - - case 'text': - case 'none': - default: - return $separator; + $separator = self::getArgSeparatorValueFromIni(); + if (! is_string($separator) || $separator === '' || str_contains($separator, '&')) { + return self::$inputArgSeparator = '&'; } + + if (str_contains($separator, ';')) { + return self::$inputArgSeparator = ';'; + } + + // uses first character + return self::$inputArgSeparator = $separator[0]; + } + + /** @return string|false */ + private static function getArgSeparatorValueFromIni() + { + /** @psalm-suppress ArgumentTypeCoercion */ + if (method_exists('PhpMyAdmin\Tests\UrlTest', 'getInputArgSeparator')) { + // phpcs:ignore SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly.ReferenceViaFullyQualifiedName + return \PhpMyAdmin\Tests\UrlTest::getInputArgSeparator(); + } + + return ini_get('arg_separator.input'); } /** @@ -352,6 +351,6 @@ class Url */ public static function getFromRoute(string $route, array $additionalParameters = []): string { - return 'index.php?route=' . $route . self::getCommon($additionalParameters, '&'); + return 'index.php?route=' . $route . self::getCommon($additionalParameters, self::getArgSeparator()); } } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 05214bf063..0a87f82ad3 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -6451,12 +6451,7 @@ parameters: path: libraries/classes/Plugins/ImportPlugin.php - - message: "#^Parameter \\#1 \\$string of function strlen expects string, mixed given\\.$#" - count: 1 - path: libraries/classes/Plugins/Schema/Dia/Dia.php - - - - message: "#^Parameter mixed of print cannot be converted to string\\.$#" + message: "#^Parameter \\#1 \\$string of function strlen expects string, int\\|string given\\.$#" count: 1 path: libraries/classes/Plugins/Schema/Dia/Dia.php @@ -6606,12 +6601,7 @@ parameters: path: libraries/classes/Plugins/Schema/Svg/RelationStatsSvg.php - - message: "#^Parameter \\#1 \\$string of function strlen expects string, mixed given\\.$#" - count: 1 - path: libraries/classes/Plugins/Schema/Svg/Svg.php - - - - message: "#^Parameter mixed of print cannot be converted to string\\.$#" + message: "#^Parameter \\#1 \\$string of function strlen expects string, int\\|string given\\.$#" count: 1 path: libraries/classes/Plugins/Schema/Svg/Svg.php diff --git a/test/classes/UrlTest.php b/test/classes/UrlTest.php index 56749105f6..9f27832a7d 100644 --- a/test/classes/UrlTest.php +++ b/test/classes/UrlTest.php @@ -5,7 +5,9 @@ declare(strict_types=1); namespace PhpMyAdmin\Tests; use PhpMyAdmin\Url; +use ReflectionProperty; +use function ini_get; use function is_string; use function parse_str; use function str_repeat; @@ -16,6 +18,9 @@ use function urldecode; */ class UrlTest extends AbstractTestCase { + /** @var string|false|null */ + private static $inputArgSeparator = null; + /** * Sets up the fixture, for example, opens a network connection. * This method is called before a test is executed. @@ -241,4 +246,49 @@ class UrlTest extends AbstractTestCase $decrypted = Url::decryptQuery($encrypted); $this->assertSame($query, $decrypted); } + + /** + * @param string|false $iniValue + * + * @dataProvider getArgSeparatorProvider + */ + public function testGetArgSeparator(string $expected, $iniValue, ?string $cacheValue): void + { + $property = new ReflectionProperty(Url::class, 'inputArgSeparator'); + $property->setAccessible(true); + $property->setValue(null, $cacheValue); + + self::$inputArgSeparator = $iniValue; + self::assertSame($expected, Url::getArgSeparator()); + + self::$inputArgSeparator = null; + $property->setValue(null, null); + } + + /** @psalm-return array */ + public static function getArgSeparatorProvider(): array + { + return [ + 'ampersand' => ['&', '&', null], + 'semicolon' => [';', ';', null], + 'prefer ampersand' => ['&', '+;&$', null], + 'prefer semicolon' => [';', '+;$', null], + 'first char' => ['+', '+$', null], + 'cache' => ['$', '&', '$'], + 'empty value' => ['&', '', null], + 'false' => ['&', false, null], + ]; + } + + /** + * Test double for ini_get('arg_separator.input') as it can't be changed using ini_set() + * + * @see Url::getArgSeparatorValueFromIni + * + * @return string|false + */ + public static function getInputArgSeparator() + { + return self::$inputArgSeparator ?? ini_get('arg_separator.input'); + } }