From 80a6f4224fe42f18f87b8dd3d9a3b78e7f1fb5e3 Mon Sep 17 00:00:00 2001 From: Morgan Pichat Date: Wed, 9 Oct 2024 17:56:12 +0200 Subject: [PATCH] Improve config validator --- classes/Parameters/ConfigurationValidator.php | 44 +++++++ classes/Parameters/UpgradeConfiguration.php | 34 +++-- classes/Task/Miscellaneous/UpdateConfig.php | 39 +++--- classes/Task/Runner/AllUpgradeTasks.php | 6 +- .../admin/AdminSelfUpgradeController.php | 1 + .../Parameters/ConfigurationValidatorTest.php | 117 ++++++++++++++++++ 6 files changed, 205 insertions(+), 36 deletions(-) create mode 100644 tests/unit/Parameters/ConfigurationValidatorTest.php diff --git a/classes/Parameters/ConfigurationValidator.php b/classes/Parameters/ConfigurationValidator.php index a5c0bdf27..f54a9666a 100644 --- a/classes/Parameters/ConfigurationValidator.php +++ b/classes/Parameters/ConfigurationValidator.php @@ -41,11 +41,25 @@ class ConfigurationValidator */ public function validate(array $array = []): void { + $isLocal = isset($array['channel']) && $array['channel'] === Upgrader::CHANNEL_LOCAL; + foreach ($array as $key => $value) { switch ($key) { case 'channel': $this->validateChannel($value); break; + case 'archive_zip': + $this->validateArchiveZip($value, $isLocal); + break; + case 'archive_xml': + $this->validateArchiveXml($value, $isLocal); + break; + case 'PS_AUTOUP_CUSTOM_MOD_DESACT': + case 'PS_AUTOUP_KEEP_MAILS': + case 'PS_AUTOUP_KEEP_IMAGES': + case 'PS_DISABLE_OVERRIDES': + $this->validateBool($value, $key); + break; } } } @@ -59,4 +73,34 @@ private function validateChannel(string $channel): void throw new UnexpectedValueException('Unknown channel ' . $channel); } } + + /** + * @throws UnexpectedValueException + */ + private function validateArchiveZip(string $zip, bool $isLocal): void + { + if ($isLocal && empty($zip)) { + throw new UnexpectedValueException('No zip archive provided'); + } + } + + /** + * @throws UnexpectedValueException + */ + private function validateArchiveXml(string $xml, bool $isLocal): void + { + if ($isLocal && empty($xml)) { + throw new UnexpectedValueException('No xml archive provided'); + } + } + + /** + * @throws UnexpectedValueException + */ + private function validateBool(string $boolValue, string $key): void + { + if (filter_var($boolValue, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) === null) { + throw new UnexpectedValueException('Value must be a boolean for ' . $key); + } + } } diff --git a/classes/Parameters/UpgradeConfiguration.php b/classes/Parameters/UpgradeConfiguration.php index f1bf586f2..2c04a49c6 100644 --- a/classes/Parameters/UpgradeConfiguration.php +++ b/classes/Parameters/UpgradeConfiguration.php @@ -42,17 +42,19 @@ class UpgradeConfiguration extends ArrayCollection const UPGRADE_CONST_KEYS = [ 'PS_AUTOUP_CUSTOM_MOD_DESACT', 'PS_AUTOUP_CHANGE_DEFAULT_THEME', - 'PS_AUTOUP_UPDATE_RTL_FILES', 'PS_AUTOUP_KEEP_MAILS', 'PS_AUTOUP_BACKUP', 'PS_AUTOUP_KEEP_IMAGES', 'PS_DISABLE_OVERRIDES', + 'channel', + 'archive_zip', + 'archive_xml', + 'archive_version_num', ]; const PS_CONST_DEFAULT_VALUE = [ 'PS_AUTOUP_CUSTOM_MOD_DESACT' => 1, 'PS_AUTOUP_CHANGE_DEFAULT_THEME' => 0, - 'PS_AUTOUP_UPDATE_RTL_FILES' => 1, 'PS_AUTOUP_KEEP_MAILS' => 0, 'PS_AUTOUP_BACKUP' => 1, 'PS_AUTOUP_KEEP_IMAGES' => 1, @@ -136,7 +138,7 @@ public function getMaxFileToBackup(): int public function shouldBackupFilesAndDatabase(): bool { - return (bool) $this->get('PS_AUTOUP_BACKUP'); + return filter_var($this->get('PS_AUTOUP_BACKUP'), FILTER_VALIDATE_BOOLEAN); } /** @@ -144,7 +146,7 @@ public function shouldBackupFilesAndDatabase(): bool */ public function shouldBackupImages(): bool { - return (bool) $this->get('PS_AUTOUP_KEEP_IMAGES'); + return filter_var($this->get('PS_AUTOUP_KEEP_IMAGES'), FILTER_VALIDATE_BOOLEAN); } /** @@ -152,7 +154,7 @@ public function shouldBackupImages(): bool */ public function shouldDeactivateCustomModules(): bool { - return (bool) $this->get('PS_AUTOUP_CUSTOM_MOD_DESACT'); + return filter_var($this->get('PS_AUTOUP_CUSTOM_MOD_DESACT'), FILTER_VALIDATE_BOOLEAN); } /** @@ -160,7 +162,7 @@ public function shouldDeactivateCustomModules(): bool */ public function shouldKeepMails(): bool { - return (bool) $this->get('PS_AUTOUP_KEEP_MAILS'); + return filter_var($this->get('PS_AUTOUP_KEEP_MAILS'), FILTER_VALIDATE_BOOLEAN); } /** @@ -168,7 +170,7 @@ public function shouldKeepMails(): bool */ public function shouldSwitchToDefaultTheme(): bool { - return (bool) $this->get('PS_AUTOUP_CHANGE_DEFAULT_THEME'); + return filter_var($this->get('PS_AUTOUP_CHANGE_DEFAULT_THEME'), FILTER_VALIDATE_BOOLEAN); } public static function isOverrideAllowed(): bool @@ -201,15 +203,25 @@ public static function updatePSDisableOverrides(bool $value): void * @throws UnexpectedValueException */ public function merge(array $array = []): void + { + foreach ($array as $key => $value) { + $this->set($key, $value); + } + } + + /** + * @param array $array + * + * @return void + * + * @throws UnexpectedValueException + */ + public function validate(array $array = []): void { if ($this->validator === null) { $this->validator = new ConfigurationValidator(); } $this->validator->validate($array); - - foreach ($array as $key => $value) { - $this->set($key, $value); - } } } diff --git a/classes/Task/Miscellaneous/UpdateConfig.php b/classes/Task/Miscellaneous/UpdateConfig.php index 6377cbc5f..b39368a4d 100644 --- a/classes/Task/Miscellaneous/UpdateConfig.php +++ b/classes/Task/Miscellaneous/UpdateConfig.php @@ -63,14 +63,23 @@ public function run(): int // Was coming from AdminSelfUpgrade::currentParams before $configurationData = $this->getConfigurationData(); $config = []; - // update channel - if (isset($configurationData['channel'])) { - $config['channel'] = $configurationData['channel']; - $config['archive_zip'] = Upgrader::DEFAULT_FILENAME; + + foreach (UpgradeConfiguration::UPGRADE_CONST_KEYS as $key) { + if (!isset($configurationData[$key])) { + continue; + } + // The PS_DISABLE_OVERRIDES variable must only be updated on the database side + if ($key === 'PS_DISABLE_OVERRIDES') { + UpgradeConfiguration::updatePSDisableOverrides((bool) $configurationData[$key]); + } else { + $config[$key] = $configurationData[$key]; + } } - if (!empty($configurationData['archive_zip'])) { - $file = $configurationData['archive_zip']; + $this->container->getUpgradeConfiguration()->validate($config); + + if ($config['channel'] === Upgrader::CHANNEL_LOCAL) { + $file = $config['archive_zip']; $fullFilePath = $this->container->getProperty(UpgradeContainer::DOWNLOAD_PATH) . DIRECTORY_SEPARATOR . $file; if (!file_exists($fullFilePath)) { $this->setErrorFlag(); @@ -88,7 +97,7 @@ public function run(): int return ExitCode::FAIL; } - $xmlFile = $configurationData['archive_xml']; + $xmlFile = $config['archive_xml']; $fullXmlPath = $this->container->getProperty(UpgradeContainer::DOWNLOAD_PATH) . DIRECTORY_SEPARATOR . $xmlFile; if (!empty($xmlFile) && !file_exists($fullXmlPath)) { $this->setErrorFlag(); @@ -113,26 +122,10 @@ public function run(): int return ExitCode::FAIL; } - $config['channel'] = Upgrader::CHANNEL_LOCAL; - $config['archive_zip'] = $configurationData['archive_zip']; $config['archive_version_num'] = $targetVersion; - $config['archive_xml'] = $configurationData['archive_xml']; - $this->logger->info($this->translator->trans('Upgrade process will use archive.')); } - foreach (UpgradeConfiguration::UPGRADE_CONST_KEYS as $key) { - if (!isset($configurationData[$key])) { - continue; - } - // The PS_DISABLE_OVERRIDES variable must only be updated on the database side - if ($key === 'PS_DISABLE_OVERRIDES') { - UpgradeConfiguration::updatePSDisableOverrides((bool) $configurationData[$key]); - } else { - $config[$key] = $configurationData[$key]; - } - } - if (!$this->writeConfig($config)) { $this->setErrorFlag(); $this->logger->info($this->translator->trans('Error on saving configuration')); diff --git a/classes/Task/Runner/AllUpgradeTasks.php b/classes/Task/Runner/AllUpgradeTasks.php index 8fbabd5eb..7e7d6814c 100644 --- a/classes/Task/Runner/AllUpgradeTasks.php +++ b/classes/Task/Runner/AllUpgradeTasks.php @@ -61,11 +61,13 @@ public function setOptions(array $options): void } if (!empty($options['channel'])) { - $this->container->getUpgradeConfiguration()->merge([ + $config = [ 'channel' => $options['channel'], // Switch on default theme if major upgrade (i.e: 1.6 -> 1.7) 'PS_AUTOUP_CHANGE_DEFAULT_THEME' => ($options['channel'] === 'major'), - ]); + ]; + $this->container->getUpgradeConfiguration()->validate($config); + $this->container->getUpgradeConfiguration()->merge($config); } if (!empty($options['data'])) { diff --git a/controllers/admin/AdminSelfUpgradeController.php b/controllers/admin/AdminSelfUpgradeController.php index dd881d596..60ce02324 100644 --- a/controllers/admin/AdminSelfUpgradeController.php +++ b/controllers/admin/AdminSelfUpgradeController.php @@ -402,6 +402,7 @@ private function handleCustomSubmitAutoUpgradeForm() } $UpConfig = $this->upgradeContainer->getUpgradeConfiguration(); + $UpConfig->validate($config); $UpConfig->merge($config); if ($this->upgradeContainer->getUpgradeConfigurationStorage()->save( diff --git a/tests/unit/Parameters/ConfigurationValidatorTest.php b/tests/unit/Parameters/ConfigurationValidatorTest.php new file mode 100644 index 000000000..7560946f3 --- /dev/null +++ b/tests/unit/Parameters/ConfigurationValidatorTest.php @@ -0,0 +1,117 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/AFL-3.0 Academic Free License 3.0 (AFL-3.0) + */ + +namespace Parameters; + +use PHPUnit\Framework\TestCase; +use PrestaShop\Module\AutoUpgrade\Parameters\ConfigurationValidator; +use UnexpectedValueException; + +class ConfigurationValidatorTest extends TestCase +{ + public function testValidateChannelSuccess() + { + $validator = new ConfigurationValidator(); + + $validator->validate(['channel' => 'online']); + $validator->validate(['channel' => 'local']); + } + + public function testValidateChannelFail() + { + $validator = new ConfigurationValidator(); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('Unknown channel toto'); + + $validator->validate(['channel' => 'toto']); + } + + public function testValidateZipSuccess() + { + $validator = new ConfigurationValidator(); + + $validator->validate(['archive_zip' => 'toto']); + $validator->validate(['channel' => 'local', 'archive_zip' => 'toto']); + $validator->validate(['channel' => 'online', 'archive_zip' => '']); + } + + public function testValidateZipFail() + { + $validator = new ConfigurationValidator(); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('No zip archive provided'); + + $validator->validate(['channel' => 'local', 'archive_zip' => '']); + } + + public function testValidateXmlSuccess() + { + $validator = new ConfigurationValidator(); + + $validator->validate(['archive_xml' => 'toto']); + $validator->validate(['channel' => 'local', 'archive_xml' => 'toto']); + $validator->validate(['channel' => 'online', 'archive_xml' => '']); + } + + public function testValidateXmlFail() + { + $validator = new ConfigurationValidator(); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('No xml archive provided'); + + $validator->validate(['channel' => 'local', 'archive_xml' => '']); + } + + public function testValidateBoolSuccess() + { + $validator = new ConfigurationValidator(); + + $validator->validate(['PS_AUTOUP_CUSTOM_MOD_DESACT' => '1']); + $validator->validate(['PS_AUTOUP_CUSTOM_MOD_DESACT' => '0']); + $validator->validate(['PS_AUTOUP_CUSTOM_MOD_DESACT' => 'true']); + $validator->validate(['PS_AUTOUP_CUSTOM_MOD_DESACT' => 'false']); + $validator->validate(['PS_AUTOUP_CUSTOM_MOD_DESACT' => 'on']); + $validator->validate(['PS_AUTOUP_CUSTOM_MOD_DESACT' => 'off']); + } + + public function testValidateBoolFail() + { + $validator = new ConfigurationValidator(); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('Value must be a boolean for PS_AUTOUP_CUSTOM_MOD_DESACT'); + + $validator->validate(['PS_AUTOUP_CUSTOM_MOD_DESACT' => 'toto']); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('Value must be a boolean for PS_AUTOUP_CUSTOM_MOD_DESACT'); + + $validator->validate(['PS_AUTOUP_CUSTOM_MOD_DESACT' => '']); + } +}