Fix routes cache validation

Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
This commit is contained in:
Maurício Meneghini Fauth 2024-06-02 16:09:13 -03:00
parent 7df158991d
commit ece6f4da0b
No known key found for this signature in database
GPG Key ID: 6A16FD38AFC89CC8
3 changed files with 91 additions and 64 deletions

View File

@ -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(
'<?php return ' . var_export($dispatchData, true) . ';'
);
if (! $writeWorks) {
/** @psalm-suppress MissingFile, UnresolvableInclude, MixedAssignment */
$cachedDispatchData = file_exists(self::ROUTES_CACHE_FILE) ? require self::ROUTES_CACHE_FILE : [];
$_SESSION['isRoutesCacheFileValid'] = $dispatchData === $cachedDispatchData;
if (
! $_SESSION['isRoutesCacheFileValid']
&& ! self::writeCache(sprintf('<?php return %s;', var_export($dispatchData, true)))
) {
$_SESSION['isRoutesCacheFileValid'] = false;
trigger_error(
sprintf(
__(
@ -190,19 +196,4 @@ class Routing
$controller = $container->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;
}
}

View File

@ -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

View File

@ -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('<?php return %s;', var_export($routeCollector->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('<?php return %s;', var_export($dispatchData, 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 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', '/'));
}
/**