diff --git a/Model/Activity/SystemConfig.php b/Model/Activity/SystemConfig.php index f5b81f1..3e33c92 100644 --- a/Model/Activity/SystemConfig.php +++ b/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; } } diff --git a/Model/Activity/ThemeConfig.php b/Model/Activity/ThemeConfig.php index 2536987..d3cb7d0 100644 --- a/Model/Activity/ThemeConfig.php +++ b/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 $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; + } } diff --git a/Test/Unit/Model/Activity/SystemConfigTest.php b/Test/Unit/Model/Activity/SystemConfigTest.php index 5a000bf..04e9b7e 100644 --- a/Test/Unit/Model/Activity/SystemConfigTest.php +++ b/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']); + } } diff --git a/Test/Unit/Model/Activity/ThemeConfigTest.php b/Test/Unit/Model/Activity/ThemeConfigTest.php index 684c694..12ffc8c 100644 --- a/Test/Unit/Model/Activity/ThemeConfigTest.php +++ b/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'] + ); + } }