From ece6f4da0b8d4d58f4233517bb4062683a1bfdee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sun, 2 Jun 2024 16:09:13 -0300 Subject: [PATCH] Fix routes cache validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- libraries/classes/Routing.php | 41 ++++++--------- phpstan-baseline.neon | 19 ++++++- test/classes/RoutingTest.php | 95 +++++++++++++++++++++-------------- 3 files changed, 91 insertions(+), 64 deletions(-) diff --git a/libraries/classes/Routing.php b/libraries/classes/Routing.php index 5318d5fb67..3bee79e515 100644 --- a/libraries/classes/Routing.php +++ b/libraries/classes/Routing.php @@ -9,7 +9,6 @@ use FastRoute\Dispatcher; use FastRoute\Dispatcher\GroupCountBased as DispatcherGroupCountBased; use FastRoute\RouteCollector; use FastRoute\RouteParser\Std as RouteParserStd; -use PhpMyAdmin\Controllers\HomeController; use PhpMyAdmin\Http\ServerRequest; use Psr\Container\ContainerInterface; @@ -17,7 +16,6 @@ use function __; use function file_exists; use function file_put_contents; use function htmlspecialchars; -use function is_array; use function is_readable; use function is_string; use function is_writable; @@ -73,12 +71,16 @@ class Routing // If skip cache is enabled, do not try to read the file // If no cache skipping then read it and use it - if (! $skipCache && file_exists(self::ROUTES_CACHE_FILE)) { + if ( + ! $skipCache + && file_exists(self::ROUTES_CACHE_FILE) + && isset($_SESSION['isRoutesCacheFileValid']) + && $_SESSION['isRoutesCacheFileValid'] + ) { /** @psalm-suppress MissingFile, UnresolvableInclude, MixedAssignment */ $dispatchData = require self::ROUTES_CACHE_FILE; - if (self::isRoutesCacheFileValid($dispatchData)) { - return new DispatcherGroupCountBased($dispatchData); - } + + return new DispatcherGroupCountBased($dispatchData); } $routeCollector = new RouteCollector( @@ -93,10 +95,14 @@ class Routing // If skip cache is enabled, do not try to write it // If no skip cache then try to write if write is possible if (! $skipCache && $canWriteCache) { - $writeWorks = self::writeCache( - 'get($controllerName); $controller($request, $vars); } - - /** - * @param mixed $dispatchData - * - * @psalm-assert-if-true array[] $dispatchData - */ - private static function isRoutesCacheFileValid($dispatchData): bool - { - return is_array($dispatchData) - && isset($dispatchData[0], $dispatchData[1]) - && is_array($dispatchData[0]) && is_array($dispatchData[1]) - && isset($dispatchData[0]['GET']) && is_array($dispatchData[0]['GET']) - && isset($dispatchData[0]['GET']['/']) && is_string($dispatchData[0]['GET']['/']) - && $dispatchData[0]['GET']['/'] === HomeController::class; - } } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index e3f1b43573..6a2ec29345 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -3646,7 +3646,7 @@ parameters: path: libraries/classes/File.php - - message: "#^Parameter \\#2 \\$length of function fread expects int\\<0, max\\>, int given\\.$#" + message: "#^Parameter \\#2 \\$length of function fread expects int\\<1, max\\>, int given\\.$#" count: 1 path: libraries/classes/File.php @@ -4376,10 +4376,15 @@ parameters: path: libraries/classes/Git.php - - message: "#^Parameter \\#2 \\$length of function fread expects int\\<0, max\\>, int given\\.$#" + message: "#^Parameter \\#2 \\$length of function fread expects int\\<1, max\\>, int given\\.$#" count: 1 path: libraries/classes/Git.php + - + message: "#^Strict comparison using \\=\\=\\= between string and false will always evaluate to false\\.$#" + count: 3 + path: libraries/classes/Git.php + - message: "#^Method PhpMyAdmin\\\\Header\\:\\:getJsParams\\(\\) return type has no value type specified in iterable type array\\.$#" count: 1 @@ -9030,6 +9035,11 @@ parameters: count: 1 path: libraries/classes/Util.php + - + message: "#^Strict comparison using \\=\\=\\= between string and false will always evaluate to false\\.$#" + count: 1 + path: libraries/classes/Util.php + - message: "#^Cannot cast mixed to int\\.$#" count: 1 @@ -9110,6 +9120,11 @@ parameters: count: 1 path: libraries/classes/VersionInformation.php + - + message: "#^Strict comparison using \\=\\=\\= between string and false will always evaluate to false\\.$#" + count: 1 + path: libraries/classes/WebAuthn/DataStream.php + - message: "#^Method PhpMyAdmin\\\\ZipExtension\\:\\:createFile\\(\\) has parameter \\$data with no value type specified in iterable type array\\.$#" count: 1 diff --git a/test/classes/RoutingTest.php b/test/classes/RoutingTest.php index 6ba90bb659..4a81089411 100644 --- a/test/classes/RoutingTest.php +++ b/test/classes/RoutingTest.php @@ -4,12 +4,18 @@ declare(strict_types=1); namespace PhpMyAdmin\Tests; +use FastRoute\DataGenerator\GroupCountBased as DataGeneratorGroupCountBased; use FastRoute\Dispatcher; +use FastRoute\RouteCollector; +use FastRoute\RouteParser\Std as RouteParserStd; use PhpMyAdmin\Controllers\HomeController; use PhpMyAdmin\Routing; -use function copy; +use function file_exists; +use function file_put_contents; +use function sprintf; use function unlink; +use function var_export; use const CACHE_DIR; use const TEST_PATH; @@ -19,47 +25,62 @@ use const TEST_PATH; */ class RoutingTest extends AbstractTestCase { - /** - * Test for Routing::getDispatcher - */ - public function testGetDispatcher(): void + public function testGetDispatcherWithDevEnv(): void { + $GLOBALS['cfg']['environment'] = 'development'; $expected = [Dispatcher::FOUND, HomeController::class, []]; - $cacheFilename = CACHE_DIR . 'routes.cache.php'; - $validCacheFilename = TEST_PATH . 'test/test_data/routes/routes-valid.cache.txt'; - $invalidCacheFilename = TEST_PATH . 'test/test_data/routes/routes-invalid.cache.txt'; - $GLOBALS['cfg']['environment'] = null; + self::assertSame($expected, Routing::getDispatcher()->dispatch('GET', '/')); + } + + public function testGetDispatcherWithValidCacheFile(): void + { + $GLOBALS['cfg']['environment'] = 'production'; + $_SESSION['isRoutesCacheFileValid'] = true; + + self::assertDirectoryIsWritable(CACHE_DIR); + + $routeCollector = new RouteCollector(new RouteParserStd(), new DataGeneratorGroupCountBased()); + $routeDefinitionCallback = require TEST_PATH . 'libraries/routes.php'; + $routeDefinitionCallback($routeCollector); + $routesData = sprintf('getData(), true)); + self::assertNotFalse(file_put_contents(Routing::ROUTES_CACHE_FILE, $routesData)); + + $expected = [Dispatcher::FOUND, HomeController::class, []]; + self::assertSame($expected, Routing::getDispatcher()->dispatch('GET', '/')); + } + + public function testGetDispatcherWithInvalidCacheFile(): void + { + $GLOBALS['cfg']['environment'] = 'production'; + $_SESSION['isRoutesCacheFileValid'] = null; + + self::assertDirectoryIsWritable(CACHE_DIR); + + $routeCollector = new RouteCollector(new RouteParserStd(), new DataGeneratorGroupCountBased()); + $routeDefinitionCallback = require TEST_PATH . 'libraries/routes.php'; + $routeDefinitionCallback($routeCollector); + $dispatchData = $routeCollector->getData(); + /** @psalm-suppress MixedArrayAccess */ + unset($dispatchData[0]['GET']['/']); + $routesData = sprintf('dispatch('GET', '/')); + } + + public function testGetDispatcherWithNoCacheFile(): void + { + $GLOBALS['cfg']['environment'] = 'production'; + $_SESSION['isRoutesCacheFileValid'] = null; $this->assertDirectoryIsWritable(CACHE_DIR); + if (file_exists(Routing::ROUTES_CACHE_FILE)) { + $this->assertTrue(unlink(Routing::ROUTES_CACHE_FILE)); + } - // Valid cache file. - $this->assertTrue(copy($validCacheFilename, $cacheFilename)); - $dispatcher = Routing::getDispatcher(); - $this->assertInstanceOf(Dispatcher::class, $dispatcher); - $this->assertSame($expected, $dispatcher->dispatch('GET', '/')); - $this->assertFileEquals($validCacheFilename, $cacheFilename); - - // Invalid cache file. - $this->assertTrue(copy($invalidCacheFilename, $cacheFilename)); - $dispatcher = Routing::getDispatcher(); - $this->assertInstanceOf(Dispatcher::class, $dispatcher); - $this->assertSame($expected, $dispatcher->dispatch('GET', '/')); - $this->assertFileNotEquals($invalidCacheFilename, $cacheFilename); - - // Create new cache file. - $this->assertTrue(unlink($cacheFilename)); - $this->assertFileDoesNotExist($cacheFilename); - - $dispatcher = Routing::getDispatcher(); - $this->assertInstanceOf(Dispatcher::class, $dispatcher); - $this->assertSame($expected, $dispatcher->dispatch('GET', '/')); - $this->assertFileExists($cacheFilename); - - // Without a cache file. - $GLOBALS['cfg']['environment'] = 'development'; - $dispatcher = Routing::getDispatcher(); - $this->assertInstanceOf(Dispatcher::class, $dispatcher); - $this->assertSame($expected, $dispatcher->dispatch('GET', '/')); + $expected = [Dispatcher::FOUND, HomeController::class, []]; + self::assertSame($expected, Routing::getDispatcher()->dispatch('GET', '/')); } /**