From 5d16076e40227aeaea587b897ff03407bd31b774 Mon Sep 17 00:00:00 2001
From: Benjamin Franzke <ben@bnf.dev>
Date: Sun, 8 Sep 2024 19:43:45 +0200
Subject: [PATCH] [BUGFIX] Fix access to array values in SiteSettings->get()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
If settings definitions are configured to use the type `stringlist`,
it is expected that `SiteSettings->get('setting.key')` returns
the array of strings, but failed for namespaced keys (separated by at
least one dot) because of array-flattening.
Array-flattening does only provide access to concrete array values,
as `ArrayUtility::flattenPlain` transform a settings tree like
```
[
'foo' => [
'bar' => ['a', 'b'],
],
],
```
…into a flat list like:
```
[
'foo.bar.0' => 'a',
'foo.bar.1' => 'b',
]
```
That means the key `foo.bar` is considered as not existent.
To provide access to actively defined settings key, a new
settings map is composed from the list of active setting
definition keys. For compatibility reasons the old
array flattening is still used for:
a) undefined settings
b) access to sub-values (as it worked before)
Releases: main
Resolves: #104858
Change-Id: Id3304d4673f778ef8a78381dd317816c0e8f1a13
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/85924
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Benni Mack <benni@typo3.org>
Reviewed-by: Benjamin Franzke <ben@bnf.dev>
Tested-by: Benjamin Franzke <ben@bnf.dev>
---
.../Configuration/SiteConfiguration.php | 3 +-
.../sysext/core/Classes/Site/Entity/Site.php | 3 +-
.../core/Classes/Site/Entity/SiteSettings.php | 42 +++++++++++---
.../core/Classes/Site/SiteSettingsFactory.php | 47 ++++++++++++----
.../Configuration/Sets/Set1/config.yaml | 2 +
.../Sets/Set1/settings.definitions.yaml | 5 ++
.../test_site_settings/composer.json | 14 +++++
.../test_site_settings/ext_emconf.php | 21 +++++++
.../Site/Entity/SiteSettingsTest.php | 56 +++++++++++++++++++
.../TypoScript/PageTsConfigFactoryTest.php | 4 +-
10 files changed, 176 insertions(+), 21 deletions(-)
create mode 100644 typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/Configuration/Sets/Set1/config.yaml
create mode 100644 typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/Configuration/Sets/Set1/settings.definitions.yaml
create mode 100644 typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/composer.json
create mode 100644 typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/ext_emconf.php
create mode 100644 typo3/sysext/core/Tests/Functional/Site/Entity/SiteSettingsTest.php
diff --git a/typo3/sysext/core/Classes/Configuration/SiteConfiguration.php b/typo3/sysext/core/Classes/Configuration/SiteConfiguration.php
index 54f0c8b6629f..1da52181a15b 100644
--- a/typo3/sysext/core/Classes/Configuration/SiteConfiguration.php
+++ b/typo3/sysext/core/Classes/Configuration/SiteConfiguration.php
@@ -158,7 +158,8 @@ class SiteConfiguration implements SingletonInterface
foreach ($siteConfiguration as $identifier => $configuration) {
// cast $identifier to string, as the identifier can potentially only consist of (int) digit numbers
$identifier = (string)$identifier;
- $siteSettings = new SiteSettings($configuration['settings'] ?? []);
+ $inlineSettings = $configuration['settings'] ?? [];
+ $siteSettings = SiteSettings::createFromSettingsTree($inlineSettings);
$siteTypoScript = $this->getSiteTypoScript($identifier);
$rootPageId = (int)($configuration['rootPageId'] ?? 0);
diff --git a/typo3/sysext/core/Classes/Site/Entity/Site.php b/typo3/sysext/core/Classes/Site/Entity/Site.php
index a2a0ca0b995f..e237fbaeb270 100644
--- a/typo3/sysext/core/Classes/Site/Entity/Site.php
+++ b/typo3/sysext/core/Classes/Site/Entity/Site.php
@@ -96,7 +96,8 @@ class Site implements SiteInterface
$this->identifier = $identifier;
$this->rootPageId = $rootPageId;
if ($settings === null) {
- $settings = new SiteSettings($configuration['settings'] ?? []);
+ // @todo deprecate null settings argument
+ $settings = SiteSettings::createFromSettingsTree($configuration['settings'] ?? []);
}
$this->settings = $settings;
$this->typoscript = $typoscript;
diff --git a/typo3/sysext/core/Classes/Site/Entity/SiteSettings.php b/typo3/sysext/core/Classes/Site/Entity/SiteSettings.php
index c16bd81205ad..918efa36a221 100644
--- a/typo3/sysext/core/Classes/Site/Entity/SiteSettings.php
+++ b/typo3/sysext/core/Classes/Site/Entity/SiteSettings.php
@@ -28,29 +28,36 @@ use TYPO3\CMS\Core\Utility\ArrayUtility;
final readonly class SiteSettings extends Settings implements \JsonSerializable
{
private array $flatSettings;
+ private array $settingsTree;
- public function __construct(array $settings)
+ public function __construct(array $settings, array $settingsTree, array $flatSettings)
{
parent::__construct($settings);
- $this->flatSettings = $this->isEmpty() ? [] : ArrayUtility::flattenPlain($settings);
+ $this->settingsTree = $settingsTree;
+ $this->flatSettings = $flatSettings;
}
public function has(string $identifier): bool
{
- return isset($this->settings[$identifier]) || isset($this->flatSettings[$identifier]);
+ return isset($this->settings[$identifier]) || isset($this->settingsTree[$identifier]) || isset($this->flatSettings[$identifier]);
}
public function isEmpty(): bool
{
- return $this->settings === [];
+ return $this->settingsTree === [];
}
public function get(string $identifier, mixed $defaultValue = null): mixed
{
- return $this->settings[$identifier] ?? $this->flatSettings[$identifier] ?? $defaultValue;
+ return $this->settings[$identifier] ?? $this->settingsTree[$identifier] ?? $this->flatSettings[$identifier] ?? $defaultValue;
}
public function getAll(): array
+ {
+ return $this->settingsTree;
+ }
+
+ public function getMap(): array
{
return $this->settings;
}
@@ -65,8 +72,29 @@ final readonly class SiteSettings extends Settings implements \JsonSerializable
return json_encode($this->settings);
}
- public static function __set_state(array $state): self
+ /**
+ * @internal
+ */
+ public static function create(array $settingsMap, array $settingsTree): self
+ {
+ $flatSettings = $settingsTree === [] ? [] : ArrayUtility::flattenPlain($settingsTree);
+ return new self(
+ settings: $settingsMap,
+ settingsTree: $settingsTree,
+ flatSettings: $flatSettings,
+ );
+ }
+
+ /**
+ * @internal
+ */
+ public static function createFromSettingsTree(array $settingsTree): self
{
- return new self($state['settings'] ?? []);
+ $flatSettings = $settingsTree === [] ? [] : ArrayUtility::flattenPlain($settingsTree);
+ return new self(
+ settings: [],
+ settingsTree: $settingsTree,
+ flatSettings: $flatSettings,
+ );
}
}
diff --git a/typo3/sysext/core/Classes/Site/SiteSettingsFactory.php b/typo3/sysext/core/Classes/Site/SiteSettingsFactory.php
index 6ce2e4730d49..5413f36bbabf 100644
--- a/typo3/sysext/core/Classes/Site/SiteSettingsFactory.php
+++ b/typo3/sysext/core/Classes/Site/SiteSettingsFactory.php
@@ -59,7 +59,11 @@ readonly class SiteSettingsFactory
} catch (\Error) {
}
- $settings = $this->createSettings($siteIdentifier, $siteConfiguration);
+ $settings = $this->createSettings(
+ $siteConfiguration['dependencies'] ?? [],
+ $siteIdentifier,
+ $siteConfiguration['settings'] ?? [],
+ );
$this->cache->set($cacheIdentifier, 'return ' . var_export($settings, true) . ';');
return $settings;
}
@@ -72,12 +76,29 @@ readonly class SiteSettingsFactory
* implementing a GUI for the settings - which should either get a dedicated method or a flag to control if
* placeholder should be resolved during yaml file loading or not. The SiteConfiguration save action currently
* avoid calling this method.
+ * @internal
*/
- public function createSettings(string $siteIdentifier, array $siteConfiguration): SiteSettings
+ public function createSettings(array $sets = [], ?string $siteIdentifier = null, array $inlineSettings = []): SiteSettings
{
- $sets = $siteConfiguration['dependencies'] ?? [];
- $settings = [];
+ $settingsTree = [];
+ if ($siteIdentifier !== null) {
+ $fileName = $this->configPath . '/' . $siteIdentifier . '/' . $this->settingsFileName;
+ if (file_exists($fileName)) {
+ $settingsTree = $this->yamlFileLoader->load(GeneralUtility::fixWindowsFilePath($fileName));
+ } else {
+ $settingsTree = $inlineSettings;
+ }
+ }
+ return $this->composeSettings($settingsTree, $sets);
+ }
+
+ /**
+ * @internal
+ */
+ public function composeSettings(array $settingsTree, array $sets): SiteSettings
+ {
+ $settings = [];
$definitions = [];
$activeSets = [];
if (is_array($sets) && $sets !== []) {
@@ -98,16 +119,20 @@ readonly class SiteSettingsFactory
ArrayUtility::mergeRecursiveWithOverrule($settings, $this->validateSettings($set->settings, $definitions));
}
- $fileName = $this->configPath . '/' . $siteIdentifier . '/' . $this->settingsFileName;
- if (file_exists($fileName)) {
- $siteSettings = $this->yamlFileLoader->load(GeneralUtility::fixWindowsFilePath($fileName));
- } else {
- $siteSettings = $siteConfiguration['settings'] ?? [];
+ if ($settingsTree !== []) {
+ ArrayUtility::mergeRecursiveWithOverrule($settings, $this->validateSettings($settingsTree, $definitions));
}
- ArrayUtility::mergeRecursiveWithOverrule($settings, $this->validateSettings($siteSettings, $definitions));
+ /** @var array<string, string|int|float|bool|array|null> $settingsMap */
+ $settingsMap = [];
+ foreach ($definitions as $settingDefinition) {
+ $settingsMap[$settingDefinition->key] = ArrayUtility::getValueByPath($settings, $settingDefinition->key, '.');
+ }
- return new SiteSettings($settings);
+ return SiteSettings::create(
+ settingsMap: $settingsMap,
+ settingsTree: $settings,
+ );
}
protected function validateSettings(array $settings, array $definitions): array
diff --git a/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/Configuration/Sets/Set1/config.yaml b/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/Configuration/Sets/Set1/config.yaml
new file mode 100644
index 000000000000..0593413cc9b9
--- /dev/null
+++ b/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/Configuration/Sets/Set1/config.yaml
@@ -0,0 +1,2 @@
+name: typo3tests/site-settings-set-1
+label: Test Settings Set Fixture 1
diff --git a/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/Configuration/Sets/Set1/settings.definitions.yaml b/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/Configuration/Sets/Set1/settings.definitions.yaml
new file mode 100644
index 000000000000..1778627469da
--- /dev/null
+++ b/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/Configuration/Sets/Set1/settings.definitions.yaml
@@ -0,0 +1,5 @@
+settings:
+ foo.bar.baz:
+ type: stringlist
+ default: ['a', 'b']
+ label: FooBar
diff --git a/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/composer.json b/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/composer.json
new file mode 100644
index 000000000000..f88efd7bb971
--- /dev/null
+++ b/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/composer.json
@@ -0,0 +1,14 @@
+{
+ "name": "typo3tests/test-site-settings",
+ "type": "typo3-cms-extension",
+ "description": "This extension contains site settings fixtures.",
+ "license": "GPL-2.0-or-later",
+ "require": {
+ "typo3/cms-core": "13.3.*@dev"
+ },
+ "extra": {
+ "typo3/cms": {
+ "extension-key": "test_site_settings"
+ }
+ }
+}
diff --git a/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/ext_emconf.php b/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/ext_emconf.php
new file mode 100644
index 000000000000..bb225a67cb7e
--- /dev/null
+++ b/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings/ext_emconf.php
@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+$EM_CONF[$_EXTKEY] = [
+ 'title' => 'This extension contains site settings fixtures.',
+ 'description' => 'This extension contains site settings fixtures.',
+ 'category' => 'example',
+ 'version' => '13.3.0',
+ 'state' => 'beta',
+ 'author' => 'Benjamin Franzke',
+ 'author_email' => 'ben@bnf.dev',
+ 'author_company' => '',
+ 'constraints' => [
+ 'depends' => [
+ 'typo3' => '13.3.0',
+ ],
+ 'conflicts' => [],
+ 'suggests' => [],
+ ],
+];
diff --git a/typo3/sysext/core/Tests/Functional/Site/Entity/SiteSettingsTest.php b/typo3/sysext/core/Tests/Functional/Site/Entity/SiteSettingsTest.php
new file mode 100644
index 000000000000..d5b7225e3d59
--- /dev/null
+++ b/typo3/sysext/core/Tests/Functional/Site/Entity/SiteSettingsTest.php
@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Tests\Functional\Site\Entity;
+
+use PHPUnit\Framework\Attributes\Test;
+use TYPO3\CMS\Core\Site\SiteSettingsFactory;
+use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
+
+final class SiteSettingsTest extends FunctionalTestCase
+{
+ protected array $testExtensionsToLoad = [
+ 'typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_site_settings',
+ ];
+
+ #[Test]
+ public function retrieveArraySetting(): void
+ {
+ $siteSettingsFactory = $this->get(SiteSettingsFactory::class);
+ $settings = $siteSettingsFactory->composeSettings([], ['typo3tests/site-settings-set-1']);
+
+ // Available because of definition from settings.definitions.yaml
+ self::assertTrue($settings->has('foo.bar.baz'));
+ self::assertSame(['a', 'b'], $settings->get('foo.bar.baz'));
+ // Available because of legacy array-flattening
+ self::assertTrue($settings->has('foo.bar.baz.0'));
+ self::assertSame('a', $settings->get('foo.bar.baz.0'));
+ self::assertTrue($settings->has('foo.bar.baz.1'));
+ self::assertSame('b', $settings->get('foo.bar.baz.1'));
+ }
+
+ #[Test]
+ public function noAccessUndefinedArray(): void
+ {
+ $siteSettingsFactory = $this->get(SiteSettingsFactory::class);
+ $settings = $siteSettingsFactory->composeSettings([], ['typo3tests/site-settings-set-1']);
+
+ // Not available because there is no settings key named `foo.bar`
+ self::assertFalse($settings->has('foo.bar'));
+ self::assertNull($settings->get('foo.bar'));
+ }
+}
diff --git a/typo3/sysext/core/Tests/Functional/TypoScript/PageTsConfigFactoryTest.php b/typo3/sysext/core/Tests/Functional/TypoScript/PageTsConfigFactoryTest.php
index 79cbc44f0321..dce9b17a032b 100644
--- a/typo3/sysext/core/Tests/Functional/TypoScript/PageTsConfigFactoryTest.php
+++ b/typo3/sysext/core/Tests/Functional/TypoScript/PageTsConfigFactoryTest.php
@@ -157,7 +157,9 @@ final class PageTsConfigFactoryTest extends FunctionalTestCase
],
];
$subject = $this->get(PageTsConfigFactory::class);
- $siteSettings = new SiteSettings(['aSiteSetting' => 'aSiteSettingValue']);
+ $siteSettings = SiteSettings::createFromSettingsTree(
+ ['aSiteSetting' => 'aSiteSettingValue'],
+ );
$site = new Site('siteIdentifier', 1, [], $siteSettings);
$pageTsConfig = $subject->create($rootLine, $site);
self::assertSame('aSiteSettingValue', $pageTsConfig->getPageTsConfigArray()['siteSetting']);
--
GitLab