Skip to content

Commit

Permalink
feat: add allow list for regexp; alter tests
Browse files Browse the repository at this point in the history 
now includes "-" and "_" as default allowed values next to A-Z (regexp)
  • Loading branch information
Pascal Querner committed Mar 17, 2024
1 parent 36e55c7 commit 7b5da9b
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 43 deletions.
33 changes: 22 additions & 11 deletions app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ class Mage_Core_Helper_EnvironmentConfigLoader extends Mage_Core_Helper_Abstract
protected const CONFIG_KEY_DEFAULT = 'DEFAULT';
protected const CONFIG_KEY_WEBSITES = 'WEBSITES';
protected const CONFIG_KEY_STORES = 'STORES';
/**
* To be used as regex condition
*/
protected const ALLOWED_CHARS = ['A-Z', '-', '_'];

protected array $envStore = [];

Expand Down Expand Up @@ -61,14 +65,7 @@ public function overrideEnvironment(Varien_Simplexml_Config $xmlConfig)
continue;
}

$configKeyParts = array_filter(
explode(
static::ENV_KEY_SEPARATOR,
$configKey
),
'trim'
);
list($_, $scope) = $configKeyParts;
list($configKeyParts, $scope) = $this->getConfigKey($configKey);

switch ($scope) {
case static::CONFIG_KEY_DEFAULT:
Expand All @@ -88,19 +85,33 @@ public function overrideEnvironment(Varien_Simplexml_Config $xmlConfig)
}
}

public function getConfigKey(string $configKey): array
{
$configKeyParts = array_filter(
explode(
static::ENV_KEY_SEPARATOR,
$configKey
),
'trim'
);
list($_, $scope) = $configKeyParts;
return array($configKeyParts, $scope);
}

public function isConfigKeyValid(string $configKey): bool
{
if (!str_starts_with($configKey, static::ENV_STARTS_WITH)) {
return false;
}

$sectionGroupFieldRegexp = '([A-Z_]*)';
$sectionGroupFieldRegexp = sprintf('([%s]*)', implode('', static::ALLOWED_CHARS));
$allowedChars = sprintf('[%s]', implode('', static::ALLOWED_CHARS));
$regexp = '/' . static::ENV_STARTS_WITH . static::ENV_KEY_SEPARATOR . '(WEBSITES' . static::ENV_KEY_SEPARATOR
. '[A-Z_]+|DEFAULT|STORES' . static::ENV_KEY_SEPARATOR . '[A-Z_]+)'
. $allowedChars . '+|DEFAULT|STORES' . static::ENV_KEY_SEPARATOR . $allowedChars . '+)'
. static::ENV_KEY_SEPARATOR . $sectionGroupFieldRegexp
. static::ENV_KEY_SEPARATOR . $sectionGroupFieldRegexp
. static::ENV_KEY_SEPARATOR . $sectionGroupFieldRegexp . '/';
// /OPENMAGE_CONFIG__(WEBSITES__[A-Z_]+|DEFAULT|STORES__[A-Z_]+)__([A-Z_]*)__([A-Z_]+)__([A-Z_]+)/
// /OPENMAGE_CONFIG__(WEBSITES__[A-Z-_]+|DEFAULT|STORES__[A-Z-_]+)__([A-Z-_]*)__([A-Z-_]*)__([A-Z-_]*)/

return preg_match($regexp, $configKey);
}
Expand Down
129 changes: 97 additions & 32 deletions dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ public function testBuildNodePath()
$this->assertEquals('default/general/store_information/name', $nodePath);
}

public function test_xml_has_test_strings()
{
$xmlStruct = $this->getTestXml();
$xml = new \Varien_Simplexml_Config();
$xml->loadString($xmlStruct);
$this->assertEquals('test_default', (string)$xml->getNode('default/general/store_information/name'));
$this->assertEquals('test_website', (string)$xml->getNode('websites/base/general/store_information/name'));
$this->assertEquals('test_store', (string)$xml->getNode('stores/german/general/store_information/name'));
}

