Fix array-to-string conversion in ThemeConfig

Model/Activity/SystemConfig.php

@@ -28,6 +28,11 @@ class SystemConfig implements ModelInterface
 {
     public const MODULE_SYSTEM_CONFIGURATION = 'system_configuration';
 
+    /**
+     * Placeholder for values that cannot be JSON encoded
+     */
+    private const UNSERIALIZABLE_VALUE = '[unserializable]';
+
     public function __construct(
         protected readonly DataObject $dataObject,
         protected readonly ValueFactory $valueFactory,
@@ -155,6 +160,8 @@ private function flattenValue(mixed $value): string
             return implode(',', $value);
         }
 
-        return (string)json_encode($value, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
+        $encoded = json_encode($value, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
+
+        return $encoded === false ? self::UNSERIALIZABLE_VALUE : $encoded;
     }
 }

Model/Activity/ThemeConfig.php

@@ -25,6 +25,11 @@
  */
 class ThemeConfig implements ModelInterface
 {
+    /**
+     * Placeholder for values that cannot be JSON encoded
+     */
+    private const UNSERIALIZABLE_VALUE = '[unserializable]';
+
     public function __construct(
         protected readonly DataObject $dataObject,
         protected readonly ConfigCollectionFactory $configCollectionFactory,
@@ -119,7 +124,7 @@ public function collectAdditionalData(array $oldData, array $newData, array $fie
     }
 
     /**
-     * Normalize a scalar value to a string for comparison
+     * Normalize a value to a string for comparison
      *
      * @param mixed $value Raw value
      * @return string Normalized string
@@ -130,6 +135,58 @@ private function normalizeValue(mixed $value): string
             return '';
         }
 
+        if (is_array($value)) {
+            return $this->flattenPostedValue($value);
+        }
+
         return (string)$value;
     }
+
+    /**
+     * Flatten an array posted by the design config form to a comparable string
+     *
+     * Image uploader fields (favicon, logos) post a list of file descriptors
+     * rather than a scalar; Theme\Model\Design\Backend\File::afterLoad() puts the
+     * stored config value in the descriptor's "file" key and its basename in
+     * "name", so prefer "file" to compare against core_config_data.
+     *
+     * Deliberately not the same as the sibling SystemConfig::flattenValue():
+     * that one sees backend model values rather than posted form data and has
+     * no file descriptors to unwrap.
+     *
+     * @param array<mixed> $value Raw value
+     * @return string Normalized string
+     */
+    private function flattenPostedValue(array $value): string
+    {
+        if (!array_is_list($value)) {
+            return $this->encodeValue($value);
+        }
+
+        $parts = [];
+        foreach ($value as $item) {
+            if (!is_array($item)) {
+                $parts[] = is_scalar($item) ? (string)$item : $this->encodeValue($item);
+                continue;
+            }
+
+            $file = $item['file'] ?? $item['name'] ?? null;
+            $parts[] = is_scalar($file) ? (string)$file : $this->encodeValue($item);
+        }
+
+        return implode(',', $parts);
+    }
+
+    /**
+     * Encode a value that has no meaningful scalar representation
+     *
+     * @param mixed $value Raw value
+     * @return string Encoded value, or a placeholder if it cannot be encoded
+     */
+    private function encodeValue(mixed $value): string
+    {
+        $encoded = json_encode($value, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
+
+        return $encoded === false ? self::UNSERIALIZABLE_VALUE : $encoded;
+    }
 }

Test/Unit/Model/Activity/SystemConfigTest.php

@@ -215,4 +215,24 @@ public function testGetEditDataFlattensNestedArrayToJson(): void
         $this->assertArrayHasKey('general/country/allow', $result);
         $this->assertSame('{"key":"value"}', $result['general/country/allow']['new_value']);
     }
+
+    public function testGetEditDataUsesPlaceholderForUnserializableValue(): void
+    {
+        $model = new DataObject([
+            'path' => 'general/country/allow',
+            'value' => ['broken' => "\xB1\x31"],
+        ]);
+        $model->setOrigData([
+            'country' => [
+                'fields' => [
+                    'allow' => ['value' => 'old'],
+                ],
+            ],
+        ]);
+
+        $result = $this->systemConfig->getEditData($model, []);
+
+        $this->assertArrayHasKey('general/country/allow', $result);
+        $this->assertSame('[unserializable]', $result['general/country/allow']['new_value']);
+    }
 }

Test/Unit/Model/Activity/ThemeConfigTest.php

@@ -124,4 +124,143 @@ public function testCollectAdditionalDataIgnoresBothEmpty(): void
 
         $this->assertEmpty($result);
     }
