Improve config validator

PrestaShop · Oct 11, 2024 · 80a6f42 · 80a6f42
1 parent ca793d3
commit 80a6f42
Show file tree

Hide file tree

Showing 6 changed files with 205 additions and 36 deletions.
diff --git 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
@@ -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,39 +138,39 @@ 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);
     }
 
     /**
      * @return bool True if the autoupgrade module backup should include the images
      */
     public function shouldBackupImages(): bool
     {
-        return (bool) $this->get('PS_AUTOUP_KEEP_IMAGES');
+        return filter_var($this->get('PS_AUTOUP_KEEP_IMAGES'), FILTER_VALIDATE_BOOLEAN);
     }
 
     /**
      * @return bool True if non-native modules must be disabled during upgrade
      */
     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);
     }
 
     /**
      * @return bool true if we should keep the merchant emails untouched
      */
     public function shouldKeepMails(): bool
     {
-        return (bool) $this->get('PS_AUTOUP_KEEP_MAILS');
+        return filter_var($this->get('PS_AUTOUP_KEEP_MAILS'), FILTER_VALIDATE_BOOLEAN);
     }
 
     /**
      * @return bool True if we have to set the native theme by default
      */
     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<string, mixed> $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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,117 @@
+<?php
+/**
+ * Copyright since 2007 PrestaShop SA and Contributors
+ * PrestaShop is an International Registered Trademark & Property of PrestaShop SA
+ *
+ * NOTICE OF LICENSE
+ *
+ * This source file is subject to the Academic Free License 3.0 (AFL-3.0)
+ * that is bundled with this package in the file LICENSE.md.
+ * It is also available through the world-wide-web at this URL:
+ * https://opensource.org/licenses/AFL-3.0
+ * If you did not receive a copy of the license and are unable to
+ * obtain it through the world-wide-web, please send an email
+ * to [email protected] so we can send you a copy immediately.
+ *
+ * DISCLAIMER
+ *
+ * Do not edit or add to this file if you wish to upgrade PrestaShop to newer
+ * versions in the future. If you wish to customize PrestaShop for your
+ * needs please refer to https://devdocs.prestashop.com/ for more information.
+ *
+ * @author    PrestaShop SA and Contributors <[email protected]>
+ * @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' => '']);
+    }
+}