/**
* @dataProvider env_overrides_correct_config_keys
* @test
Expand All @@ -41,31 +51,16 @@ public function env_overrides_for_valid_config_keys(array $config)
$xml = new \Varien_Simplexml_Config();
$xml->loadString($xmlStruct);

$this->assertEquals('test_default', (string)$xml->getNode('default/general/store_information/name'));
$this->assertEquals('test_website', (string)$xml->getNode('websites/base/general/store_information/name'));
$this->assertEquals('test_store', (string)$xml->getNode('stores/german/general/store_information/name'));

// act
$loader = new Mage_Core_Helper_EnvironmentConfigLoader();
$loader->setEnvStore([
$config['path'] => $config['value']
$config['env_path'] => $config['value']
]);
$loader->overrideEnvironment($xml);

switch ($config['case']) {
case 'DEFAULT':
$defaultValue = $xmlDefault->getNode('default/general/store_information/name');
$valueAfterOverride = $xml->getNode('default/general/store_information/name');
break;
case 'STORE':
$defaultValue = $xmlDefault->getNode('stores/german/general/store_information/name');
$valueAfterOverride = $xml->getNode('stores/german/general/store_information/name');
break;
case 'WEBSITE':
$defaultValue = $xmlDefault->getNode('websites/base/general/store_information/name');
$valueAfterOverride = $xml->getNode('websites/base/general/store_information/name');
break;
}
$configPath = $config['xml_path'];
$defaultValue = $xmlDefault->getNode($configPath);
$valueAfterOverride = $xml->getNode($configPath);

// assert
$this->assertNotEquals((string)$defaultValue, (string)$valueAfterOverride, 'Default value was not overridden.');
Expand All @@ -74,28 +69,70 @@ public function env_overrides_for_valid_config_keys(array $config)
public function env_overrides_correct_config_keys(): array
{
$defaultPath = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__STORE_INFORMATION__NAME';

$websitePath = 'OPENMAGE_CONFIG__WEBSITES__BASE__GENERAL__STORE_INFORMATION__NAME';
$websiteWithDashPath = 'OPENMAGE_CONFIG__WEBSITES__BASE-AT__GENERAL__STORE_INFORMATION__NAME';
$websiteWithUnderscorePath = 'OPENMAGE_CONFIG__WEBSITES__BASE_CH__GENERAL__STORE_INFORMATION__NAME';

$storeWithDashPath = 'OPENMAGE_CONFIG__STORES__GERMAN-AT__GENERAL__STORE_INFORMATION__NAME';
$storeWithUnderscorePath = 'OPENMAGE_CONFIG__STORES__GERMAN_CH__GENERAL__STORE_INFORMATION__NAME';
$storePath = 'OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__STORE_INFORMATION__NAME';

return [
[
'Case DEFAULT with ' . $defaultPath . ' overrides.' => [
'case' => 'DEFAULT',
'path' => $defaultPath,
'value' => 'default_new_value'
'Case DEFAULT overrides.' => [
'case' => 'DEFAULT',
'xml_path' => 'default/general/store_information/name',
'env_path' => $defaultPath,
'value' => 'default_new_value'
]
],
[
'Case STORE with ' . $storePath . ' overrides.' => [
'case' => 'STORE',
'path' => $storePath,
'value' => 'store_new_value'
'Case STORE overrides.' => [
'case' => 'STORE',
'xml_path' => 'stores/german/general/store_information/name',
'env_path' => $storePath,
'value' => 'store_new_value'
]
],
[
'Case WEBSITE with ' . $websitePath . ' overrides.' => [
'case' => 'WEBSITE',
'path' => $websitePath,
'value' => 'website_new_value'
'Case STORE overrides.' => [
'case' => 'STORE',
'xml_path' => 'stores/german-at/general/store_information/name',
'env_path' => $storeWithDashPath,
'value' => 'store_new_value'
]
],
[
'Case STORE overrides.' => [
'case' => 'STORE',
'xml_path' => 'stores/german_ch/general/store_information/name',
'env_path' => $storeWithUnderscorePath,
'value' => 'store_new_value'
]
],
[
'Case WEBSITE overrides.' => [
'case' => 'WEBSITE',
'xml_path' => 'websites/base/general/store_information/name',
'env_path' => $websitePath,
'value' => 'website_new_value'
]
],
[
'Case WEBSITE overrides.' => [
'case' => 'WEBSITE',
'xml_path' => 'websites/base_ch/general/store_information/name',
'env_path' => $websiteWithUnderscorePath,
'value' => 'website_new_value'
]
],
[
'Case WEBSITE overrides.' => [
'case' => 'WEBSITE',
'xml_path' => 'websites/base-at/general/store_information/name',
'env_path' => $websiteWithDashPath,
'value' => 'website_new_value'
]
]
];
Expand All @@ -105,7 +142,7 @@ public function env_overrides_correct_config_keys(): array
* @dataProvider env_does_not_override_on_wrong_config_keys
* @test
*/
public function env_does_not_override_for_valid_config_keys(array $config)
public function env_does_not_override_for_invalid_config_keys(array $config)
{
$xmlStruct = $this->getTestXml();

Expand Down Expand Up @@ -198,6 +235,20 @@ public function getTestXml(): string
</store_information>
</general>
</base>
<base-at>
<general>
<store_information>
<name>test_website</name>
</store_information>
</general>
</base-at>
<base_ch>
<general>
<store_information>
<name>test_website</name>
</store_information>
</general>
</base_ch>
</websites>
<stores>
<german>
Expand All @@ -207,6 +258,20 @@ public function getTestXml(): string
</store_information>
</general>
</german>
<german-at>
<general>
<store_information>
<name>test_store</name>
</store_information>
</general>
</german-at>
<german_ch>
<general>
<store_information>
<name>test_store</name>
</store_information>
</general>
</german_ch>
</stores>
</config>
XML;
Expand Down

0 comments on commit 7b5da9b

Please sign in to comment.