fix: Task 8 review round 1 - quote-aware tokenizer, context check, skipped global nodes (S45)
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:
Claude Opus 5.5
@@ -0,0 +1,83 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace LIT\MCP\Tests\Unit\Integrations\Divi\Compose;
|
||||
|
||||
use LIT\MCP\Integrations\Divi\Compose\HtmlContext;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
final class HtmlContextTest extends TestCase
|
||||
{
|
||||
/**
|
||||
* HTML with the probe marked as [[…]]: whether the browser reads it as text.
|
||||
*
|
||||
* @return array<string, array{string, bool}>
|
||||
*/
|
||||
public static function probes(): array
|
||||
{
|
||||
return [
|
||||
'plain text' => ['a [[B]] c', true],
|
||||
'between tags' => ['<p>[[B]]</p>', true],
|
||||
'double-quoted ">" keeps the tag' => ['<a title="A > [[B]]" href="/x">T</a>', false],
|
||||
'single-quoted ">" keeps the tag' => ["<img alt='A > [[B]]' src=x>", false],
|
||||
'quoted value after spaces around =' => ['<a title = "A > [[B]]">', false],
|
||||
'after a quoted ">"' => ['<a title="A > B">[[T]]</a>', true],
|
||||
'quote in an attribute name is no value' => ['<a b"c>[[T]]', true],
|
||||
'unquoted value' => ['<a href=[[x]]>', false],
|
||||
'end tag' => ['</a [[x]]>', false],
|
||||
'comment' => ['<!-- a > [[B]] -->', false],
|
||||
'after a comment' => ['<!-- a -->[[B]]', true],
|
||||
'comment closed by --!>' => ['<!-- a --!>[[B]]', true],
|
||||
'abrupt comment' => ['<!-->[[B]]', true],
|
||||
'unterminated comment' => ['<!-- a [[B]]', false],
|
||||
'bogus comment' => ['<!DOCTYPE [[x]]>', false],
|
||||
'processing instruction' => ['<?x [[B]] >', false],
|
||||
'unterminated tag' => ['<a title="x [[B]]', false],
|
||||
'unterminated quote' => ['<a title="x>[[B]]</a>', false],
|
||||
'lone "<"' => ['a < [[B]]', true],
|
||||
'"<" + digit' => ['a <3 [[B]]', true],
|
||||
'script body' => ['<script>var x = "[[B]]";</script>', false],
|
||||
'after the script' => ['<script>x</script>[[B]]', true],
|
||||
'script end needs a delimiter' => ['<script>x</script-x>[[B]]</script>', false],
|
||||
'script end in upper case' => ['<SCRIPT>x</Script >[[B]]', true],
|
||||
'double-escaped script' => ['<script><!--<script></script>[[B]]</script>', false],
|
||||
'style body' => ['<style>.a{} [[B]]</style>', false],
|
||||
'iframe body' => ['<iframe>[[B]]</iframe>', false],
|
||||
'plaintext' => ['<plaintext></plaintext>[[B]]', false],
|
||||
'textarea is text' => ['<textarea>[[B]]</textarea>', true],
|
||||
];
|
||||
}
|
||||
|
||||
/** @dataProvider probes */
|
||||
public function testKnowsWhatIsText(string $marked, bool $text): void
|
||||
{
|
||||
$start = (int) strpos($marked, '[[');
|
||||
$html = str_replace(['[[', ']]'], '', $marked);
|
||||
$end = (int) strpos($marked, ']]') - 2;
|
||||
|
||||
$spans = HtmlContext::markupSpans($html);
|
||||
|
||||
self::assertSame($text, HtmlContext::isText($spans, $start, $end), $marked);
|
||||
}
|
||||
|
||||
public function testBoundaries(): void
|
||||
{
|
||||
$spans = HtmlContext::markupSpans('<p>ab</p>');
|
||||
|
||||
self::assertSame([[0, 3], [5, 9]], $spans);
|
||||
self::assertTrue(HtmlContext::isBoundary($spans, 3));
|
||||
self::assertTrue(HtmlContext::isBoundary($spans, 5));
|
||||
self::assertFalse(HtmlContext::isBoundary($spans, 1));
|
||||
}
|
||||
|
||||
public function testLongInputsStayLinear(): void
|
||||
{
|
||||
$start = microtime(true);
|
||||
foreach (['<', '<a ', '<a title="', '<!--', '<script>', '</'] as $unit) {
|
||||
HtmlContext::markupSpans(str_repeat($unit, 100000));
|
||||
}
|
||||
|
||||
self::assertLessThan(2.0, microtime(true) - $start);
|
||||
}
|
||||
}
|
||||
@@ -195,6 +195,134 @@ final class TextReplacerTest extends TestCase
|
||||
(new TextReplacer($search, $replace))->apply(self::text($value), self::same());
|
||||
}
|
||||
|
||||
/**
|
||||
* Ruling S45: a ">" inside a quoted attribute value does not end the tag, so nothing after it
|
||||
* in the tag is text. With the old "<[^>]*>" split these tests fail (the rest of the tag
|
||||
* was searched), which is how a reverted tokenizer shows.
|
||||
*/
|
||||
public function testQuotedGreaterThanDoesNotEndTheTag(): void
|
||||
{
|
||||
$double = '<a title="A > Kontakt" href="/kontakt">Kontakt</a> und Kontakt';
|
||||
$single = "<img alt='x > Kontakt' src='/kontakt.jpg'> Kontakt";
|
||||
|
||||
$result = (new TextReplacer('Kontakt', 'Anfrage'))->apply(self::text($double), self::same());
|
||||
$image = (new TextReplacer('Kontakt', 'Anfrage'))->apply(self::text($single), self::same());
|
||||
|
||||
self::assertSame('<a title="A > Kontakt" href="/kontakt">Anfrage</a> und Anfrage', self::valueOf($result['attrs']));
|
||||
self::assertSame(2, $result['count']);
|
||||
self::assertSame("<img alt='x > Kontakt' src='/kontakt.jpg'> Anfrage", self::valueOf($image['attrs']));
|
||||
}
|
||||
|
||||
public function testCommentsAreNoText(): void
|
||||
{
|
||||
$value = "<!-- Kontakt > Kontakt\n<b> --><p>Kontakt</p><!---->Kontakt";
|
||||
|
||||
$result = (new TextReplacer('Kontakt', 'Anfrage'))->apply(self::text($value), self::same());
|
||||
|
||||
self::assertSame("<!-- Kontakt > Kontakt\n<b> --><p>Anfrage</p><!---->Anfrage", self::valueOf($result['attrs']));
|
||||
}
|
||||
|
||||
/** More raw-text elements; a closing tag must really close ("</script-x>" does not). */
|
||||
public function testRawTextBodiesOfAllKinds(): void
|
||||
{
|
||||
$value = '<iframe>Kontakt</iframe><noscript>Kontakt</noscript><script>a</script-x> Kontakt</script>'
|
||||
. '<textarea>Kontakt</textarea>';
|
||||
|
||||
$result = (new TextReplacer('Kontakt', 'Anfrage'))->apply(self::text($value), self::same());
|
||||
|
||||
self::assertSame(1, $result['count']);
|
||||
self::assertStringEndsWith('<textarea>Anfrage</textarea>', (string) self::valueOf($result['attrs']));
|
||||
}
|
||||
|
||||
/** Search == replacement (or a cleaned replacement equal to the match) is no change and no hit. */
|
||||
public function testNoEffectiveChangeIsNoHit(): void
|
||||
{
|
||||
$same = (new TextReplacer('Müller', 'Müller'))->apply(self::ATTRS, self::same());
|
||||
$cleanedAway = (new TextReplacer('Müller', '<b>Müller</b>'))->apply(
|
||||
self::ATTRS,
|
||||
static fn (string $p, ?string $k, string $v): string => strip_tags($v)
|
||||
);
|
||||
$partly = (new TextReplacer('müller', 'Müller', false))->apply(self::ATTRS, self::same());
|
||||
|
||||
self::assertSame(0, $same['count']);
|
||||
self::assertSame(self::ATTRS, $same['attrs']);
|
||||
self::assertSame(0, $cleanedAway['count']);
|
||||
self::assertSame(self::ATTRS, $cleanedAway['attrs']);
|
||||
self::assertSame(1, $partly['count'], 'only MÜLLER changes');
|
||||
}
|
||||
|
||||
/**
|
||||
* Ruling S45 malicious rows: either refused (exception) or no change at all.
|
||||
*
|
||||
* @return array<string, array{string, string, string}>
|
||||
*/
|
||||
public static function s45(): array
|
||||
{
|
||||
return [
|
||||
'double-quoted ">" then handler' => ['<a title="A > B" href="/kontakt">Link</a>', ' B', '" onmouseover="alert(1)'],
|
||||
'single-quoted ">" then handler' => ["<img alt='A > B' src='/b.jpg'>", ' B', "' onerror='alert(1)"],
|
||||
'href rewrite after quoted ">"' => ['<a title="x > y" href="/kontakt">Link</a>', 'kontakt', 'javascript:alert(1)'],
|
||||
'comment with ">"' => ['<!-- a > B --><p>x</p>', ' B', '--><img src=x onerror=alert(1)>'],
|
||||
'comment closed by --!>' => ['<!-- B --!><p>x</p>', ' B', 'x'],
|
||||
'unterminated comment' => ['<p>x B</p><!-- B', ' B', 'y'],
|
||||
'unterminated tag' => ['<p>ok B</p><a title="x B', ' B', 'y'],
|
||||
'unbalanced quote' => ['<a title="x>B B</a>', ' B', 'y'],
|
||||
'unquoted "<" in a tag' => ['<a <b>x B</b>', ' B', 'y'],
|
||||
'lone "<" before text' => ['a < B', ' B', 'img src=x onerror=alert(1)>'],
|
||||
'"<" + letter in text' => ['a<B c', ' c', 'y'],
|
||||
'raw body' => ['<iframe>x B</iframe>', ' B', '<img src=x onerror=alert(1)>'],
|
||||
'fake script end' => ['<script>a</script-x> B</script>', ' B', 'y'],
|
||||
'double-escaped script' => ['<script><!--<script></script> B--></script>', ' B', 'y'],
|
||||
'unterminated reference' => ['$variable({"type":"content"} B', ' B', ')$'],
|
||||
];
|
||||
}
|
||||
|
||||
/** @dataProvider s45 */
|
||||
public function testS45RowsAreRefusedOrUnchanged(string $value, string $search, string $replace): void
|
||||
{
|
||||
try {
|
||||
$result = (new TextReplacer($search, $replace))->apply(self::text($value), self::same());
|
||||
} catch (InvalidArgumentException $e) {
|
||||
self::assertStringContainsString('content.innerContent', $e->getMessage());
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
self::assertSame(0, $result['count']);
|
||||
self::assertSame($value, self::valueOf($result['attrs']));
|
||||
}
|
||||
|
||||
/** @return array<string, array{string}> */
|
||||
public static function longInputs(): array
|
||||
{
|
||||
$n = 200000;
|
||||
|
||||
return [
|
||||
'many "<"' => [str_repeat('<', $n) . ' Kontakt'],
|
||||
'many "<a"' => [str_repeat('<a ', intdiv($n, 3)) . ' Kontakt'],
|
||||
'many open quotes' => [str_repeat('<a title="', intdiv($n, 10)) . ' Kontakt'],
|
||||
'many comment openers' => [str_repeat('<!--', intdiv($n, 4)) . ' Kontakt'],
|
||||
'comments then a closer' => [str_repeat('<!--', intdiv($n, 4)) . ' --> Kontakt'],
|
||||
'many markers' => [str_repeat('$variable(', intdiv($n, 10)) . ' Kontakt'],
|
||||
'many markers then a closer' => [str_repeat('$variable(', intdiv($n, 10)) . ')$ Kontakt'],
|
||||
'many entities' => [str_repeat('&', intdiv($n, 4)) . ' Kontakt'],
|
||||
'long text' => [str_repeat('Kontakt ', intdiv($n, 8))],
|
||||
];
|
||||
}
|
||||
|
||||
/** @dataProvider longInputs */
|
||||
public function testLongInputsStayLinear(string $value): void
|
||||
{
|
||||
$start = microtime(true);
|
||||
try {
|
||||
(new TextReplacer('Kontakt', 'Anfrage'))->apply(self::text($value), self::same());
|
||||
} catch (InvalidArgumentException) {
|
||||
// refused is fine
|
||||
}
|
||||
|
||||
self::assertLessThan(2.0, microtime(true) - $start);
|
||||
}
|
||||
|
||||
public function testRefusesAMarkerFromTheCallback(): void
|
||||
{
|
||||
$this->expectException(InvalidArgumentException::class);
|
||||
@@ -218,6 +346,8 @@ final class TextReplacerTest extends TestCase
|
||||
'NUL-split marker (S32a)' => ['Müller', "\$vari\0able(", 'replace'],
|
||||
'VT-split marker' => ['Müller', "\$variable\x0B(", 'replace'],
|
||||
'octet-split marker' => ['Müller', '$v%41ariable(', 'replace'],
|
||||
'NUL in search' => ["Mül\0ler", 'x', 'search'],
|
||||
'VT in search' => ["Mül\x0Bler", 'x', 'search'],
|
||||
'nested octet-split marker' => ['Müller', '$v%4%411ariable(', 'replace'],
|
||||
];
|
||||
}
|
||||
|
||||
@@ -394,6 +394,141 @@ final class DiviReplaceTextAbilityTest extends TestCase
|
||||
self::assertSame([], $dry['matches'], 'the dry run shows no hit either');
|
||||
}
|
||||
|
||||
/**
|
||||
* Ruling S45 end to end (the live stored XSS): a ">" inside a quoted attribute, comments,
|
||||
* unterminated tags and "<" in text – no stored change, error or no-op.
|
||||
*
|
||||
* @return array<string, array{string, string, string}>
|
||||
*/
|
||||
public static function s45(): array
|
||||
{
|
||||
return [
|
||||
'double quotes' => ['<p><a title="A > B" href="/kontakt">Kontakt</a></p>', ' B', '" onmouseover="alert(1)'],
|
||||
'single quotes' => ["<p><img alt='A > B' src='/b.jpg'></p>", ' B', "' onerror='alert(1)"],
|
||||
'href after a quoted ">"' => ['<p><a title="x > y" href="/kontakt">Link</a></p>', 'kontakt', 'javascript:alert(1)'],
|
||||
'comment' => ['<!-- a > B --><p>x</p>', ' B', '--><img src=x onerror=alert(1)>'],
|
||||
'unterminated tag' => ['<p>ok B</p><a title="x B', ' B', 'y'],
|
||||
'unbalanced quote' => ['<a title="x>B B</a>', ' B', 'y'],
|
||||
'lone "<" in text' => ['a < B', ' B', 'img src=x onerror=alert(1)>'],
|
||||
'"<" + letter in text' => ['a<B c', ' c', 'y'],
|
||||
];
|
||||
}
|
||||
|
||||
/** @dataProvider s45 */
|
||||
public function testS45RowsStoreNothing(string $html, string $search, string $replace): void
|
||||
{
|
||||
foreach ([false, true] as $unfiltered) {
|
||||
$this->saved = [];
|
||||
$ability = $this->ability(self::page([self::text($html)]), $unfiltered);
|
||||
|
||||
$dry = $ability->execute(['search' => $search, 'replace' => $replace]);
|
||||
$result = $this->replaceFor($ability, ['search' => $search, 'replace' => $replace]);
|
||||
|
||||
self::assertSame([], $this->saved);
|
||||
$entry = $result['results'][0];
|
||||
self::assertTrue(! $entry['ok'] || $entry['count'] === 0, (string) json_encode($entry));
|
||||
self::assertSame([], array_filter($dry['matches'], static fn (array $match): bool => $match['count'] > 0));
|
||||
}
|
||||
}
|
||||
|
||||
/** Global modules are never changed – but reported, also when they hold the only hits. */
|
||||
public function testGlobalModulesAreReportedNotChanged(): void
|
||||
{
|
||||
$global = ['divi/text', ['globalModule' => '123', 'content' => ['innerContent' => ['desktop' => ['value' => '<p>Müller</p>']]]]];
|
||||
$ability = $this->ability(self::page([$global, self::text('<p>Meier</p>')]));
|
||||
|
||||
$dry = $ability->execute(['search' => 'müller', 'replace' => 'Schmidt', 'case_sensitive' => false]);
|
||||
$result = $this->replaceFor($ability, ['search' => 'Müller', 'replace' => 'Schmidt']);
|
||||
|
||||
$node = ['post_id' => 7, 'id' => 's0.r0.c0.g0', 'reason' => 'globales Modul – in der Bibliothek bearbeiten'];
|
||||
self::assertSame([$node], $dry['skipped_nodes']);
|
||||
self::assertSame(7, $dry['matches'][0]['post_id'], 'the page does not vanish');
|
||||
self::assertSame(0, $dry['matches'][0]['count']);
|
||||
self::assertSame(1, $dry['matches'][0]['skipped_nodes']);
|
||||
self::assertStringContainsString('globale Module', $dry['summary']);
|
||||
self::assertSame([$node], $result['skipped_nodes']);
|
||||
self::assertSame(0, $result['results'][0]['count']);
|
||||
self::assertStringContainsString('Knoten s0.r0.c0.g0', implode("\n", $result['warnings']));
|
||||
self::assertSame([], $this->saved);
|
||||
}
|
||||
|
||||
/** No effective change (search == replacement, cleaned replacement == match): nothing saved. */
|
||||
public function testNoEffectiveChangeSavesNothing(): void
|
||||
{
|
||||
$ability = $this->ability(self::page([self::heading('Müller'), self::text('<p>Müller</p>')]));
|
||||
|
||||
$same = $this->replaceFor($ability, ['search' => 'Müller', 'replace' => 'Müller']);
|
||||
$caseOnly = $this->replaceFor($ability, ['search' => 'müller', 'replace' => 'Müller', 'case_sensitive' => false]);
|
||||
$dry = $ability->execute(['search' => 'Müller', 'replace' => 'Müller']);
|
||||
|
||||
self::assertSame(0, $same['results'][0]['count']);
|
||||
self::assertSame(0, $caseOnly['results'][0]['count']);
|
||||
self::assertArrayNotHasKey('revision_id', $same['results'][0]);
|
||||
self::assertSame([], $dry['matches']);
|
||||
self::assertSame([], $this->saved);
|
||||
self::assertSame([], $same['changed']);
|
||||
}
|
||||
|
||||
public function testDescriptionMentionsTrimming(): void
|
||||
{
|
||||
self::assertStringContainsString('Leerzeichen am Anfang und Ende', $this->ability()->description());
|
||||
}
|
||||
|
||||
public function testControlCharactersAndEmptySearch(): void
|
||||
{
|
||||
$ability = $this->ability();
|
||||
|
||||
foreach (['', "Ter\0min", "Ter\x0Bmin"] as $search) {
|
||||
$error = $ability->execute(['search' => $search, 'replace' => 'X']);
|
||||
self::assertSame('lit_validation_failed', $error->get_error_code(), json_encode($search));
|
||||
self::assertArrayHasKey('search', self::clientError($error)['fields']);
|
||||
}
|
||||
|
||||
$result = $this->replaceFor($ability, ['search' => 'Termin', 'replace' => "R\0\x0Bück\x7Fruf"]);
|
||||
|
||||
self::assertTrue($result['results'][0]['ok'], (string) json_encode($result));
|
||||
self::assertSame(['text' => 'Rückruf buchen', 'linkUrl' => '/kontakt', 'linkTarget' => 'off'], $this->savedValue('s0.r0.c1.m2', 'button.innerContent'));
|
||||
self::assertSame([], $this->queries);
|
||||
}
|
||||
|
||||
public function testCursorAndLimit(): void
|
||||
{
|
||||
$ability = $this->ability(null, false, [7, 8, 9]);
|
||||
|
||||
$first = $ability->execute(['search' => 'Termin', 'replace' => 'X', 'limit' => 2]);
|
||||
$second = $ability->execute(['search' => 'Termin', 'replace' => 'X', 'limit' => 2, 'cursor' => $first['next_cursor']]);
|
||||
$invalid = $ability->execute(['search' => 'Termin', 'replace' => 'X', 'cursor' => 'kaputt']);
|
||||
$capped = $ability->execute(['search' => 'Termin', 'replace' => 'X', 'limit' => 5000]);
|
||||
|
||||
self::assertCount(2, $first['matches']);
|
||||
self::assertStringContainsString('LIMIT 3 OFFSET 0', $this->queries[0]);
|
||||
self::assertStringContainsString('LIMIT 3 OFFSET 2', $this->queries[1]);
|
||||
self::assertIsString($second['next_cursor'], 'the fake query returns three rows again');
|
||||
self::assertSame('lit_validation_failed', $invalid->get_error_code());
|
||||
self::assertStringContainsString('LIMIT 101 OFFSET 0', $this->queries[2]);
|
||||
self::assertCount(3, $capped['matches']);
|
||||
}
|
||||
|
||||
/** Byte identity on a page whose other blocks use dynamic content names outside the allow map. */
|
||||
public function testByteIdentityNextToUnknownDynamicContent(): void
|
||||
{
|
||||
$odd = self::text('<p>' . '$variable({"type":"content","value":{"name":"acf_field_x","settings":{"before":"<b>"}}})$' . '</p>');
|
||||
$odd[1]['module'] = ['decoration' => ['background' => ['desktop' => ['value' => ['color' => '$variable({"type":"color","value":{"name":"gcid-nope","settings":{}}})$']]]]];
|
||||
$hit = self::heading('Willkommen bei Müller');
|
||||
$ability = $this->ability(self::page([$odd, $hit, $odd]));
|
||||
$original = (string) $this->post->post_content;
|
||||
|
||||
$result = $this->replaceFor($ability, ['search' => 'Müller', 'replace' => 'Schmidt']);
|
||||
|
||||
self::assertTrue($result['results'][0]['ok'], (string) json_encode($result));
|
||||
$changed = $hit;
|
||||
$changed[1]['title']['innerContent']['desktop']['value'] = 'Willkommen bei Schmidt';
|
||||
self::assertSame(
|
||||
str_replace(Serializer::attributes($hit[1]), Serializer::attributes($changed[1]), $original),
|
||||
$this->saved[0]
|
||||
);
|
||||
}
|
||||
|
||||
/** Ruling S10: an unexpected error on one page is logged, reported neutrally, and the batch goes on. */
|
||||
public function testAnUnexpectedErrorOnOnePageDoesNotStopTheBatch(): void
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user