Skip to content

Commit

Permalink
Replace use of eval() with file inclusion
Browse files Browse the repository at this point in the history
  • Loading branch information
@Quetzacoalt91
Quetzacoalt91 committed Oct 21, 2024
1 parent 03e56f8 commit 2df6966
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 36 deletions.
2 changes: 1 addition & 1 deletion classes/Task/Update/UpdateModules.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function run(): int
$moduleSourceList = new ModuleSourceAggregate($this->container->getModuleSourceProviders());
$moduleDownloader = new ModuleDownloader($this->translator, $this->logger, $this->container->getProperty(UpgradeContainer::TMP_PATH));
$moduleUnzipper = new ModuleUnzipper($this->translator, $this->container->getZipAction(), $modulesPath);
$moduleMigration = new ModuleMigration($this->translator, $this->logger);
$moduleMigration = new ModuleMigration($this->translator, $this->logger, $this->container->getProperty(UpgradeContainer::TMP_PATH));

if ($listModules->getRemainingTotal()) {
$moduleInfos = $listModules->getNext();
Expand Down
35 changes: 18 additions & 17 deletions classes/UpgradeTools/Module/ModuleMigration.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,14 @@ class ModuleMigration
/** @var Logger */
private $logger;

public function __construct(Translator $translator, Logger $logger)
/** @var string */
private $sandboxFolder;

public function __construct(Translator $translator, Logger $logger, string $sandboxFolder)
{
$this->translator = $translator;
$this->logger = $logger;
$this->sandboxFolder = $sandboxFolder;
}

public function needMigration(ModuleMigrationContext $moduleMigrationContext): bool
Expand Down Expand Up @@ -106,13 +110,15 @@ public function runMigration(ModuleMigrationContext $moduleMigrationContext): vo
$methodName = $this->getUpgradeMethodName($migrationFilePath);

try {
$this->loadAndCallFunction($migrationFilePath, $methodName, $moduleMigrationContext);
if (!$this->loadAndCallFunction($migrationFilePath, $methodName, $moduleMigrationContext)) {
throw (new UpgradeException($this->translator->trans('[WARNING] Migration failed while running the file %s. Module %s disabled.', [basename($migrationFilePath), $moduleMigrationContext->getModuleName()])))->setSeverity(UpgradeException::SEVERITY_WARNING);
}
} catch (UpgradeException $e) {
$moduleMigrationContext->getModuleInstance()->disable();
throw $e;
} catch (Throwable $t) {
$moduleMigrationContext->getModuleInstance()->disable();
throw new UpgradeException($this->translator->trans('[WARNING] Unexpected error when trying to upgrade module %s. Module %s disabled.', [$moduleMigrationContext->getModuleName(), $moduleMigrationContext->getModuleName()]), 0, $t);
throw (new UpgradeException($this->translator->trans('[WARNING] Unexpected error when trying to upgrade module %s. Module %s disabled.', [$moduleMigrationContext->getModuleName(), $moduleMigrationContext->getModuleName()]), 0, $t))->setSeverity(UpgradeException::SEVERITY_WARNING);
}
}
}
Expand Down Expand Up @@ -147,23 +153,18 @@ private function loadAndCallFunction(string $filePath, string $methodName, Modul
{
$uniqueMethodName = $moduleMigrationContext->getModuleName() . '_' . $methodName;

$fileContent = file_get_contents($filePath);
$sandboxedFilePath = $this->sandboxFolder . DIRECTORY_SEPARATOR . $uniqueMethodName . '.php';
$pushedFileContents = file_put_contents($sandboxedFilePath, str_replace($methodName, $uniqueMethodName, file_get_contents($filePath)));

if ($fileContent === false) {
throw new UpgradeException(sprintf('[WARNING] Could not read file %s.', $filePath));
if ($pushedFileContents === false) {
throw (new UpgradeException($this->translator->trans('[WARNING] Could not write temporary file %s.', [$sandboxedFilePath])))->setSeverity(UpgradeException::SEVERITY_WARNING);
}

$fileContent = str_replace($methodName, $uniqueMethodName, $fileContent);
$fileContent = str_replace(['<?php', '?>'], '', $fileContent);

return (function () use ($fileContent, $uniqueMethodName, $moduleMigrationContext) {
eval($fileContent);

if (!function_exists($uniqueMethodName)) {
throw new UpgradeException(sprintf('[WARNING] Method %s does not exist in evaluated file.', $uniqueMethodName));
}
require_once $sandboxedFilePath;
if (!function_exists($uniqueMethodName)) {
throw (new UpgradeException($this->translator->trans('[WARNING] Method %s does not exist. Module %s disabled.', [$uniqueMethodName, $moduleMigrationContext->getModuleName()])))->setSeverity(UpgradeException::SEVERITY_WARNING);
}

return call_user_func($uniqueMethodName, $moduleMigrationContext->getModuleInstance());
})();
return call_user_func($uniqueMethodName, $moduleMigrationContext->getModuleInstance());
}
}
28 changes: 10 additions & 18 deletions tests/unit/UpgradeTools/Module/ModuleMigrationTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ class ModuleMigrationTest extends TestCase
*/
private $moduleMigration;

private static $fixtureFolder;

public static function setUpBeforeClass()
{
self::$fixtureFolder = sys_get_temp_dir() . '/fakeMigrationFilesDestination';
@mkdir(self::$fixtureFolder);
}

protected function setUp(): void
{
if (!defined('_PS_MODULE_DIR_')) {
Expand All @@ -58,7 +66,7 @@ protected function setUp(): void
});

$this->logger = $this->createMock(Logger::class);
$this->moduleMigration = new ModuleMigration($translator, $this->logger);
$this->moduleMigration = new ModuleMigration($translator, $this->logger, self::$fixtureFolder);
}

public function testNeedMigrationWithSameVersion()
Expand Down Expand Up @@ -179,22 +187,6 @@ public function testRunMigrationWithXYZDifferentFiles()
$this->moduleMigration->runMigration($moduleMigrationContext);
}

public function testRunMigrationWithSameInstanceThrowDuplicateMethod()
{
$mymodule = new \fixtures\mymodule\mymodule();
$mymodule->version = '1.1.1';
$dbVersion = '0.0.9';

$moduleMigrationContext = new ModuleMigrationContext($mymodule, $dbVersion);

$this->moduleMigration->needMigration($moduleMigrationContext);

$this->expectException(\PrestaShop\Module\AutoUpgrade\Exceptions\UpgradeException::class);
$this->expectExceptionMessage('[WARNING] Method upgrade_module_1 already exists. Migration for module mymodule aborted, you can try again later on the module manager. Module mymodule disabled.');

$this->moduleMigration->runMigration($moduleMigrationContext);
}

public function testRunMigrationWithBadUpgradeMethodName()
{
$mymodule = new \fixtures\mymodule\mymodule();
Expand All @@ -206,7 +198,7 @@ public function testRunMigrationWithBadUpgradeMethodName()
$this->moduleMigration->needMigration($moduleMigrationContext);

$this->expectException(\PrestaShop\Module\AutoUpgrade\Exceptions\UpgradeException::class);
$this->expectExceptionMessage('[WARNING] Method upgrade_module_1_2_0 does not exist. Module mymodule disabled.');
$this->expectExceptionMessage('[WARNING] Method mymodule_upgrade_module_1_2_0 does not exist. Module mymodule disabled.');

$this->moduleMigration->runMigration($moduleMigrationContext);
}
Expand Down

0 comments on commit 2df6966

Please sign in to comment.