fix: bound render-summary strings, restore postdata globals, drain buffers

Review fix round 1 for lit/page-render-summary: extract shared Text::shorten() (reused by NodeSummary), replace data: URIs with a short marker, prefer data-src/data-lazy-src over placeholder src, dedup nested p/li/blockquote text and ignore script/style/noscript content, restore every setup_postdata global (not just $post), drain output buffers robustly around the_content, and disclose that rendering can trigger oEmbed/network requests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7
2026-09-26 12:31:20 +02:00
parent 2e9acd686a
commit b3cd0eee62
7 changed files with 345 additions and 47 deletions
@@ -34,8 +34,10 @@ final class PageRenderSummaryAbility implements Ability
    {
        return 'Rendert den Inhaltsbereich eines Eintrags wie im Frontend und liefert einen strukturierten Auszug: '
            . 'Überschriften, Texte, Links, Bilder mit Alt-Text und Warnungen (Hierarchie-Sprünge, fehlende '
-            . 'Alt-Texte). Header, Footer und Theme-Builder-Vorlagen sind nicht enthalten. Gut zum Prüfen nach '
-            . 'Änderungen.';
+            . 'Alt-Texte). Header, Footer und Theme-Builder-Vorlagen sind nicht enthalten. Das Rendern führt die '
+            . 'üblichen Inhalts-Filter aus (Shortcodes, Divi, andere Plugins) und kann dabei Netzwerkanfragen '
+            . 'auslösen, z. B. oEmbed-Abrufe, die von WordPress oder anderen Plugins zwischengespeichert werden. '
+            . 'Gut zum Prüfen nach Änderungen.';
    }

    public function inputSchema(): array
@@ -90,7 +92,11 @@ final class PageRenderSummaryAbility implements Ability
            return $post;
        }

-        $notes = ['Nur der Inhaltsbereich; Header, Footer und Theme-Builder-Vorlagen fehlen.'];
+        $notes = [
+            'Nur der Inhaltsbereich; Header, Footer und Theme-Builder-Vorlagen fehlen.',
+            'Das Rendern führt Plugin-/Theme-Filter aus (Shortcodes, oEmbed u. Ä.) und kann dabei '
+                . 'Netzwerkanfragen auslösen, die von WordPress oder anderen Plugins zwischengespeichert werden.',
+        ];
        if ((string) $post->post_password !== '') {
            $notes[] = 'Der Eintrag ist passwortgeschützt; Besucher sehen zuerst das Passwortformular.';
        }
