From d772331b9173550f98a5ba8c55f576170fc1120b Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 19 Feb 2023 12:57:39 +0100 Subject: [PATCH] Fix multiple serialized values --- src/Core/Config/Model/DatabaseConfig.php | 10 ++++---- src/Core/Config/Util/SerializeUtil.php | 18 +++++++++++++-- tests/src/Core/Config/ConfigTest.php | 29 ++++++++++++++++++++++++ update.php | 20 ++++++++++++++++ 4 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/Core/Config/Model/DatabaseConfig.php b/src/Core/Config/Model/DatabaseConfig.php index 01366cf71..7167ccbf8 100644 --- a/src/Core/Config/Model/DatabaseConfig.php +++ b/src/Core/Config/Model/DatabaseConfig.php @@ -61,15 +61,13 @@ class DatabaseConfig implements IManageConfigValues foreach ($setCache->getAll() as $category => $data) { foreach ($data as $key => $value) { - $this->cache->set($category, $key, $value, Cache::SOURCE_DATA); - $this->database->insert('config', ['cat' => $category, 'k' => $key, 'v' => serialize($value)], Database::INSERT_UPDATE); + $this->set($category, $key, $value); } } foreach ($delCache->getAll() as $category => $keys) { foreach ($keys as $key => $value) { - $this->cache->delete($category, $key); - $this->database->delete('config', ['cat' => $category, 'k' => $key]); + $this->delete($category, $key); } } @@ -85,6 +83,10 @@ class DatabaseConfig implements IManageConfigValues /** {@inheritDoc} */ public function set(string $cat, string $key, $value): bool { + // In case someone or something already serialized a config entry, unserialize it first + // We serialize values just once + $value = SerializeUtil::maybeUnserialize($value); + $this->cache->set($cat, $key, $value, Cache::SOURCE_DATA); return $this->database->insert('config', ['cat' => $cat, 'k' => $key, 'v' => serialize($value)], Database::INSERT_UPDATE); } diff --git a/src/Core/Config/Util/SerializeUtil.php b/src/Core/Config/Util/SerializeUtil.php index 0a886e230..1d944f264 100644 --- a/src/Core/Config/Util/SerializeUtil.php +++ b/src/Core/Config/Util/SerializeUtil.php @@ -28,10 +28,24 @@ namespace Friendica\Core\Config\Util; */ class SerializeUtil { + /** + * Checks if the value needs to get unserialized and returns the unserialized value + * + * @param mixed $value A possible serialized value + * + * @return mixed The unserialized value + */ public static function maybeUnserialize($value) { - if (static::isSerialized($value)) { - return @unserialize(trim($value)); + // This checks for possible multiple serialized values + while (SerializeUtil::isSerialized($value)) { + $oldValue = $value; + $value = @unserialize($value); + + // If there's no change after the unserialize call, break the loop (avoid endless loops) + if ($oldValue === $value) { + break; + } } return $value; diff --git a/tests/src/Core/Config/ConfigTest.php b/tests/src/Core/Config/ConfigTest.php index 643c73754..cf13ea05a 100644 --- a/tests/src/Core/Config/ConfigTest.php +++ b/tests/src/Core/Config/ConfigTest.php @@ -537,4 +537,33 @@ class ConfigTest extends DatabaseTest self::assertEquals($assertion, $config->get($category)); } + + public function dataSerialized(): array + { + return [ + 'default' => [ + 'value' => ['test' => ['array']], + 'assertion' => ['test' => ['array']], + ], + 'issue-12803' => [ + 'value' => 's:48:"s:40:"s:32:"https://punkrock-underground.com";";";', + 'assertion' => 'https://punkrock-underground.com', + ], + 'double-serialized-array' => [ + 'value' => 's:53:"a:1:{s:9:"testArray";a:1:{s:4:"with";s:7:"entries";}}";', + 'assertion' => ['testArray' => ['with' => 'entries']], + ], + ]; + } + + /** + * @dataProvider dataSerialized + */ + public function testSerializedValues($value, $assertion) + { + $config = $this->getInstance(); + + $config->set('test', 'it', $value); + self:self::assertEquals($assertion, $config->get('test', 'it')); + } } diff --git a/update.php b/update.php index 15ef8b1dc..672c11a4f 100644 --- a/update.php +++ b/update.php @@ -1295,3 +1295,23 @@ function update_1515() DBA::update('verb', ['name' => Activity::VIEW], ['name' => 'https://joinpeertube.org/view']); return Update::SUCCESS; } + +function update_1516() +{ + // Fixes https://github.com/friendica/friendica/issues/12803 + // de-serialize multiple serialized values + $configTrans = DI::config()->beginTransaction(); + $configArray = DI::config()->getCache()->getDataBySource(Cache::SOURCE_DATA); + + foreach ($configArray as $category => $keyValues) { + if (is_array($keyValues)) { + foreach ($keyValues as $key => $value) { + $configTrans->set($category, $key, $value); + } + } + } + + $configTrans->commit(); + + return Update::SUCCESS; +}