Loosen-IT/WP-LIT-MCP
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 7 Wiki Activity

refactor: ValueCleaner is the named choke point (H1)

Browse Source 
The choke point moves out of AttrWriter into ValueCleaner (catalog, the
composition's VariableRef, sanitizer): clean() (was AttrWriter::cleanValue),
kindOf(), guardPath() and the private cleanNode/kind/unknownKind/tightens with
VALUE_KEY, URL_KEY, URL_SEGMENT, STRICTNESS, BUILDER_ONLY (its docblock now sits
on it; STRUCTURE stays in AttrWriter). Code of the pipeline moved unchanged.
- ValueCleaner's docblock is the one canonical description: steps 0-8 with their
  rulings in code order, invariants I1-I3; the step comments in cleanNode carry
  the same numbers. AttrWriter/ShortformWriter/Composer/TextReplacer docblocks
  point there and keep only their own rules.
- ValueCleaner::textReplacementFor($module) holds the D10 rule that was a
  closure in DiviReplaceTextAbility::replace (text/html only, clean() by kind).
- AttrWriter, ShortformWriter and Composer take the ValueCleaner; DiviComposition
  builds one per composition (cleaner()).
Tests: the eight AttrWriterTest methods that only exercised cleanValue/kindOf
moved verbatim to ValueCleanerTest (clean/kindOf, ComposeFixture::cleaner());
mixed tests call the cleaner in place; +1 test for textReplacementFor. Every
provider row and per-test assertion count unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7
This commit is contained in:
janniclas.loosen
2026-09-26 18:11:28 +02:00
co-authored by Claude Opus 5.5
parent 203d14a2f9
commit 62ce99e558
13 changed files with 552 additions and 447 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Unit/Integrations/Divi/Compose/ValueCleanerTest.php
+200
View File
@@ -0,0 +1,200 @@
<?php
declare(strict_types=1);
namespace LIT\MCP\Tests\Unit\Integrations\Divi\Compose;
use InvalidArgumentException;
use LIT\MCP\Tests\Support\ComposeFixture;
use LIT\MCP\Tests\Support\DiviFixtures;
use PHPUnit\Framework\TestCase;
/** The choke point (ValueCleaner): tests moved from AttrWriterTest with clean()/kindOf() (H1). */
final class ValueCleanerTest extends TestCase
{
/** Ruling S3/S3a: kind from the catalog, per sub-field for composite paths, else key name, else text. */
public function testRoutesByCatalogKindAndKeyNameAtEveryDepth(): void
{
$cleaner = ComposeFixture::cleaner();
// Unknown kind, URL keys deep inside the value.
self::assertSame(
['color' => '#fff', 'image' => ['url' => '', 'parallax' => ['enabled' => 'on']]],
$cleaner->clean('divi/text', 'module.decoration.background', [
'color' => '<i>#fff</i>',
'image' => ['url' => 'javascript:alert(1)', 'parallax' => ['enabled' => 'on']],
])
);
self::assertSame(
['url' => '', 'target' => 'on'],
$cleaner->clean('divi/text', 'module.advanced.link', ['url' => ' JavaScript:alert(1)', 'target' => '<b>on</b>'])
);
self::assertSame(['href' => '', 'src' => '', 'image' => ''], $cleaner->clean('divi/text', 'module.advanced.link', [
'href' => 'javascript:a()', 'src' => 'javascript:b()', 'image' => 'javascript:c()',
]));
// Rich text keeps markup, text kind strips it, unknown innerContent is text too.
self::assertSame('<p>Hallo <b>Welt</b></p>', $cleaner->clean('divi/text', 'content.innerContent', '<p>Hallo <b>Welt</b></p>'));
self::assertSame('Titel', $cleaner->clean('divi/heading', 'title.innerContent', '<h1>Titel</h1>'));
self::assertSame('Los', $cleaner->clean('divi/button', 'button.innerContent', '<b>Los</b>', 'text'));
// Composite path with per-subName kinds (buttonOne.innerContent: text → text, linkUrl → url).
self::assertSame(
['text' => 'Mehr', 'linkUrl' => ''],
$cleaner->clean('divi/fullwidth-header', 'buttonOne.innerContent', [
'text' => '<b>Mehr</b>',
'linkUrl' => 'javascript:alert(1)',
])
);
self::assertSame('', $cleaner->clean('divi/fullwidth-header', 'buttonOne.innerContent', 'javascript:x', 'linkUrl'));
// An explicit sub-kind wins over the key name; the image module's alt is text, src a URL.
self::assertSame(
['useIcon' => 'on', 'src' => ''],
$cleaner->clean('divi/blurb', 'imageIcon.innerContent', ['useIcon' => 'on', 'src' => 'javascript:x'])
);
self::assertSame(
['src' => 'https://kunde.test/a.jpg', 'alt' => 'Team'],
$cleaner->clean('divi/image', 'image.innerContent', ['src' => 'https://kunde.test/a.jpg', 'alt' => '<b>Team</b>'])
);
// Numbers and booleans are no strings: unchanged.
self::assertSame(['size' => 12, 'on' => true], $cleaner->clean('divi/text', 'module.decoration.spacing', ['size' => 12, 'on' => true]));
}
public function testEmbeddedReferenceNamesTheRule(): void
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('Verweise nur allein im Wert');
ComposeFixture::cleaner()->clean('divi/text', 'content.innerContent', 'a $variable({}) b');
}
/** Ruling S26: every free-form custom CSS path (css.*) is builder-only for editors, by path shape. */
public function testCustomCssPathsNeedUnfilteredHtml(): void
{
$css = ['mainElement' => 'color:red;} body{display:none'];
foreach (['css', 'css.desktop', 'css.advanced.customCss', 'css.advanced.mainElement', 'title.css'] as $path) {
try {
ComposeFixture::cleaner()->clean('divi/text', $path, $css);
self::fail('Accepted for an editor: ' . $path);
} catch (InvalidArgumentException $e) {
self::assertStringContainsString('nur im Divi-Builder', $e->getMessage());
}
self::assertSame($css, ComposeFixture::cleaner(true)->clean('divi/text', $path, $css));
}
// Only a whole "css" segment counts.
self::assertSame('x', ComposeFixture::cleaner()->clean('divi/text', 'module.advanced.cssClass', 'x'));
self::assertSame('x', ComposeFixture::cleaner()->clean('divi/text', 'module.cssish', 'x'));
}
/** Ruling S23: content.innerContent of unknown kind (defined only in Divi's JS) is rich text. */
public function testUnknownContentInnerContentIsRichText(): void
{
self::assertSame('unknown', DiviFixtures::catalog()->pathKind('divi/blurb', 'content.innerContent'));
self::assertSame('<p>Hallo</p>', ComposeFixture::cleaner()->clean('divi/blurb', 'content.innerContent', '<p>Hallo</p><script>x()</script>'));
// Other unknown innerContent stays text.
self::assertSame('Los', ComposeFixture::cleaner()->clean('divi/button', 'button.innerContent', '<b>Los</b>'));
}
/** kindOf() names the routing clean() applies (lit/divi-replace-text skips url/code). */
public function testKindOfNamesTheRouting(): void
{
$cleaner = ComposeFixture::cleaner();
self::assertSame('html', $cleaner->kindOf('divi/text', 'content.innerContent'));
self::assertSame('text', $cleaner->kindOf('divi/heading', 'title.innerContent'));
self::assertSame('code', $cleaner->kindOf('divi/code', 'content.innerContent'));
self::assertSame('text', $cleaner->kindOf('divi/fullwidth-header', 'buttonOne.innerContent', 'text'));
self::assertSame('url', $cleaner->kindOf('divi/fullwidth-header', 'buttonOne.innerContent', 'linkUrl'));
self::assertSame('text', $cleaner->kindOf('divi/image', 'image.innerContent', 'alt'));
self::assertSame('url', $cleaner->kindOf('divi/image', 'image.innerContent', 'src'));
}
/** Ruling S24: keys below a scalar-kind path are unknown; a …Url path segment means URL. */
public function testKeysBelowScalarKindsAndUrlPathSegments(): void
{
$cleaner = ComposeFixture::cleaner();
self::assertSame(
['url' => '', 'text' => 'a'],
$cleaner->clean('divi/heading', 'title.innerContent', ['url' => 'javascript:x', 'text' => '<b>a</b>'])
);
self::assertSame('', $cleaner->clean('divi/contact-form', 'success.advanced.redirectUrl', 'javascript:alert(1)'));
self::assertSame('', $cleaner->clean('divi/text', 'module.advanced.imageUrl', 'javascript:alert(1)'));
self::assertSame('Titel', $cleaner->clean('divi/heading', 'title.innerContent', '<b>Titel</b>'));
}
/** Ruling S32: C0 controls (except tab, LF, CR) and DEL go at the choke point, before the marker check. */
public function testControlCharactersAreStrippedBeforeTheMarkerCheck(): void
{
$cleaner = ComposeFixture::cleaner();
self::assertSame("a\tb\n", $cleaner->clean('divi/code', 'content.innerContent', "a\x00\t\x01b\x1f\n\x7f"));
self::assertSame("<p>a\nb\r</p>", $cleaner->clean('divi/text', 'content.innerContent', "<p>a\x0b\nb\r\x08</p>"));
self::assertSame('Titel', $cleaner->clean('divi/heading', 'title.innerContent', "Ti\x00tel\x7f"));
// S32a: controls go on the RAW value, so a control-split marker IS a marker: the value goes
// through canonical() like any reference – validated, cleaned (S21) and re-serialized.
$tail = '({"type":"content","value":{"name":"post_title","settings":{"before":"<script>x()</script>","after":""}}})$';
$canonical = '$variable({"type":"content","value":{"name":"post_title","settings":{"before":"","after":""}}})$';
$targets = [
'text' => ['divi/heading', 'title.innerContent'],
'html' => ['divi/text', 'content.innerContent'],
'code' => ['divi/code', 'content.innerContent'],
];
foreach ([chr(0), chr(1), chr(8), chr(27), chr(127)] as $control) {
$value = '$variable' . $control . $tail;
// The fake keeps them like sanitize_text_field does: the production code handles it.
self::assertStringNotContainsString('$variable(', ComposeFixture::sanitizer()->text('$variable' . $control . '(x'));
foreach ($targets as $kind => [$module, $path]) {
self::assertSame($canonical, $cleaner->clean($module, $path, $value), $kind . ' ' . ord($control));
}
// Embedded in text it is an embedded reference: refused.
try {
$cleaner->clean('divi/heading', 'title.innerContent', 'Hallo $variable' . $control . $tail);
self::fail('embedded control-split reference accepted');
} catch (InvalidArgumentException $e) {
self::assertStringContainsString('allein im Wert', $e->getMessage());
}
}
}
/** Ruling S33a: a kind override may only tighten the routed kind (code < html < text; url is its own family). */
public function testKindOverrideMayOnlyTighten(): void
{
$cleaner = ComposeFixture::cleaner();
self::assertSame('x', $cleaner->clean('divi/text', 'content.innerContent', '<p>x</p>', null, 'text'));
self::assertSame('x', $cleaner->clean('divi/code', 'content.innerContent', '<b>x</b>', null, 'text'));
self::assertSame('<p>x</p>', $cleaner->clean('divi/code', 'content.innerContent', '<p>x</p><script>y</script>', null, 'html'));
self::assertSame('Los', $cleaner->clean('divi/heading', 'title.innerContent', '<b>Los</b>', null, 'text'));
self::assertSame('/a', $cleaner->clean('divi/button', 'button.innerContent', '/a', 'linkUrl', 'url'));
$weaker = [
'html over text' => ['divi/heading', 'title.innerContent', null, 'html'],
'code over text' => ['divi/heading', 'title.innerContent', null, 'code'],
'code over html' => ['divi/text', 'content.innerContent', null, 'code'],
'url over text' => ['divi/heading', 'title.innerContent', null, 'url'],
'text over url' => ['divi/button', 'button.innerContent', 'linkUrl', 'text'],
'html over url' => ['divi/button', 'button.innerContent', 'linkUrl', 'html'],
];
foreach ($weaker as $name => [$module, $path, $sub, $kind]) {
try {
$cleaner->clean($module, $path, 'x', $sub, $kind);
self::fail($name . ' was accepted');
} catch (InvalidArgumentException $e) {
self::assertStringContainsString('Feldart', $e->getMessage(), $name);
}
}
}
/** D10: replace-text inserts only into text/html places, cleaned by the routed kind; URL and code stay. */
public function testTextReplacementTouchesOnlyTextAndHtml(): void
{
$cleaner = ComposeFixture::cleaner();
self::assertSame('<p>x</p>', ($cleaner->textReplacementFor('divi/text'))('content.innerContent', null, '<p>x</p><script>y</script>'));
self::assertSame('Los', ($cleaner->textReplacementFor('divi/button'))('button.innerContent', 'text', '<b>Los</b>'));
self::assertNull(($cleaner->textReplacementFor('divi/button'))('button.innerContent', 'linkUrl', 'https://x.test'));
self::assertNull(($cleaner->textReplacementFor('divi/code'))('content.innerContent', null, 'x'));
}
}