diff --git a/src/Integrations/Divi/BlockTree/AttrMap.php b/src/Integrations/Divi/BlockTree/AttrMap.php index aa784f6..dd977ad 100644 --- a/src/Integrations/Divi/BlockTree/AttrMap.php +++ b/src/Integrations/Divi/BlockTree/AttrMap.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace LIT\MCP\Integrations\Divi\BlockTree; +use InvalidArgumentException; use stdClass; /** @@ -52,6 +53,9 @@ final class AttrMap { $value = $attrs; foreach (explode('.', $path) as $segment) { + if ($value instanceof stdClass) { + $value = (array) $value; + } if (! is_array($value) || ! array_key_exists($segment, $value)) { return null; } @@ -76,10 +80,15 @@ final class AttrMap /** * @param array $attrs * @return array + * @throws InvalidArgumentException When $path has an empty segment ("a..b", ".a", "a."). */ public static function set(array $attrs, string $path, mixed $value): array { $segments = explode('.', $path); + if (in_array('', $segments, true)) { + throw new InvalidArgumentException(sprintf('"%s" has an empty path segment.', $path)); + } + $key = (string) array_shift($segments); if ($segments === []) { $attrs[$key] = $value; @@ -87,9 +96,13 @@ final class AttrMap return $attrs; } - $child = $attrs[$key] ?? []; - $child = $child instanceof stdClass ? (array) $child : (is_array($child) ? $child : []); - $attrs[$key] = self::set($child, implode('.', $segments), $value); + $existing = $attrs[$key] ?? null; + $wasObject = $existing instanceof stdClass; + $child = $wasObject ? (array) $existing : (is_array($existing) ? $existing : []); + $child = self::set($child, implode('.', $segments), $value); + // Keep an object an object (spec §5.2/Json.php): a numeric-looking key like "0" would + // otherwise silently turn $child into a PHP list and re-encode as a JSON array. + $attrs[$key] = $wasObject ? (object) $child : $child; return $attrs; } diff --git a/src/Integrations/Divi/BlockTree/Block.php b/src/Integrations/Divi/BlockTree/Block.php index 532f8ae..e7212e2 100644 --- a/src/Integrations/Divi/BlockTree/Block.php +++ b/src/Integrations/Divi/BlockTree/Block.php @@ -4,6 +4,10 @@ declare(strict_types=1); namespace LIT\MCP\Integrations\Divi\BlockTree; +use InvalidArgumentException; +use LogicException; +use stdClass; + /** * One block of the tree. A parsed block keeps its original opener/closer bytes; they are * dropped only when the block itself changes (setAttrs(), a void block getting children), @@ -15,6 +19,9 @@ namespace LIT\MCP\Integrations\Divi\BlockTree; */ final class Block extends Container { + /** A valid block name: "namespace/block-name" (WordPress' own block name convention). */ + private const NAME_PATTERN = '/^[a-z][a-z0-9_-]*\/[a-z][a-z0-9_-]*$/'; + /** * @param array $attrs */ @@ -42,6 +49,10 @@ final class Block extends Container */ public static function create(string $name, array $attrs = [], array $children = []): self { + if (preg_match(self::NAME_PATTERN, $name) !== 1) { + throw new InvalidArgumentException(sprintf('"%s" is not a valid block name.', $name)); + } + $block = new self($name, $attrs, null, null, false, false); foreach ($children as $child) { $block->parts[] = $child; @@ -77,6 +88,10 @@ final class Block extends Container /** @param array $attrs */ public function setAttrs(array $attrs): void { + if ($this->opaque) { + throw new LogicException('Cannot set attributes of an opaque block.'); + } + $this->attrs = $attrs; $this->rawOpen = null; } @@ -125,6 +140,28 @@ final class Block extends Container $this->parts[$position] = clone $part; } } + $this->attrs = self::deepCloneAttrs($this->attrs); + } + + /** + * PHP's own array-copy-on-write already deep-copies nested arrays, but stdClass values + * (Json.php keeps empty/list-like JSON objects as stdClass) are objects – clone() alone + * would leave the copy's attrs() sharing them with the original. + * + * @param array $attrs + * @return array + */ + private static function deepCloneAttrs(array $attrs): array + { + foreach ($attrs as $key => $value) { + if ($value instanceof stdClass) { + $attrs[$key] = (object) self::deepCloneAttrs((array) $value); + } elseif (is_array($value)) { + $attrs[$key] = self::deepCloneAttrs($value); + } + } + + return $attrs; } protected function touched(): void diff --git a/src/Integrations/Divi/BlockTree/Container.php b/src/Integrations/Divi/BlockTree/Container.php index 08cd5f1..610e58b 100644 --- a/src/Integrations/Divi/BlockTree/Container.php +++ b/src/Integrations/Divi/BlockTree/Container.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace LIT\MCP\Integrations\Divi\BlockTree; use InvalidArgumentException; +use LogicException; /** * Ordered parts of a document or of a block's inner content: blocks and the raw text @@ -28,9 +29,21 @@ abstract class Container return array_values(array_filter($this->parts, static fn (string|Block $part): bool => $part instanceof Block)); } - /** Used by the Parser: adds a part without marking the container as changed. */ + /** + * Adds a part without marking the container as changed – the Parser's own way of + * building the tree (a block is appended to its container immediately, before parsing + * knows whether it stays void). + * + * @internal Parser only. Any other caller adding a child must use insertChild(), which + * also converts a void block so the new child is not silently dropped again + * (Serializer::block() never descends into a void block's parts()). + */ public function append(string|Block $part): void { + if ($this instanceof Block && $this->isVoid()) { + throw new LogicException('Cannot append to a void block; use insertChild() instead.'); + } + $this->parts[] = $part; } @@ -48,6 +61,7 @@ abstract class Container */ public function insertChild(int $index, Block $block): void { + $this->guardMutable(); $children = $this->children(); $index = max(0, min($index, count($children))); if ($index < count($children)) { @@ -65,6 +79,7 @@ abstract class Container /** Removes the child block; the text around it stays. Returns its former child index. */ public function removeChild(Block $block): int { + $this->guardMutable(); $index = $this->indexOf($block); if ($index === null) { throw new InvalidArgumentException('Block is not a child of this container.'); @@ -81,6 +96,14 @@ abstract class Container { } + /** insertChild()/removeChild() are the tools' structural edits – never on an opaque block. */ + private function guardMutable(): void + { + if ($this instanceof Block && $this->isOpaque()) { + throw new LogicException('Cannot modify the children of an opaque block.'); + } + } + private function partPosition(Block $block): int { $position = array_search($block, $this->parts, true); diff --git a/src/Integrations/Divi/BlockTree/Json.php b/src/Integrations/Divi/BlockTree/Json.php index f865d34..e479df9 100644 --- a/src/Integrations/Divi/BlockTree/Json.php +++ b/src/Integrations/Divi/BlockTree/Json.php @@ -15,7 +15,11 @@ use stdClass; */ final class Json { - /** @return array|null Null when $json is not a JSON object. */ + /** + * @return array|null Null when $json is not a JSON object, or its top level + * is one only nominally (keys "0", "1", … – see the class docblock): re-encoding those + * keys as a PHP array would silently turn the object back into a JSON list. + */ public static function decodeObject(string $json): ?array { try { @@ -28,6 +32,11 @@ final class Json return null; } + $keys = get_object_vars($value); + if ($keys !== [] && array_is_list($keys)) { + return null; + } + $converted = self::fromJson($value); return is_array($converted) ? $converted : []; @@ -52,7 +61,13 @@ final class Json return $value; } - /** @throws JsonException */ + /** + * @throws JsonException When $value cannot be represented as JSON – including a string + * with an invalid UTF-8 byte sequence (JSON_ERROR_UTF8). 2a only ever encodes + * attributes this plugin already decoded from valid block markup, so that never + * happens here; a caller that later encodes user-supplied strings (2b's writers) + * must catch this and map it to a validation error instead of a fatal. + */ public static function encode(mixed $value): string { // phpcs:ignore WordPress.WP.AlternativeFunctions.json_encode_json_encode -- pure PHP (spec §4.4). diff --git a/src/Integrations/Divi/BlockTree/Serializer.php b/src/Integrations/Divi/BlockTree/Serializer.php index b298530..76d55ee 100644 --- a/src/Integrations/Divi/BlockTree/Serializer.php +++ b/src/Integrations/Divi/BlockTree/Serializer.php @@ -8,7 +8,8 @@ namespace LIT\MCP\Integrations\Divi\BlockTree; * Turns a Document back into block markup. Unchanged blocks reuse their original * delimiters; changed or new blocks get delimiters generated exactly like WordPress' * get_comment_delimited_block_content() / serialize_block_attributes() do, which also - * yields the escaping Divi expects inside $variable(…)$ payloads (" → "). + * yields the escaping Divi expects inside $variable(…)$ payloads: json_encode()'s own + * \" (an escaped quote inside a JSON string) is replaced by ", see ESCAPES below. */ final class Serializer { diff --git a/tests/Unit/Integrations/Divi/BlockTree/AttrMapTest.php b/tests/Unit/Integrations/Divi/BlockTree/AttrMapTest.php new file mode 100644 index 0000000..1ae665a --- /dev/null +++ b/tests/Unit/Integrations/Divi/BlockTree/AttrMapTest.php @@ -0,0 +1,59 @@ + */ + public static function emptySegmentPaths(): array + { + return [ + 'double dot' => ['a..b'], + 'leading dot' => ['.a'], + 'trailing dot' => ['a.'], + 'empty' => [''], + ]; + } + + /** @dataProvider emptySegmentPaths */ + public function testSetRejectsEmptyPathSegments(string $path): void + { + $this->expectException(InvalidArgumentException::class); + + AttrMap::set([], $path, 'z'); + } + + public function testSetKeepsAnObjectAnObjectWhenRewrapping(): void + { + $attrs = ['a' => new stdClass()]; + + $changed = AttrMap::set($attrs, 'a.0', 'z'); + + self::assertInstanceOf(stdClass::class, $changed['a']); + self::assertSame('z', $changed['a']->{'0'}); + self::assertSame('{"a":{"0":"z"}}', json_encode($changed)); + } + + public function testSetStillBuildsPlainArraysWhenNoObjectExisted(): void + { + $changed = AttrMap::set([], 'a.b', 'z'); + + self::assertSame(['a' => ['b' => 'z']], $changed); + } + + /** get() must still read back through a value set() kept as an object (regression guard). */ + public function testGetReadsThroughAnObjectRewrappedBySet(): void + { + $attrs = ['a' => new stdClass()]; + $changed = AttrMap::set($attrs, 'a.b', 'z'); + + self::assertSame('z', AttrMap::get($changed, 'a.b')); + } +} diff --git a/tests/Unit/Integrations/Divi/BlockTree/BlockTest.php b/tests/Unit/Integrations/Divi/BlockTree/BlockTest.php new file mode 100644 index 0000000..ba1b31a --- /dev/null +++ b/tests/Unit/Integrations/Divi/BlockTree/BlockTest.php @@ -0,0 +1,69 @@ + */ + public static function invalidNames(): array + { + return [ + 'no namespace' => ['button'], + 'trailing markup injection' => ['divi/x -->