From 8688a024022072c27e76210b65209962960904ae Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Sat, 26 Sep 2026 14:26:18 +0200 Subject: [PATCH] fix: Task 3 review round 1 - strip control chars before the sanitizer (S32a), S34 minors Control characters now go on the raw value in AttrWriter::cleanNode and VariableRef::setting; after the sanitizer only rejection remains ('<' + VT/FF no longer turns into a tag). VALUE_KEY anchored with D, exact column-width fractions, nested rows in columns, specialty columns refused, fullwidth by catalog category, preset ambiguity in DesignSystem::modulePresetId(). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7 --- src/Integrations/Divi/Compose/AttrWriter.php | 16 +-- src/Integrations/Divi/Compose/Composer.php | 97 +++++++++---------- .../Divi/Compose/ShortformWriter.php | 6 +- .../Divi/Compose/StructureRules.php | 44 +++++++-- src/Integrations/Divi/Compose/VariableRef.php | 25 +++-- src/Integrations/Divi/Design/DesignSystem.php | 37 +++++-- .../Divi/Compose/AttrWriterTest.php | 63 +++++++++--- .../Divi/Compose/ComposeFixture.php | 3 + .../Divi/Compose/ComposerTest.php | 29 +++++- .../Divi/Compose/ShortformWriterTest.php | 27 ++++-- .../Divi/Compose/StructureRulesTest.php | 12 +++ .../Divi/Compose/VariableRefTest.php | 9 ++ .../Divi/Design/DesignSystemTest.php | 14 +++ 13 files changed, 278 insertions(+), 104 deletions(-) diff --git a/src/Integrations/Divi/Compose/AttrWriter.php b/src/Integrations/Divi/Compose/AttrWriter.php index 615ef83..bdd59c5 100644 --- a/src/Integrations/Divi/Compose/AttrWriter.php +++ b/src/Integrations/Divi/Compose/AttrWriter.php @@ -29,7 +29,7 @@ final class AttrWriter private const URL_KEY = '/(url|src|link|href|image)$/i'; /** Keys inside values: plain identifiers (ruling S13); list indexes are ints. */ - private const VALUE_KEY = '/^[A-Za-z0-9_-]+$/'; + private const VALUE_KEY = '/^[A-Za-z0-9_-]+$/D'; /** Last path segment naming a URL when neither kind nor key does (ruling S24): redirectUrl. */ private const URL_SEGMENT = '/url$/i'; @@ -202,6 +202,11 @@ final class AttrWriter throw new InvalidArgumentException('Wert ist kein gültiges UTF-8.'); } + // Ruling S32/S32a: control characters go on the RAW value, before the marker check and + // before the sanitizer – the WordPress sanitizers keep them ("<" + VT is a literal "<" + // for strip_tags), so any later strip could build a tag or a marker nobody checked. + $value = VariableRef::withoutControls($value); + $routed = $this->kind($module, $path, $subName, $key); if ($kind !== null && ! self::tightens($kind, $routed)) { throw new InvalidArgumentException(sprintf( @@ -225,11 +230,10 @@ final class AttrWriter default => $this->sanitizer->text($value), }; - // Ruling S32: control characters (the WordPress sanitizers keep NUL & co.) go before the - // check. Ruling S28: sanitizers strip characters ("", NUL, "%41") and can thereby build - // a marker the raw check above never saw – so the final string is checked again. - $clean = VariableRef::withoutControls($clean); - if (VariableRef::contains($clean)) { + // Ruling S28: sanitizers strip characters ("", "%41") and can thereby build a marker the + // raw check above never saw – so the final string is checked again. After the sanitizer + // the only action is rejection (S32a): nothing transforms the cleaned string any more. + if (VariableRef::contains($clean) || VariableRef::withoutControls($clean) !== $clean) { throw VariableRef::notAlone(); } diff --git a/src/Integrations/Divi/Compose/Composer.php b/src/Integrations/Divi/Compose/Composer.php index 6c62b9f..81e9c93 100644 --- a/src/Integrations/Divi/Compose/Composer.php +++ b/src/Integrations/Divi/Compose/Composer.php @@ -25,7 +25,7 @@ use LIT\MCP\Integrations\Divi\Schema\ShortformMap; * writes itself is not client text: the builder version (checked in the constructor), block * names (catalog + StructureRules + Block::create), "fullwidth" and column flexTypes built * from digits. Design-system names that match several entries are errors listing the ids - * (ruling S13a: colors/variables in Placeholders, presets here). + * (ruling S13a: colors/variables in Placeholders, presets in DesignSystem::modulePresetId()). */ final class Composer { @@ -390,16 +390,22 @@ final class Composer return $attrs; } - $id = is_string($preset) ? $this->presetId($name, $preset, $path) : null; + try { + $id = is_string($preset) ? $this->design->modulePresetId($name, $preset) : null; + } catch (InvalidArgumentException $e) { + // Ruling S13a: a name several presets share (DesignSystem lists their ids). + $this->errors[$path] = $e->getMessage(); + + return $attrs; + } + if ($id === null) { - if (! isset($this->errors[$path])) { - $names = $this->design->modulePresetNames($name); - $this->errors[$path] = sprintf( - 'unbekanntes Preset; vorhanden für %s: %s', - $name, - $names === [] ? '(keine)' : implode(', ', $names) - ); - } + $names = $this->design->modulePresetNames($name); + $this->errors[$path] = sprintf( + 'unbekanntes Preset; vorhanden für %s: %s', + $name, + $names === [] ? '(keine)' : implode(', ', $names) + ); return $attrs; } @@ -422,42 +428,6 @@ final class Composer return $attrs; } - /** - * Module preset id by id, else by name (case-insensitive); a name several presets of the - * module share is an error listing their ids (ruling S13a). Null when unknown or ambiguous. - */ - private function presetId(string $name, string $preset, string $path): ?string - { - $wanted = trim($preset); - $key = mb_strtolower($wanted); - $ids = []; - foreach ($this->design->presets() as $entry) { - if ($entry['kind'] !== 'module' || $entry['module'] !== $name) { - continue; - } - - if ($entry['id'] === $wanted) { - return $entry['id']; - } - - if (mb_strtolower($entry['name']) === $key) { - $ids[] = $entry['id']; - } - } - - if (count($ids) > 1) { - $this->errors[$path] = sprintf( - 'Preset-Name "%s" ist mehrdeutig; bitte die ID angeben: %s.', - $wanted, - implode(', ', $ids) - ); - - return null; - } - - return $ids[0] ?? null; - } - /** * Ruling S13: the short-form color check (ShortformWriter::color()), never text(). * @@ -494,7 +464,8 @@ final class Composer } /** - * "1_3,2_3" → flexTypes ["8_24", "16_24"]; the fractions must add up to one row. + * "1_3,2_3" → flexTypes ["8_24", "16_24"]; the fractions must add up to exactly one row + * (exact integer arithmetic over the common denominator, ruling S34 – no float tolerance). * * @return list */ @@ -502,7 +473,7 @@ final class Composer { $parts = is_string($columns) ? array_map('trim', explode(',', $columns)) : []; $widths = []; - $sum = 0.0; + $fractions = []; foreach ($parts as $part) { $valid = preg_match('/^(\d{1,2})_(\d{1,2})$/D', $part, $match) === 1 && (int) $match[1] >= 1 @@ -523,10 +494,10 @@ final class Composer return ['24_24']; } - $sum += $numerator / $denominator; + $fractions[] = [$numerator, $denominator]; } - if ($widths === [] || abs($sum - 1.0) > 0.01) { + if ($widths === [] || ! self::sumsToOne($fractions)) { $this->errors[$path] = 'die Spaltenbreiten müssen zusammen eine volle Zeile ergeben (z. B. "1_3,2_3")'; return ['24_24']; @@ -535,6 +506,32 @@ final class Composer return $widths; } + /** + * Whether the fractions add up to exactly 1: numerators scaled to the least common + * denominator (denominators are divisors of 24 or 5, 7, 9, 10, 11 – at most 27720). + * + * @param list $fractions [numerator, denominator] + */ + private static function sumsToOne(array $fractions): bool + { + $common = 1; + foreach ($fractions as [, $denominator]) { + $common = intdiv($common * $denominator, self::gcd($common, $denominator)); + } + + $sum = 0; + foreach ($fractions as [$numerator, $denominator]) { + $sum += $numerator * intdiv($common, $denominator); + } + + return $sum === $common; + } + + private static function gcd(int $a, int $b): int + { + return $b === 0 ? $a : self::gcd($b, $a % $b); + } + /** * @param array $spec * @param list $allowed diff --git a/src/Integrations/Divi/Compose/ShortformWriter.php b/src/Integrations/Divi/Compose/ShortformWriter.php index 8442906..a709625 100644 --- a/src/Integrations/Divi/Compose/ShortformWriter.php +++ b/src/Integrations/Divi/Compose/ShortformWriter.php @@ -177,7 +177,9 @@ final class ShortformWriter */ private function alt(string $module, string $path, string $alt, string $altKey): string { - if ($alt === '' || VariableRef::contains($alt)) { + // Controls are stripped at the choke point before its marker check (S32a) – so a + // NUL-split marker counts as a marker here too, and a canonical result is refused. + if ($alt === '' || VariableRef::contains(VariableRef::withoutControls($alt))) { return ''; } @@ -187,7 +189,7 @@ final class ShortformWriter return ''; } - return is_string($clean) ? $clean : ''; + return is_string($clean) && ! VariableRef::contains($clean) ? $clean : ''; } /** diff --git a/src/Integrations/Divi/Compose/StructureRules.php b/src/Integrations/Divi/Compose/StructureRules.php index 7de7bc4..c2645e8 100644 --- a/src/Integrations/Divi/Compose/StructureRules.php +++ b/src/Integrations/Divi/Compose/StructureRules.php @@ -10,15 +10,19 @@ use LIT\MCP\Integrations\Divi\Schema\ModuleCatalog; /** * Which node may sit where (spec §5.4 "Struktur"): page → sections; regular section → rows; - * fullwidth section → fullwidth modules; column/group → modules; parent module → its child - * modules (catalog). Columns only come with their row; specialty sections and read-only - * nodes (opaque, non-Divi) are not rebuilt. Pure PHP. + * fullwidth section → fullwidth modules (catalog category "fullwidth-module"); column/group → + * modules; column → a nested row too (Divi 5 row is "nestable": true; neither the server + * – BlockParserStore::is_nested_module() only adds et_pb_row_nested – nor the builder's drop + * rules limit the depth); parent module → its child modules (catalog). Columns only come with + * their row; specialty sections, specialty columns (module.advanced.specialtyColumns or + * row-inner children, ColumnModule.php:306-324) and read-only nodes (opaque, non-Divi) are not + * rebuilt. Pure PHP. */ final class StructureRules { private const COLUMNS = ['divi/column', 'divi/column-inner']; private const ROWS = ['divi/row', 'divi/row-inner']; - private const FULLWIDTH = 'divi/fullwidth-'; + private const FULLWIDTH = 'fullwidth-module'; public function __construct(private ModuleCatalog $catalog) { @@ -49,8 +53,15 @@ final class StructureRules . 'einfügen'; } + if (in_array($parentName, self::COLUMNS, true) && self::isSpecialtyColumn($parent)) { + return 'Spezialspalten (specialty) nur im Divi-Builder umbauen; Module in ihre inneren Spalten einfügen'; + } + if (in_array($parentName, self::COLUMNS, true) || $parentName === 'divi/group') { - return $this->isPlainModule($child) || $child === 'divi/group' + // A nested row: only straight into a (Divi 5) column – like the builder's drop target. + $nestedRow = $child === 'divi/row' && $parentName === 'divi/column'; + + return $this->isPlainModule($child) || $child === 'divi/group' || $nestedRow ? null : sprintf('%s passt nicht in eine Spalte', $child); } @@ -65,10 +76,27 @@ final class StructureRules : sprintf('%s erlaubt nur %s als Kind', $parentName, implode(', ', $children)); } - /** A module that goes into a column: category "module", not a fullwidth module. */ + /** A module that goes into a column: catalog category "module" (not "fullwidth-module"). */ public function isPlainModule(string $name): bool { - return $this->catalog->category($name) === 'module' && ! str_starts_with($name, self::FULLWIDTH); + return $this->catalog->category($name) === 'module'; + } + + /** A column of a specialty section: it holds inner rows, not modules. */ + private static function isSpecialtyColumn(Block $column): bool + { + $specialty = AttrMap::desktop($column->attrs(), 'module.advanced.specialtyColumns'); + if ($specialty !== null && $specialty !== '') { + return true; + } + + foreach ($column->children() as $child) { + if ($child->name() === 'divi/row-inner') { + return true; + } + } + + return false; } private function inSection(Block $section, string $child): ?string @@ -79,7 +107,7 @@ final class StructureRules } if ($type === 'fullwidth') { - return str_starts_with($child, self::FULLWIDTH) + return $this->catalog->category($child) === self::FULLWIDTH ? null : 'in einen Vollbreite-Abschnitt passen nur fullwidth-Module'; } diff --git a/src/Integrations/Divi/Compose/VariableRef.php b/src/Integrations/Divi/Compose/VariableRef.php index c054883..7367d58 100644 --- a/src/Integrations/Divi/Compose/VariableRef.php +++ b/src/Integrations/Divi/Compose/VariableRef.php @@ -136,9 +136,10 @@ final class VariableRef } /** - * Removes C0 control characters except tab, LF and CR, and DEL (ruling S32). The WordPress - * sanitizers keep them (sanitize_text_field keeps NUL), and "$variable\0(" would otherwise - * hide a marker from the final-string check that a later strip anywhere could rebuild. + * Removes C0 control characters except tab, LF and CR, and DEL (rulings S32/S32a). Applied + * to RAW values only, before the marker check and the sanitizer: the WordPress sanitizers keep + * them (sanitize_text_field keeps NUL; strip_tags reads "<" + VT/FF as a literal "<"), so a + * strip after sanitizing could build a marker or a tag no check saw. */ public static function withoutControls(string $value): string { @@ -341,6 +342,12 @@ final class VariableRef /** Cleans one content setting by its field type (ruling S21/S22). */ private static function setting(string $key, mixed $value, string $fieldType, ValueSanitizer $sanitizer): mixed { + // Ruling S32/S32a: control characters go on the RAW value, before every check and the + // sanitizer ("<" + VT stays a literal "<" for strip_tags; a later strip would build a tag). + if (is_string($value)) { + $value = self::withoutControls($value); + } + if (is_string($value) && (self::contains($value) || str_contains($value, ')$'))) { throw self::notAlone(); } @@ -366,12 +373,12 @@ final class VariableRef $clean = is_string($value) ? $sanitizer->text($value) : $value; } - // Ruling S28: sanitizing may rebuild a marker ("$variable(" → "$variable("); control - // characters go first so a NUL-split marker is rebuilt – and caught – here (S32). - if (is_string($clean)) { - $clean = self::withoutControls($clean); - } - if (is_string($clean) && (self::contains($clean) || str_contains($clean, ')$'))) { + // Ruling S28: sanitizing may rebuild a marker ("$variable(" → "$variable("). After the + // sanitizer the only action is rejection (S32a) – controls went on the raw value above. + if ( + is_string($clean) + && (self::contains($clean) || str_contains($clean, ')$') || self::withoutControls($clean) !== $clean) + ) { throw self::notAlone(); } diff --git a/src/Integrations/Divi/Design/DesignSystem.php b/src/Integrations/Divi/Design/DesignSystem.php index ceaa554..a030b59 100644 --- a/src/Integrations/Divi/Design/DesignSystem.php +++ b/src/Integrations/Divi/Design/DesignSystem.php @@ -4,6 +4,8 @@ declare(strict_types=1); namespace LIT\MCP\Integrations\Divi\Design; +use InvalidArgumentException; + /** * The site's Divi design system as read live from Divi (spec §2 principle 2, §5.7): global * colors, global variables, theme fonts and sizes, module and group presets. Pure PHP – @@ -162,21 +164,40 @@ final class DesignSystem return null; } - /** Module preset ID by name or ID for a full module name ("divi/button"); null when unknown. */ + /** + * Module preset ID for a full module name ("divi/button"): an exact ID first, else the one + * preset of that name (case-insensitive); null when unknown. + * + * @throws InvalidArgumentException When several presets of the module share the name (ruling S13a). + */ public function modulePresetId(string $module, string $name): ?string { - $key = mb_strtolower(trim($name)); + $wanted = trim($name); + $key = mb_strtolower($wanted); + $ids = []; foreach ($this->presets as $preset) { - if ( - $preset['kind'] === 'module' - && $preset['module'] === $module - && ($preset['id'] === trim($name) || mb_strtolower($preset['name']) === $key) - ) { + if ($preset['kind'] !== 'module' || $preset['module'] !== $module) { + continue; + } + + if ($preset['id'] === $wanted) { return $preset['id']; } + + if (mb_strtolower($preset['name']) === $key) { + $ids[] = $preset['id']; + } } - return null; + if (count($ids) > 1) { + throw new InvalidArgumentException(sprintf( + 'Preset-Name "%s" ist mehrdeutig; bitte die ID angeben: %s.', + $wanted, + implode(', ', $ids) + )); + } + + return $ids[0] ?? null; } /** @return list Names of the module presets of $module. */ diff --git a/tests/Unit/Integrations/Divi/Compose/AttrWriterTest.php b/tests/Unit/Integrations/Divi/Compose/AttrWriterTest.php index a2a939c..78afb71 100644 --- a/tests/Unit/Integrations/Divi/Compose/AttrWriterTest.php +++ b/tests/Unit/Integrations/Divi/Compose/AttrWriterTest.php @@ -177,9 +177,6 @@ final class AttrWriterTest extends TestCase $cases['tag-split marker in rich text'] = [ 'divi/text', 'content.innerContent', ['desktop' => '$variable' . $reference], self::TEXT, ]; - $cases['NUL-split marker in rich text'] = [ - 'divi/text', 'content.innerContent', ['desktop' => '$variable' . chr(0) . $reference], self::TEXT, - ]; $cases['octet-split marker in a text title'] = [ 'divi/heading', 'title.innerContent', ['desktop' => '$variable%41' . $reference], self::HEADING, ]; @@ -228,7 +225,7 @@ final class AttrWriterTest extends TestCase public function testSplitMarkersAreRebuiltBySanitizing(): void { $sanitizer = ComposeFixture::sanitizer(); - $rows = ['tag-split marker in rich text', 'NUL-split marker in rich text', 'octet-split marker in a text title', + $rows = ['tag-split marker in rich text', 'octet-split marker in a text title', 'tag-split marker deep in a value']; foreach (array_intersect_key(self::malicious(), array_flip($rows)) as $name => [, , $map]) { $raw = (string) json_encode($map, JSON_UNESCAPED_SLASHES); @@ -236,7 +233,6 @@ final class AttrWriterTest extends TestCase self::assertStringNotContainsString('$variable(', $value, $name); self::assertStringContainsString('$variable(', $sanitizer->text($value) . $sanitizer->html($value), $name . ' ' . $raw); } - self::assertSame('$variable' . chr(0) . '(', substr(self::malicious()['NUL-split marker in rich text'][2]['desktop'], 0, 11)); } /** Ruling S21: an editor's script in before/after is cleaned out, the reference re-serialized. */ @@ -385,7 +381,10 @@ final class AttrWriterTest extends TestCase self::assertSame("

