fix: harden the Divi BlockTree against opaque/invalid writes

Final fix wave for stage 2a, load-bearing for 2b's writers:
- Block::setAttrs()/Container::insertChild()/removeChild() refuse on an opaque
  block (undecodable attrs or a global module) instead of silently corrupting it.
- Container::append() is now @internal (Parser only) and refuses on a void
  block, so a child can no longer be silently dropped from the serialized output.
- Block::create() validates the block name against namespace/block-name.
- Json::decodeObject() returns null (-> opaque) for a top-level object whose
  keys are a plain list ("0","1",...) rather than returning [] and letting the
  Serializer re-encode it as a corrupting JSON array.
- AttrMap::set() rejects empty path segments and keeps an existing stdClass
  value an object when rewrapping it (no silent object->list flip); AttrMap::get()
  is updated in step so it can still read back through such an object.
- Block::copy() deep-clones stdClass attribute values instead of sharing them.
- Json::encode()'s existing JsonException-on-invalid-UTF-8 behaviour is documented.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-09-26 13:12:49 +02:00
co-authored by Claude Opus 5.5
parent f433fef8f7
commit 4137882008
10 changed files with 339 additions and 7 deletions
+16 -3
View File
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace LIT\MCP\Integrations\Divi\BlockTree; namespace LIT\MCP\Integrations\Divi\BlockTree;
use InvalidArgumentException;
use stdClass; use stdClass;
/** /**
@@ -52,6 +53,9 @@ final class AttrMap
{ {
$value = $attrs; $value = $attrs;
foreach (explode('.', $path) as $segment) { foreach (explode('.', $path) as $segment) {
if ($value instanceof stdClass) {
$value = (array) $value;
}
if (! is_array($value) || ! array_key_exists($segment, $value)) { if (! is_array($value) || ! array_key_exists($segment, $value)) {
return null; return null;
} }
@@ -76,10 +80,15 @@ final class AttrMap
/** /**
* @param array<string, mixed> $attrs * @param array<string, mixed> $attrs
* @return array<string, mixed> * @return array<string, mixed>
* @throws InvalidArgumentException When $path has an empty segment ("a..b", ".a", "a.").
*/ */
public static function set(array $attrs, string $path, mixed $value): array public static function set(array $attrs, string $path, mixed $value): array
{ {
$segments = explode('.', $path); $segments = explode('.', $path);
if (in_array('', $segments, true)) {
throw new InvalidArgumentException(sprintf('"%s" has an empty path segment.', $path));
}
$key = (string) array_shift($segments); $key = (string) array_shift($segments);
if ($segments === []) { if ($segments === []) {
$attrs[$key] = $value; $attrs[$key] = $value;
@@ -87,9 +96,13 @@ final class AttrMap
return $attrs; return $attrs;
} }
$child = $attrs[$key] ?? []; $existing = $attrs[$key] ?? null;
$child = $child instanceof stdClass ? (array) $child : (is_array($child) ? $child : []); $wasObject = $existing instanceof stdClass;
$attrs[$key] = self::set($child, implode('.', $segments), $value); $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; return $attrs;
} }
+37
View File
@@ -4,6 +4,10 @@ declare(strict_types=1);
namespace LIT\MCP\Integrations\Divi\BlockTree; 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 * 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), * 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 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<string, mixed> $attrs * @param array<string, mixed> $attrs
*/ */
@@ -42,6 +49,10 @@ final class Block extends Container
*/ */
public static function create(string $name, array $attrs = [], array $children = []): self 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); $block = new self($name, $attrs, null, null, false, false);
foreach ($children as $child) { foreach ($children as $child) {
$block->parts[] = $child; $block->parts[] = $child;
@@ -77,6 +88,10 @@ final class Block extends Container
/** @param array<string, mixed> $attrs */ /** @param array<string, mixed> $attrs */
public function setAttrs(array $attrs): void public function setAttrs(array $attrs): void
{ {
if ($this->opaque) {
throw new LogicException('Cannot set attributes of an opaque block.');
}
$this->attrs = $attrs; $this->attrs = $attrs;
$this->rawOpen = null; $this->rawOpen = null;
} }
@@ -125,6 +140,28 @@ final class Block extends Container
$this->parts[$position] = clone $part; $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<string, mixed> $attrs
* @return array<string, mixed>
*/
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 protected function touched(): void
+24 -1
View File
@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace LIT\MCP\Integrations\Divi\BlockTree; namespace LIT\MCP\Integrations\Divi\BlockTree;
use InvalidArgumentException; use InvalidArgumentException;
use LogicException;
/** /**
* Ordered parts of a document or of a block's inner content: blocks and the raw text * 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)); 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 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; $this->parts[] = $part;
} }
@@ -48,6 +61,7 @@ abstract class Container
*/ */
public function insertChild(int $index, Block $block): void public function insertChild(int $index, Block $block): void
{ {
$this->guardMutable();
$children = $this->children(); $children = $this->children();
$index = max(0, min($index, count($children))); $index = max(0, min($index, count($children)));
if ($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. */ /** Removes the child block; the text around it stays. Returns its former child index. */
public function removeChild(Block $block): int public function removeChild(Block $block): int
{ {
$this->guardMutable();
$index = $this->indexOf($block); $index = $this->indexOf($block);
if ($index === null) { if ($index === null) {
throw new InvalidArgumentException('Block is not a child of this container.'); 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 private function partPosition(Block $block): int
{ {
$position = array_search($block, $this->parts, true); $position = array_search($block, $this->parts, true);
+17 -2
View File
@@ -15,7 +15,11 @@ use stdClass;
*/ */
final class Json final class Json
{ {
/** @return array<string, mixed>|null Null when $json is not a JSON object. */ /**
* @return array<string, mixed>|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 public static function decodeObject(string $json): ?array
{ {
try { try {
@@ -28,6 +32,11 @@ final class Json
return null; return null;
} }
$keys = get_object_vars($value);
if ($keys !== [] && array_is_list($keys)) {
return null;
}
$converted = self::fromJson($value); $converted = self::fromJson($value);
return is_array($converted) ? $converted : []; return is_array($converted) ? $converted : [];
@@ -52,7 +61,13 @@ final class Json
return $value; 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 public static function encode(mixed $value): string
{ {
// phpcs:ignore WordPress.WP.AlternativeFunctions.json_encode_json_encode -- pure PHP (spec §4.4). // phpcs:ignore WordPress.WP.AlternativeFunctions.json_encode_json_encode -- pure PHP (spec §4.4).
@@ -8,7 +8,8 @@ namespace LIT\MCP\Integrations\Divi\BlockTree;
* Turns a Document back into block markup. Unchanged blocks reuse their original * Turns a Document back into block markup. Unchanged blocks reuse their original
* delimiters; changed or new blocks get delimiters generated exactly like WordPress' * delimiters; changed or new blocks get delimiters generated exactly like WordPress'
* get_comment_delimited_block_content() / serialize_block_attributes() do, which also * 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 final class Serializer
{ {
@@ -0,0 +1,59 @@
<?php
declare(strict_types=1);
namespace LIT\MCP\Tests\Unit\Integrations\Divi\BlockTree;
use InvalidArgumentException;
use LIT\MCP\Integrations\Divi\BlockTree\AttrMap;
use PHPUnit\Framework\TestCase;
use stdClass;
final class AttrMapTest extends TestCase
{
/** @return array<string, array{string}> */
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'));
}
}
@@ -0,0 +1,69 @@
<?php
declare(strict_types=1);
namespace LIT\MCP\Tests\Unit\Integrations\Divi\BlockTree;
use InvalidArgumentException;
use LIT\MCP\Integrations\Divi\BlockTree\Block;
use LIT\MCP\Integrations\Divi\BlockTree\Parser;
use LogicException;
use PHPUnit\Framework\TestCase;
use stdClass;
final class BlockTest extends TestCase
{
/** @return array<string, array{string}> */
public static function invalidNames(): array
{
return [
'no namespace' => ['button'],
'trailing markup injection' => ['divi/x --><script>'],
'uppercase' => ['Divi/Button'],
'empty' => [''],
];
}
/** @dataProvider invalidNames */
public function testCreateRejectsInvalidBlockNames(string $name): void
{
$this->expectException(InvalidArgumentException::class);
Block::create($name);
}
public function testCreateAcceptsAWellFormedName(): void
{
self::assertSame('divi/button', Block::create('divi/button')->name());
}
public function testSetAttrsRefusesOnAnOpaqueGlobalModule(): void
{
$document = (new Parser())->parse('<!-- wp:divi/section {"globalModule":"x"} --><!-- /wp:divi/section -->');
$section = $document->children()[0];
self::assertTrue($section->isOpaque());
$this->expectException(LogicException::class);
$section->setAttrs(['a' => 1]);
}
public function testSetAttrsRefusesOnABlockWithUndecodableAttrs(): void
{
$document = (new Parser())->parse('<!-- wp:divi/text {"a":} /-->');
$block = $document->children()[0];
self::assertTrue($block->isOpaque());
$this->expectException(LogicException::class);
$block->setAttrs(['a' => 1]);
}
public function testCopyDeepClonesStdClassAttributeValues(): void
{
$block = Block::create('divi/text', ['localAttrs' => new stdClass(), 'nested' => ['obj' => new stdClass()]]);
$copy = $block->copy();
self::assertNotSame($block->attrs()['localAttrs'], $copy->attrs()['localAttrs']);
self::assertEquals($block->attrs()['localAttrs'], $copy->attrs()['localAttrs']);
self::assertNotSame($block->attrs()['nested']['obj'], $copy->attrs()['nested']['obj']);
}
}
@@ -0,0 +1,54 @@
<?php
declare(strict_types=1);
namespace LIT\MCP\Tests\Unit\Integrations\Divi\BlockTree;
use LIT\MCP\Integrations\Divi\BlockTree\Block;
use LIT\MCP\Integrations\Divi\BlockTree\Parser;
use LogicException;
use PHPUnit\Framework\TestCase;
final class ContainerTest extends TestCase
{
public function testInsertChildRefusesOnAnOpaqueBlock(): void
{
$document = (new Parser())->parse('<!-- wp:divi/section {"globalModule":"x"} --><!-- /wp:divi/section -->');
$section = $document->children()[0];
self::assertTrue($section->isOpaque());
$this->expectException(LogicException::class);
$section->insertChild(0, Block::create('divi/text'));
}
public function testInsertChildRefusesOnABlockWithUndecodableAttrs(): void
{
$document = (new Parser())->parse('<!-- wp:divi/section {"a":} --><!-- /wp:divi/section -->');
$section = $document->children()[0];
self::assertTrue($section->isOpaque());
$this->expectException(LogicException::class);
$section->insertChild(0, Block::create('divi/text'));
}
public function testRemoveChildRefusesOnAnOpaqueBlock(): void
{
$document = (new Parser())->parse(
'<!-- wp:divi/section {"globalModule":"x"} --><!-- wp:divi/row --><!-- /wp:divi/row --><!-- /wp:divi/section -->'
);
$section = $document->children()[0];
$row = $section->children()[0];
self::assertTrue($section->isOpaque());
$this->expectException(LogicException::class);
$section->removeChild($row);
}
public function testAppendRefusesOnAVoidBlock(): void
{
$void = Block::parsed('divi/image', [], '<!-- wp:divi/image /-->', true, false);
$this->expectException(LogicException::class);
$void->append('text');
}
}
@@ -0,0 +1,51 @@
<?php
declare(strict_types=1);
namespace LIT\MCP\Tests\Unit\Integrations\Divi\BlockTree;
use JsonException;
use LIT\MCP\Integrations\Divi\BlockTree\Json;
use PHPUnit\Framework\TestCase;
use stdClass;
final class JsonTest extends TestCase
{
public function testDecodesAPlainObject(): void
{
self::assertSame(['a' => 1], Json::decodeObject('{"a":1}'));
self::assertSame([], Json::decodeObject('{}'));
}
public function testRejectsNonObjectTopLevelJson(): void
{
self::assertNull(Json::decodeObject('[1,2,3]'));
self::assertNull(Json::decodeObject('"a string"'));
self::assertNull(Json::decodeObject('not json'));
}
/** A JSON object whose keys are "0", "1", … decodes to a PHP list – re-encoding it would
* silently turn it back into a JSON array (Json.php docblock, EmptyArrayCorruptionMigration). */
public function testTreatsAListLikeTopLevelObjectAsOpaqueRatherThanAnArray(): void
{
self::assertNull(Json::decodeObject('{"0":"a","1":"b"}'));
self::assertNull(Json::decodeObject('{"0":"a"}'));
}
public function testDoesNotFlagAGenuineObjectWithNonSequentialNumericKeys(): void
{
self::assertSame(['0' => 'a', '2' => 'b'], Json::decodeObject('{"0":"a","2":"b"}'));
}
public function testEncodeThrowsOnInvalidUtf8(): void
{
$this->expectException(JsonException::class);
Json::encode("\xB1\x31");
}
public function testEncodeRoundtripsAnEmptyObject(): void
{
self::assertSame('{}', Json::encode(new stdClass()));
}
}
@@ -94,6 +94,16 @@ final class ParserTest extends TestCase
self::assertSame([], $broken->children()[0]->attrs()); self::assertSame([], $broken->children()[0]->attrs());
} }
public function testMarksListLikeTopLevelAttributesOpaque(): void
{
$content = '<!-- wp:divi/text {"0":"a","1":"b"} /-->';
$document = (new Parser())->parse($content);
self::assertTrue($document->children()[0]->isOpaque());
self::assertSame([], $document->children()[0]->attrs());
self::assertSame($content, Serializer::document($document));
}
public function testKeepsEmptyAndListLikeObjectsAsObjects(): void public function testKeepsEmptyAndListLikeObjectsAsObjects(): void
{ {
$document = (new Parser())->parse($this->fixture('synthetic-mixed.html')); $document = (new Parser())->parse($this->fixture('synthetic-mixed.html'));