Merge pull request #12814 from nupplaphil/bug/config_multi_serialize

Fix multiple serialized values

database.sql

@@ -1,6 +1,6 @@
 -- ------------------------------------------
 -- Friendica 2023.03-dev (Giant Rhubarb)
--- DB_UPDATE_VERSION 1515
+-- DB_UPDATE_VERSION 1516
 -- ------------------------------------------
 
 

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);
 	}

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 possibly 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 (static::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;

static/dbstructure.config.php

@@ -55,7 +55,7 @@
 use Friendica\Database\DBA;
 
 if (!defined('DB_UPDATE_VERSION')) {
-	define('DB_UPDATE_VERSION', 1515);
+	define('DB_UPDATE_VERSION', 1516);
 }
 
 return [

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'));
+	}
 }

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;
+}