a\nb\r

", $writer->cleanValue('divi/text', 'content.innerContent', "

a\x0b\nb\r\x08

")); self::assertSame('Titel', $writer->cleanValue('divi/heading', 'title.innerContent', "Ti\x00tel\x7f")); - $tail = '({"type":"color","value":{"name":"gcid-a1b2","settings":{}}})$'; + // 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":"","after":""}}})$'; + $canonical = '$variable({"type":"content","value":{"name":"post_title","settings":{"before":"","after":""}}})$'; $targets = [ 'text' => ['divi/heading', 'title.innerContent'], 'html' => ['divi/text', 'content.innerContent'], @@ -393,19 +392,55 @@ final class AttrWriterTest extends TestCase ]; 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 must catch it. - self::assertStringNotContainsString('$variable(', ComposeFixture::sanitizer()->text($value)); + // 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]) { - try { - $writer->cleanValue($module, $path, $value); - self::fail(sprintf('%s: control %d not caught', $kind, ord($control))); - } catch (InvalidArgumentException $e) { - self::assertStringContainsString('allein im Wert', $e->getMessage()); - } + self::assertSame($canonical, $writer->cleanValue($module, $path, $value), $kind . ' ' . ord($control)); + } + // Embedded in text it is an embedded reference: refused. + try { + $writer->cleanValue('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 S32a: "<" followed by VT/FF is a literal "<" for strip_tags / sanitize_text_field; + * stripping the control character only after the sanitizer would build a real tag. Controls + * go on the RAW value, so the sanitizer sees – and removes – the tag. + */ + public function testControlCharactersGoBeforeTheSanitizer(): void + { + $writer = ComposeFixture::attrWriter(); + $img = '<' . chr(11) . 'img src=x onerror=alert(1)>'; + $script = '<' . chr(12) . 'script>alert(1)'; + // The fake reproduces the WordPress behaviour the attack relies on. + self::assertStringContainsString('<' . chr(11) . 'img', ComposeFixture::sanitizer()->text($img)); + + foreach ([$img . 'Titel', $script . 'Titel'] as $value) { + $result = $writer->apply('divi/heading', self::HEADING, ['title.innerContent' => ['desktop' => $value]], 'attrs'); + self::assertSame([], $result['errors']); + $stored = $result['attrs']['title']['innerContent']['desktop']['value']; + self::assertStringNotContainsString('<', $stored); + + $button = $writer->cleanValue('divi/button', 'button.innerContent', $value, 'text'); + self::assertStringNotContainsString('<', $button); + } + } + + /** Ruling S34: value keys are whole identifiers – "$" must not match before a trailing newline. */ + public function testValueKeysRejectATrailingNewline(): void + { + $result = ComposeFixture::attrWriter()->apply('divi/text', self::TEXT, [ + 'module.decoration.spacing' => ['desktop' => ['padding' => ["top\n" => '1px']]], + ], 'attrs'); + + self::assertStringContainsString('Schlüssel', $result['errors']['attrs.module.decoration.spacing']); + } + /** Ruling S33a: a kind override may only tighten the routed kind (code < html < text; url is its own family). */ public function testKindOverrideMayOnlyTighten(): void { diff --git a/tests/Unit/Integrations/Divi/Compose/ComposeFixture.php b/tests/Unit/Integrations/Divi/Compose/ComposeFixture.php index 54f5afb..83a8090 100644 --- a/tests/Unit/Integrations/Divi/Compose/ComposeFixture.php +++ b/tests/Unit/Integrations/Divi/Compose/ComposeFixture.php @@ -111,6 +111,9 @@ final class ComposeFixture 12 => ['url' => 'https://kunde.test/wp-content/uploads/halle.jpg', 'alt' => ''], // Alt texts carrying a reference, whole or NUL-split (ruling S33b: dropped). 44 => ['url' => 'https://kunde.test/wp-content/uploads/a.jpg', 'alt' => ComposeFixture::REFERENCE_ALT], + // "<" + VT/FF: strip_tags keeps it as text – a later strip would build the tag (S32a). + 46 => ['url' => 'https://kunde.test/wp-content/uploads/c.jpg', 'alt' => '<' . chr(11) + . 'img src=x onerror=alert(1)>Team'], 45 => ['url' => 'https://kunde.test/wp-content/uploads/b.jpg', 'alt' => '$variable' . chr(0) . substr(ComposeFixture::REFERENCE_ALT, strlen('$variable'))], default => null, diff --git a/tests/Unit/Integrations/Divi/Compose/ComposerTest.php b/tests/Unit/Integrations/Divi/Compose/ComposerTest.php index eff8f84..f3f22bd 100644 --- a/tests/Unit/Integrations/Divi/Compose/ComposerTest.php +++ b/tests/Unit/Integrations/Divi/Compose/ComposerTest.php @@ -128,6 +128,10 @@ final class ComposerTest extends TestCase self::assertSame(['24_24'], $widths('1_7,5_7')[0]); self::assertArrayHasKey('n[0].rows[0].columns', $widths('1_7,5_7')[1]); self::assertArrayHasKey('n[0].rows[0].columns', $widths('abc')[1]); + // Ruling S34: exact fractions – 1/11 + 11/12 is 133/132, not "about one". + self::assertArrayHasKey('n[0].rows[0].columns', $widths('1_11,11_12')[1]); + self::assertSame([['1_5', '1_5', '1_5', '1_5', '1_5'], []], $widths('1_5,1_5,1_5,1_5,1_5')); + self::assertSame([['8_24', '8_24', '8_24'], []], $widths('1_3,1_3,1_3')); } public function testUpdateChangesShortFormPresetAndLabel(): void @@ -191,9 +195,14 @@ final class ComposerTest extends TestCase ['type' => 'section', 'label' => $reference], ['type' => 'section', 'label' => ['Hero']], ['type' => 'section', 'modules' => [['type' => 'heading', 'label' => "Titel\x01\x7f"]]], + ['type' => 'section', 'label' => '<' . chr(11) . 'img src=x onerror=alert(1)>Hero'], + ['type' => 'section', 'label' => '<' . chr(12) . 'script>alert(1)Hero'], ], null, 'nodes'); - [$clean, $split, $nul, $whole, $list, $control] = $result['blocks']; + [$clean, $split, $nul, $whole, $list, $control, $vt, $ff] = $result['blocks']; + // Ruling S32a: controls go before the sanitizer, so no tag is built after it. + self::assertSame('Hero', NodeSummary::label($vt)); + self::assertSame('Hero', NodeSummary::label($ff)); self::assertSame('Hero', NodeSummary::label($clean)); self::assertSame('Hero – Heading', NodeSummary::label($clean->children()[0]->children()[0]->children()[0])); @@ -344,6 +353,24 @@ final class ComposerTest extends TestCase self::assertSame([], $readOnly['blocks']); } + /** Ruling S34: Divi 5 rows are nestable – a row may go into a column. */ + public function testNestedRowInAColumn(): void + { + $result = ComposeFixture::composer()->compose([['type' => 'section', 'label' => 'Hero', 'rows' => [[ + 'columns' => '1_2,1_2', + 'modules' => [ + [['type' => 'row', 'columns' => '1_2,1_2', 'modules' => [[['type' => 'text', 'content' => '

a

']], []]]], + [['type' => 'text', 'content' => '

b

']], + ], + ]]]], null, 'nodes'); + + self::assertSame([], $result['errors']); + $nested = $result['blocks'][0]->children()[0]->children()[0]->children()[0]; + self::assertSame('divi/row', $nested->name()); + self::assertSame(['12_24', '12_24'], array_map([NodeSummary::class, 'columnWidth'], $nested->children())); + self::assertSame('divi/text', $nested->children()[0]->children()[0]->name()); + } + public function testRejectsNonObjectsAndMissingTypes(): void { $result = ComposeFixture::composer()->compose(['section', [], ['label' => 'x']], null, 'nodes'); diff --git a/tests/Unit/Integrations/Divi/Compose/ShortformWriterTest.php b/tests/Unit/Integrations/Divi/Compose/ShortformWriterTest.php index 70ca3bf..f9c0e01 100644 --- a/tests/Unit/Integrations/Divi/Compose/ShortformWriterTest.php +++ b/tests/Unit/Integrations/Divi/Compose/ShortformWriterTest.php @@ -186,7 +186,7 @@ final class ShortformWriterTest extends TestCase 'color: two references' => ['color', $color . $color], // Markers the sanitizer would rebuild (S28). 'text: tag-split marker' => ['text', '$variable(' . substr($color, 10)], - 'text: NUL-split marker' => ['text', '$variable' . chr(0) . '(' . substr($color, 10)], + 'text: NUL-split marker embedded' => ['text', 'Hallo $variable' . chr(0) . '(' . substr($color, 10)], 'text: octet-split marker' => ['text', '$variable%41(' . substr($color, 10)], 'url: stripped-characters marker' => ['url', '$variable<"{}>(y)$'], 'url: NUL-split marker' => ['url', '$variable' . chr(0) . '(y)$'], @@ -220,17 +220,17 @@ final class ShortformWriterTest extends TestCase self::assertStringContainsString('$variable(', $sanitizer->text($rows[$row][1])); } - // S32: like sanitize_text_field, the fake text() keeps NUL – so the marker the NUL row - // hides is rebuilt only by AttrWriter's control-character stripping (production code). - self::assertStringNotContainsString('$variable(', $sanitizer->text($rows['text: NUL-split marker'][1])); - self::assertStringContainsString(chr(0), $sanitizer->text($rows['text: NUL-split marker'][1])); + // S32/S32a: like sanitize_text_field, the fake text() keeps NUL – the marker the NUL row + // hides appears only through AttrWriter's control stripping on the raw value (production code). + self::assertStringNotContainsString('$variable(', $sanitizer->text($rows['text: NUL-split marker embedded'][1])); + self::assertStringContainsString(chr(0), $sanitizer->text($rows['text: NUL-split marker embedded'][1])); foreach (['url: stripped-characters marker', 'url: NUL-split marker', 'url: line-break octet marker'] as $row) { self::assertStringNotContainsString('$variable(', $rows[$row][1]); self::assertStringContainsString('$variable(', $sanitizer->url($rows[$row][1])); } - self::assertStringContainsString(chr(0), $rows['text: NUL-split marker'][1]); + self::assertStringContainsString(chr(0), $rows['text: NUL-split marker embedded'][1]); } /** Ruling S33b: an attachment alt carrying a reference (whole or split) is dropped, never canonicalized. */ @@ -245,6 +245,21 @@ final class ShortformWriterTest extends TestCase } } + /** Ruling S32a: a control character after "<" never turns into a tag – in text and in media alt. */ + public function testControlSplitTagsAreRemoved(): void + { + $writer = ComposeFixture::shortform(); + foreach (['<' . chr(11) . 'img src=x onerror=alert(1)>Hallo', '<' . chr(12) . 'script>alert(1)Hallo'] as $value) { + $result = $writer->apply('heading', [], ['text' => $value], 'n'); + self::assertSame([], $result['errors']); + self::assertSame('Hallo', $result['attrs']['title']['innerContent']['desktop']['value']); + } + + $image = $writer->apply('image', [], ['image_id' => 46], 'n'); + self::assertSame([], $image['errors']); + self::assertSame('Team', $image['attrs']['image']['innerContent']['desktop']['value']['alt']); + } + /** Ruling S21 applies to short-form text too: before/after of a reference are cleaned, not rejected. */ public function testReferenceSettingsInTextAreCleaned(): void { diff --git a/tests/Unit/Integrations/Divi/Compose/StructureRulesTest.php b/tests/Unit/Integrations/Divi/Compose/StructureRulesTest.php index 38f00cb..4513345 100644 --- a/tests/Unit/Integrations/Divi/Compose/StructureRulesTest.php +++ b/tests/Unit/Integrations/Divi/Compose/StructureRulesTest.php @@ -28,6 +28,18 @@ final class StructureRulesTest extends TestCase 'anything in a specialty section' => [$section('specialty'), 'divi/row', 'Spezialabschnitte'], 'column in a row' => [Block::create('divi/row'), 'divi/column', 'Spalten entstehen mit der Zeile'], 'module in a column' => [Block::create('divi/column'), 'divi/button', null], + 'row in a column (nestable)' => [Block::create('divi/column'), 'divi/row', null], + 'row in a group' => [Block::create('divi/group'), 'divi/row', 'passt nicht'], + 'module in a specialty column' => [ + Block::create('divi/column', ['module' => ['advanced' => ['specialtyColumns' => ['desktop' => ['value' => '2']]]]]), + 'divi/text', + 'Spezial', + ], + 'module in a column holding inner rows' => [ + Block::create('divi/column', [], [Block::create('divi/row-inner')]), + 'divi/text', + 'Spezial', + ], 'group in a column' => [Block::create('divi/column'), 'divi/group', null], 'module in a group' => [Block::create('divi/group'), 'divi/text', null], 'fullwidth module in a column' => [Block::create('divi/column'), 'divi/fullwidth-header', 'passt nicht'], diff --git a/tests/Unit/Integrations/Divi/Compose/VariableRefTest.php b/tests/Unit/Integrations/Divi/Compose/VariableRefTest.php index 32efd41..6659ea9 100644 --- a/tests/Unit/Integrations/Divi/Compose/VariableRefTest.php +++ b/tests/Unit/Integrations/Divi/Compose/VariableRefTest.php @@ -93,6 +93,15 @@ final class VariableRefTest extends TestCase self::content('post_title', '{"before":"' . $script . 'Hallo","after":""}'), self::content('post_title', '{"before":"Hallo","after":""}'), ], + // S32a: controls go before text(), so "<" + VT/FF is a tag text() removes. + 'VT-split tag in date format' => [ + self::content('post_date', '{"custom_date_format":"<' . self::escape('000b') . 'img src=x onerror=alert(1)>d.m"}'), + self::content('post_date', '{"custom_date_format":"d.m"}'), + ], + 'FF-split script in date format' => [ + self::content('post_date', '{"custom_date_format":"<' . self::escape('000c') . 'script>alert(1)d.m"}'), + self::content('post_date', '{"custom_date_format":"d.m"}'), + ], 'tags in date format' => [ self::content('post_date', '{"custom_date_format":"d.m"}'), self::content('post_date', '{"custom_date_format":"d.m"}'), diff --git a/tests/Unit/Integrations/Divi/Design/DesignSystemTest.php b/tests/Unit/Integrations/Divi/Design/DesignSystemTest.php index 34f78e3..c1d9a9b 100644 --- a/tests/Unit/Integrations/Divi/Design/DesignSystemTest.php +++ b/tests/Unit/Integrations/Divi/Design/DesignSystemTest.php @@ -83,6 +83,20 @@ final class DesignSystemTest extends TestCase self::assertSame('{{color:Nachtblau}}', DesignSystem::colorPlaceholder('gcid-a1b2', 'Nachtblau')); } + /** Ruling S13a/S34: a preset name several presets of the module share fails with their ids. */ + public function testAmbiguousPresetNamesFail(): void + { + $design = DesignSystem::fromDivi([], [], ['module' => ['divi/button' => ['default' => 'p-1', 'items' => [ + 'p-1' => ['id' => 'p-1', 'name' => 'Standard', 'moduleName' => 'divi/button'], + 'p-3' => ['id' => 'p-3', 'name' => 'standard', 'moduleName' => 'divi/button'], + ]]]], []); + + self::assertSame('p-3', $design->modulePresetId('divi/button', 'p-3')); + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('p-1, p-3'); + $design->modulePresetId('divi/button', 'STANDARD'); + } + public function testToleratesEmptyOrOddInput(): void { $design = DesignSystem::fromDivi([], ['fonts' => 'x'], ['module' => 'y'], []);