+
+    // --- collectAdditionalData: image uploader fields posting file descriptors ---
+
+    public function testCollectAdditionalDataIgnoresUntouchedImageUploaderField(): void
+    {
+        $oldData = ['header_logo_src' => 'stores/1/logo.png'];
+        $newData = ['header_logo_src' => [
+            [
+                'url' => 'https://example.com/media/logo/stores/1/logo.png',
+                'file' => 'stores/1/logo.png',
+                'name' => 'logo.png',
+                'size' => 1234,
+            ]
+        ]];
+
+        $result = $this->themeConfig->collectAdditionalData($oldData, $newData, []);
+
+        $this->assertEmpty($result, 'Re-posting an unchanged image descriptor is not a change');
+    }
+
+    public function testCollectAdditionalDataFallsBackToDescriptorNameWhenFileMissing(): void
+    {
+        $oldData = ['head_shortcut_icon' => 'favicon.png'];
+        $newData = ['head_shortcut_icon' => [
+            ['name' => 'favicon.png', 'url' => 'https://example.com/media/favicon.png']
+        ]];
+
+        $result = $this->themeConfig->collectAdditionalData($oldData, $newData, []);
+
+        $this->assertEmpty($result);
+    }
+
+    public function testCollectAdditionalDataDetectsReplacedImage(): void
+    {
+        $oldData = ['header_logo_src' => 'old-logo.png'];
+        $newData = ['header_logo_src' => [
+            ['file' => 'new-logo.png', 'name' => 'new-logo.png', 'size' => 99]
+        ]];
+
+        $result = $this->themeConfig->collectAdditionalData($oldData, $newData, []);
+
+        $this->assertArrayHasKey('design/header/logo_src', $result);
+        $this->assertSame('old-logo.png', $result['design/header/logo_src']['old_value']);
+        $this->assertSame('new-logo.png', $result['design/header/logo_src']['new_value']);
+    }
+
+    public function testCollectAdditionalDataDetectsClearedImage(): void
+    {
+        $oldData = ['header_logo_src' => 'logo.png'];
+        $newData = ['header_logo_src' => []];
+
+        $result = $this->themeConfig->collectAdditionalData($oldData, $newData, []);
+
+        $this->assertArrayHasKey('design/header/logo_src', $result);
+        $this->assertSame('logo.png', $result['design/header/logo_src']['old_value']);
+        $this->assertSame('', $result['design/header/logo_src']['new_value']);
+    }
+
+    public function testCollectAdditionalDataJoinsListOfScalars(): void
+    {
+        $oldData = ['watermark_image_size' => 'a,b'];
+        $newData = ['watermark_image_size' => ['a', 'b']];
+
+        $result = $this->themeConfig->collectAdditionalData($oldData, $newData, []);
+
+        $this->assertEmpty($result);
+    }
+
+    public function testCollectAdditionalDataEncodesAssociativeArray(): void
+    {
+        $oldData = ['header_logo_src' => 'logo.png'];
+        $newData = ['header_logo_src' => ['delete' => '1', 'value' => 'logo.png']];
+
+        $result = $this->themeConfig->collectAdditionalData($oldData, $newData, []);
+
+        $this->assertArrayHasKey('design/header/logo_src', $result);
+        $this->assertSame(
+            '{"delete":"1","value":"logo.png"}',
+            $result['design/header/logo_src']['new_value']
+        );
+    }
+
+    /**
+     * A hand-crafted POST can nest arrays arbitrarily deep. The cast must not
+     * raise "Array to string conversion": with swissup/module-ignition
+     * installed that warning becomes an ErrorException, SaveAfter aborts and
+     * the activity is never logged.
+     */
+    public function testCollectAdditionalDataHandlesNestedArrayInDescriptor(): void
+    {
+        $oldData = ['header_logo_src' => 'logo.png'];
+        $newData = ['header_logo_src' => [['file' => ['nested'], 'name' => ['nested']]]];
+
+        $warnings = [];
+        set_error_handler(
+            static function (int $errno, string $errstr) use (&$warnings): bool {
+                $warnings[] = $errstr;
+                return true;
+            },
+            E_WARNING | E_NOTICE
+        );
+
+        try {
+            $result = $this->themeConfig->collectAdditionalData($oldData, $newData, []);
+        } finally {
+            restore_error_handler();
+        }
+
+        $this->assertSame([], $warnings, 'Flattening must not raise a PHP warning');
+        $this->assertArrayHasKey('design/header/logo_src', $result);
+        $this->assertSame(
+            '{"file":["nested"],"name":["nested"]}',
+            $result['design/header/logo_src']['new_value']
+        );
+    }
+
+    public function testCollectAdditionalDataUsesPlaceholderForUnserializableValue(): void
+    {
+        $oldData = ['header_logo_src' => 'logo.png'];
+        $newData = ['header_logo_src' => ['broken' => "\xB1\x31"]];
+
+        $result = $this->themeConfig->collectAdditionalData($oldData, $newData, []);
+
+        $this->assertArrayHasKey('design/header/logo_src', $result);
+        $this->assertSame('[unserializable]', $result['design/header/logo_src']['new_value']);
+    }
+
+    public function testCollectAdditionalDataDoesNotEscapeSlashesInEncodedValue(): void
+    {
+        $oldData = [];
+        $newData = ['header_logo_src' => ['path' => 'stores/1/logo.png']];
+
+        $result = $this->themeConfig->collectAdditionalData($oldData, $newData, []);
+
+        $this->assertSame(
+            '{"path":"stores/1/logo.png"}',
+            $result['design/header/logo_src']['new_value']
+        );
+    }
 }