@@ -9,28 +9,60 @@ use WP_Post;
 /**
 * Renders the content area of an entry in-process, like the front end does for the_content
 * (Divi 5 renders its blocks there – verified in DDEV). Header, footer and Theme Builder
- * layouts are not part of it. Not final so abilities can be unit-tested with a mock.
+ * layouts are not part of it. Rendering runs every plugin's/theme's content filters and
+ * shortcodes exactly as the front end would, which can trigger network requests (e.g. oEmbed
+ * lookups) that get cached by WordPress or other plugins as a side effect. Not final so
+ * abilities can be unit-tested with a mock.
 */
 class PageRenderer
 {
+    /**
+     * Globals setup_postdata() populates from the post loop (WP_Query::the_post()); all of
+     * them are restored to their pre-render value afterwards, not just $post itself.
+     */
+    private const POSTDATA_GLOBALS = ['id', 'authordata', 'pages', 'page', 'more', 'numpages', 'multipage'];
+
    public function render(WP_Post $post): string
    {
-        $previous = $GLOBALS['post'] ?? null;
+        $previousPost = $GLOBALS['post'] ?? null;
+        $previousGlobals = [];
+        foreach (self::POSTDATA_GLOBALS as $name) {
+            $previousGlobals[$name] = $GLOBALS[$name] ?? null;
+        }
+
        $GLOBALS['post'] = $post;
-        setup_postdata($post);
+        $level = ob_get_level();
        ob_start();
        try {
+            setup_postdata($post);
            $html = (string) apply_filters('the_content', (string) $post->post_content);
        } finally {
-            $echoed = (string) ob_get_clean();
-            $GLOBALS['post'] = $previous;
-            if ($previous instanceof WP_Post) {
-                setup_postdata($previous);
-            } else {
-                wp_reset_postdata();
+            $echoed = self::drain($level);
+            $GLOBALS['post'] = $previousPost;
+            foreach ($previousGlobals as $name => $value) {
+                $GLOBALS[$name] = $value;
            }
        }

        return $echoed . $html;
    }
+
+    /**
+     * Pops every output buffer opened while rendering, including one a rogue shortcode/filter
+     * left open (drained here instead of leaking into the caller) – but never one it already
+     * closed for us: the loop only pops down to the level recorded before our own ob_start(),
+     * so a filter that closes one buffer too many simply loses whatever was already echoed
+     * (nothing left to drain) rather than us popping a buffer that belongs to the caller.
+     * Buffers are combined in the order their content was written (innermost popped first,
+     * then prepended) so a leaked nested buffer's output stays after what came before it.
+     */
+    private static function drain(int $level): string
+    {
+        $output = '';
+        while (ob_get_level() > $level) {
+            $output = (string) ob_get_clean() . $output;
+        }
+
+        return $output;
+    }
 }
@@ -6,15 +6,34 @@ namespace LIT\MCP\Integrations\WordPress\Render;

 use DOMDocument;
 use DOMElement;
+use DOMNode;
 use DOMXPath;
+use LIT\MCP\Abilities\Text;

 /**
 * Structured excerpt of rendered HTML (spec §5.7 lit/page-render-summary): headings, text
- * blocks, links, images with alt text and the obvious problems. Pure PHP (ext-dom).
+ * blocks, links, images with alt text and the obvious problems. Every extracted string is
+ * bounded in length through the shared Text::shorten() helper (also used by
+ * Divi\BlockTree\NodeSummary) – a rendered page can otherwise embed arbitrarily large text,
+ * a huge inline data: URI, or a long tracking URL, none of which belong in an MCP response.
+ * Pure PHP (ext-dom).
 */
 final class RenderSummary
 {
    private const TEXT_LIMIT = 300;
+    private const HEADING_LIMIT = 200;
+    private const LINK_TEXT_LIMIT = 150;
+    private const ALT_LIMIT = 200;
+    private const URL_LIMIT = 300;
+
+    private const HEADING_TAGS = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'];
+    private const TEXT_TAGS = ['p', 'li', 'blockquote'];
+
+    /** Never real page content: script/style source and noscript fallback markup. */
+    private const IGNORED_ANCESTORS = ['script', 'style', 'noscript'];
+
+    /** Attributes lazy-load libraries put the real image URL in; `src` is then a placeholder. */
+    private const LAZY_SRC_ATTRIBUTES = ['data-src', 'data-lazy-src'];

    /**
     * @return array{headings: list<array{level: int, text: string}>, texts: list<string>,
@@ -34,32 +53,48 @@ final class RenderSummary
        $xpath = new DOMXPath($dom);

        $headings = [];
-        foreach (self::elements($xpath, '//h1|//h2|//h3|//h4|//h5|//h6', $max) as $element) {
-            $headings[] = ['level' => (int) substr($element->nodeName, 1), 'text' => self::text($element)];
-        }
-
-        $texts = [];
-        foreach (self::elements($xpath, '//p|//li|//blockquote', $max) as $element) {
+        $query = self::xpathFor(self::HEADING_TAGS, self::IGNORED_ANCESTORS);
+        foreach (self::elements($xpath, $query) as $element) {
            $text = self::text($element);
            if ($text !== '') {
-                $texts[] = mb_strlen($text) > self::TEXT_LIMIT
-                    ? mb_substr($text, 0, self::TEXT_LIMIT - 1) . '…'
-                    : $text;
+                $headings[] = [
+                    'level' => (int) substr($element->nodeName, 1),
+                    'text' => Text::shorten($text, self::HEADING_LIMIT),
+                ];
            }
        }
+        $headings = array_slice($headings, 0, $max);
+
+        // p/li/blockquote nested inside another one of these (a <p> inside a <li>, a nested
+        // list item, …) are excluded, otherwise their text would be extracted twice: once as
+        // part of the outer element's textContent, once again on its own.
+        $texts = [];
+        $query = self::xpathFor(self::TEXT_TAGS, array_merge(self::IGNORED_ANCESTORS, self::TEXT_TAGS));
+        foreach (self::elements($xpath, $query) as $element) {
+            $text = self::text($element);
+            if ($text !== '') {
+                $texts[] = Text::shorten($text, self::TEXT_LIMIT);
+            }
+        }
+        $texts = array_slice($texts, 0, $max);

        $links = [];
-        foreach (self::elements($xpath, '//a[@href]', $max) as $element) {
-            $links[] = ['text' => self::text($element), 'href' => $element->getAttribute('href')];
-        }
-
-        $images = [];
-        foreach (self::elements($xpath, '//img', $max) as $element) {
-            $images[] = [
-                'src' => $element->getAttribute('src'),
-                'alt' => $element->hasAttribute('alt') ? $element->getAttribute('alt') : null,
+        $query = self::xpathFor(['a[@href]'], self::IGNORED_ANCESTORS);
+        foreach (self::elements($xpath, $query) as $element) {
+            $links[] = [
+                'text' => Text::shorten(self::text($element), self::LINK_TEXT_LIMIT),
+                'href' => self::shortenUrl($element->getAttribute('href')),
            ];
        }
+        $links = array_slice($links, 0, $max);
+
+        $images = [];
+        $query = self::xpathFor(['img'], self::IGNORED_ANCESTORS);
+        foreach (self::elements($xpath, $query) as $element) {
+            $alt = $element->hasAttribute('alt') ? Text::shorten($element->getAttribute('alt'), self::ALT_LIMIT) : null;
+            $images[] = ['src' => self::shortenUrl(self::imageSrc($element)), 'alt' => $alt];
+        }
+        $images = array_slice($images, 0, $max);

        return [
            'headings' => $headings,
@@ -112,13 +147,25 @@ final class RenderSummary
        return $warnings;
    }

+    /** Builds an XPath union of $tags, each excluding descendants of $excludeAncestors. */
+    private static function xpathFor(array $tags, array $excludeAncestors): string
+    {
+        $conditions = array_map(
+            static fn (string $tag): string => "not(ancestor::{$tag})",
+            $excludeAncestors
+        );
+        $guard = $conditions === [] ? '' : '[' . implode(' and ', $conditions) . ']';
+
+        return implode(' | ', array_map(static fn (string $tag): string => "//{$tag}{$guard}", $tags));
+    }
+
    /** @return list<DOMElement> */
-    private static function elements(DOMXPath $xpath, string $query, int $max): array
+    private static function elements(DOMXPath $xpath, string $query): array
    {
        $nodes = $xpath->query($query);
        $elements = [];
        foreach ($nodes === false ? [] : $nodes as $node) {
-            if ($node instanceof DOMElement && count($elements) < $max) {
+            if ($node instanceof DOMElement) {
                $elements[] = $node;
            }
        }
@@ -126,8 +173,63 @@ final class RenderSummary
        return $elements;
    }

+    /**
+     * Real lazy-loaded images keep the true URL in data-src/data-lazy-src and only put a
+     * placeholder (often a tiny data: URI) in src – prefer the real URL when present.
+     */
+    private static function imageSrc(DOMElement $element): string
+    {
+        foreach (self::LAZY_SRC_ATTRIBUTES as $attribute) {
+            if ($element->hasAttribute($attribute) && trim($element->getAttribute($attribute)) !== '') {
+                return $element->getAttribute($attribute);
+            }
+        }
+
+        return $element->getAttribute('src');
+    }
+
+    /** A data: URI is replaced by a short marker instead of being truncated mid-payload. */
+    private static function shortenUrl(string $url): string
+    {
+        if (stripos($url, 'data:') === 0) {
+            return self::dataUriMarker($url);
+        }
+
+        return Text::shorten($url, self::URL_LIMIT);
+    }
+
+    private static function dataUriMarker(string $url): string
+    {
+        $mime = 'unknown';
+        if (preg_match('/^data:([^;,]*)/i', $url, $match) === 1 && $match[1] !== '') {
+            $mime = $match[1];
+        }
+
+        return sprintf('data:%s;… (%d Bytes)', $mime, strlen($url));
+    }
+
+    /** Text of the element, ignoring script/style/noscript content, whitespace-normalized. */
    private static function text(DOMElement $element): string
    {
-        return trim((string) preg_replace('/\s+/u', ' ', $element->textContent));
+        return trim((string) preg_replace('/\s+/u', ' ', self::plainText($element)));
+    }
+
+    private static function plainText(DOMNode $node): string
+    {
+        $text = '';
+        foreach ($node->childNodes as $child) {
+            if ($child->nodeType === XML_TEXT_NODE || $child->nodeType === XML_CDATA_SECTION_NODE) {
+                $text .= $child->nodeValue;
+                continue;
+            }
+
+            if ($child instanceof DOMElement && in_array(strtolower($child->nodeName), self::IGNORED_ANCESTORS, true)) {
+                continue;
+            }
+
+            $text .= self::plainText($child);
+        }
+
+        return $text;
    }
 }