N°6618 - Router: Add protection against invalid routes cache

Combodo · Aug 2, 2023 · 85f66f5 · 85f66f5
1 parent a5c9801
commit 85f66f5
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 4 deletions.
diff --git a/sources/Service/Router/Router.php b/sources/Service/Router/Router.php
@@ -144,7 +144,16 @@ public function GetRoutes(): array
 		// Try to read from cache
 		if ($bUseCache) {
 			if (is_file($sCacheFilePath)) {
-				$aRoutes = include $sCacheFilePath;
+				$aCachedRoutes = include $sCacheFilePath;
+
+				// N°6618 - Protection against corrupted cache returning `1` instead of an array of routes
+				if (is_array($aCachedRoutes)) {
+					$aRoutes = $aCachedRoutes;
+				} else {
+					// Invalid cache force re-generation
+					// Note that even if it is re-generated corrupted again, this protection should prevent crashes
+					$bMustWriteCache = true;
+				}
 			} else {
 				$bMustWriteCache = true;
 			}

diff --git a/tests/php-unit-tests/unitary-tests/sources/Service/Router/RouterTest.php b/tests/php-unit-tests/unitary-tests/sources/Service/Router/RouterTest.php
@@ -183,10 +183,9 @@ public function testGetRoutes(string $sRoute, bool $bShouldBePresent): void
 	 * @covers \Combodo\iTop\Service\Router\Router::GetRoutes
 	 * @return void
 	 *
-	 * @details Covers the cache generation within the GetRoutes method
-	 * @since N°6618
+	 * @since N°6618 Covers that the cache isn't re-generated at each call of the GetRoutes method
 	 */
-	public function testGetRoutesCacheGeneration(): void
+	public function testGetRoutesCacheGeneratedOnlyOnce(): void
 	{
 		$oRouter = Router::GetInstance();
 		$sRoutesCacheFilePath = $this->InvokeNonPublicMethod(Router::class, 'GetCacheFileAbsPath', $oRouter, []);
@@ -229,6 +228,45 @@ public function testGetRoutesCacheGeneration(): void
 		$oConf->Set('developer_mode.enabled', $mDeveloperModePreviousValue);
 	}
 
+	/**
+	 * @covers \Combodo\iTop\Service\Router\Router::GetRoutes
+	 * @return void
+	 *
+	 * @since N°6618 Covers that the cache is re-generated correctly if corrupted
+	 */
+	public function testGetRoutesCacheRegeneratedCorrectlyIfCorrupted(): void
+	{
+		$oRouter = Router::GetInstance();
+		$sRoutesCacheFilePath = $this->InvokeNonPublicMethod(Router::class, 'GetCacheFileAbsPath', $oRouter, []);
+
+		// Developer mode must be disabled for the routes cache to be used
+		$oConf = utils::GetConfig();
+		$mDeveloperModePreviousValue = $oConf->Get('developer_mode.enabled');
+		$oConf->Set('developer_mode.enabled', false);
+
+		// Generate corrupted cache manually
+		$sFaultyStatement = 'return 1;';
+		file_put_contents($sRoutesCacheFilePath, <<<PHP
+<?php
+
+{$sFaultyStatement}
+PHP
+		);
+
+		// Retrieve routes to access / fix cache in the process
+		$aRoutes = $this->InvokeNonPublicMethod(Router::class, 'GetRoutes', $oRouter, []);
+
+		// Check that routes are an array
+		$this->assertTrue(is_array($aRoutes));
+
+		// Check that file content doesn't contain `return 1`
+		clearstatcache();
+		$this->assertStringNotContainsString($sFaultyStatement, file_get_contents($sRoutesCacheFilePath), "Cache file still contains the faulty statement ($sFaultyStatement)");
+
+		// Restore previous value for following tests
+		$oConf->Set('developer_mode.enabled', $mDeveloperModePreviousValue);
+	}
+
 	public function GetRoutesProvider(): array
 	{
 		return [