From 9fca467183164a77cf0fcae6990bb81b6dc7cd40 Mon Sep 17 00:00:00 2001 From: Luca Dobrescu Date: Fri, 31 Jul 2026 16:55:44 +0300 Subject: [PATCH 1/2] fix: postContent dynamic tag reads the loop globals instead of its context post get_the_content() takes the post as its third argument; the context post ID was passed as the first one ($more_link_text), so $post stayed null and core fell back to the loop globals. When a theme or plugin leaves those globals clobbered, post-template.php warns "Undefined array key -1" (plus a preg_match() deprecation on PHP 8) and the tag renders empty. Fixes #2929 Co-Authored-By: Claude Fable 5 --- inc/plugins/class-dynamic-content.php | 4 +- .../mu-plugins/otter-e2e-bootstrap.php | 48 +++++++++++++ .../blocks/dynamic-content-frontend.spec.js | 60 ++++++++++++++++ tests/test-dynamic-content.php | 71 +++++++++++++++++++ 4 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 src/blocks/test/e2e/blocks/dynamic-content-frontend.spec.js diff --git a/inc/plugins/class-dynamic-content.php b/inc/plugins/class-dynamic-content.php index 1924d6474..b782417fd 100644 --- a/inc/plugins/class-dynamic-content.php +++ b/inc/plugins/class-dynamic-content.php @@ -545,7 +545,7 @@ public function get_content( $data ) { return ''; } - $content = get_the_content( $data['context'] ); + $content = get_the_content( null, false, $data['context'] ); $content = apply_filters( 'the_content', str_replace( ']]>', ']]>', $content ) ); return wp_kses_post( $content ); } @@ -902,7 +902,7 @@ class_exists( '\Neve_Pro\Modules\Custom_Layouts\Module' ) if ( ! $post instanceof \WP_Post ) { return $data; } - $content = get_the_content( $data['context'] ); + $content = get_the_content( null, false, $data['context'] ); if ( strpos( $content, 'data-type="postContent"' ) ) { $key = $this->get_exception_key( $data, $post->ID ); if ( $key ) { diff --git a/packages/e2e-tests/mu-plugins/otter-e2e-bootstrap.php b/packages/e2e-tests/mu-plugins/otter-e2e-bootstrap.php index 6288186ff..649de761d 100644 --- a/packages/e2e-tests/mu-plugins/otter-e2e-bootstrap.php +++ b/packages/e2e-tests/mu-plugins/otter-e2e-bootstrap.php @@ -754,6 +754,54 @@ function stub_openai_http_for_e2e( $preempt, $parsed_args, $url ) { return stub_openai_http_response( $content ); } +/** + * Issue #2929 rig: with ?otter_e2e_corrupt_pages=1, mimic a theme/plugin that + * clobbers the $pages loop global (main templates are included at global scope, + * so any template-level $pages variable overwrites it) while Otter evaluates a + * dynamic tag, and surface PHP notices in the output so the spec can assert + * none are emitted. + * + * The corruption is scoped to blocks carrying an tag and restored + * straight after Otter's filter (priority 10): leaving it in place for every + * block would make core's own the_content() warn too, which is core behavior + * rather than the bug under test. + */ +function corrupt_pages_around_dynamic_tags() { + if ( is_admin() || ! isset( $_GET['otter_e2e_corrupt_pages'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended + return; + } + + ini_set( 'display_errors', '1' ); // phpcs:ignore WordPress.PHP.IniSet.display_errors_Disallowed + + $saved = null; + + add_filter( + 'render_block', + function ( $block_content ) use ( &$saved ) { + if ( false !== strpos( $block_content, ' { + const TARGET_CONTENT = 'Otter dynamic target content 2929'; + let pageId; + + test.beforeAll( async({ requestUtils }) => { + // The Query Loop block has no include/post__in arg, so the loop is + // scoped to the target post via a unique search token in its title. + await requestUtils.createPost({ + title: 'Dynamic content target frontier2929', + content: `

${ TARGET_CONTENT }

`, + status: 'publish' + }); + + const holder = await requestUtils.createPage({ + title: 'Dynamic content holder 2929', + // The tag is wrapped in a group: postContent runs the_content, which + // wraps its output in

, and a nested

would be auto-closed by + // the browser parser and land outside the marker element. + content: ` +

+

Post Content

+
+`, + status: 'publish' + }); + + pageId = holder.id; + }); + + test( 'renders the target post content on the frontend', async({ page }) => { + await page.goto( `/?page_id=${ pageId }` ); + + await expect( page.locator( '.o-dyn-2929' ) ).toContainText( TARGET_CONTENT ); + }); + + test( 'survives a corrupted $pages loop global without PHP warnings', async({ page }) => { + await page.goto( `/?page_id=${ pageId }&otter_e2e_corrupt_pages=1` ); + + // Regression #2929: "Warning: Undefined array key -1 in .../post-template.php" + // (PHP 7.4 words it "Undefined offset: -1") plus a preg_match() deprecation. + await expect( page.locator( 'body' ) ).not.toContainText( /Undefined (array key|offset)/ ); + await expect( page.locator( 'body' ) ).not.toContainText( 'preg_match' ); + + // The tag must still resolve the context post's content. + await expect( page.locator( '.o-dyn-2929' ) ).toContainText( TARGET_CONTENT ); + }); +}); diff --git a/tests/test-dynamic-content.php b/tests/test-dynamic-content.php index 3fba9e359..a5c723b18 100644 --- a/tests/test-dynamic-content.php +++ b/tests/test-dynamic-content.php @@ -1095,4 +1095,75 @@ public function test_is_protected_meta_key_edge_cases() { $this->assertTrue( \ThemeIsle\OtterPro\Plugins\Dynamic_Content::is_protected_meta_key( 'USER_PASS' ) ); $this->assertFalse( \ThemeIsle\OtterPro\Plugins\Dynamic_Content::is_protected_meta_key( 'test_meta' ) ); } + + /** + * postContent must render the context post, not whatever post the loop + * globals happen to point at (issue #2929: the context ID was passed to + * get_the_content() as $more_link_text, so the post argument stayed null). + */ + public function test_post_content_uses_context_not_loop_globals() { + $other_id = $this->factory()->post->create( + array( + 'post_title' => 'Other', + 'post_content' => 'Other post content', + 'post_status' => 'publish', + ) + ); + + $this->go_to( get_permalink( $this->post_id ) ); + + // A secondary loop (page builder, related-posts widget) that forgot wp_reset_postdata(). + $query = new WP_Query( array( 'p' => $other_id ) ); + while ( $query->have_posts() ) { + $query->the_post(); + } + + $result = $this->dynamic_content->apply_dynamic_content( '

Post Content

' ); + + wp_reset_postdata(); + wp_delete_post( $other_id, true ); + + $this->assertStringNotContainsString( 'Other post content', $result ); + $this->assertStringContainsString( 'Test', $result ); + } + + /** + * A clobbered $pages loop global (theme templates are included at global + * scope, so any template-level $pages variable overwrites it) must not + * surface "Undefined array key -1" from post-template.php nor swallow the + * content (issue #2929). + */ + public function test_post_content_survives_corrupted_pages_global() { + $this->go_to( get_permalink( $this->post_id ) ); + + // Fire the loop so did_action( 'the_post' ) is truthy, then corrupt the global. + $query = new WP_Query( array( 'p' => $this->post_id ) ); + while ( $query->have_posts() ) { + $query->the_post(); + } + $GLOBALS['pages'] = array(); + + $captured = array(); + set_error_handler( + function ( $errno, $errstr ) use ( &$captured ) { + $captured[] = $errstr; + return true; + } + ); + + $result = $this->dynamic_content->apply_dynamic_content( '

Post Content

' ); + + restore_error_handler(); + wp_reset_postdata(); + + $page_warnings = array_filter( + $captured, + function ( $message ) { + return false !== strpos( $message, 'Undefined' ) || false !== strpos( $message, 'preg_match' ); + } + ); + + $this->assertSame( array(), array_values( $page_warnings ) ); + $this->assertStringContainsString( 'Test', $result ); + } } From fd9560c2a245af2a19a1064f081b583ba023550f Mon Sep 17 00:00:00 2001 From: Luca Dobrescu Date: Mon, 3 Aug 2026 10:23:50 +0300 Subject: [PATCH 2/2] test: cover the postContent recursion guard and isolate the e2e fixtures Addresses the Copilot review on #2939. The guard in mark_exceptions() is what the get_the_content() argument fix actually changes: it reads the context post's content to decide whether a nested postContent tag would recurse. Both mismatch cases are now covered - a nested tag in the context post must fire the guard, and a nested tag in the loop-global post must not blank a clean context post. The guard is asserted directly before rendering because an unfired guard makes apply_dynamic_content() recurse until the process dies, so the test has to fail fast instead of hanging. The e2e spec namespaced its Query Loop search token per run and now deletes its own post and page in afterAll. wp-env is persistent and the fixed token had already accumulated twelve records over six local runs, any of which the perPage:1 loop could resolve instead of the run's own post. Teardown is best-effort per record so one failed request cannot orphan the others. Co-Authored-By: Claude Opus 5 (1M context) --- .../blocks/dynamic-content-frontend.spec.js | 51 ++++++++-- tests/test-dynamic-content.php | 93 +++++++++++++++++++ 2 files changed, 135 insertions(+), 9 deletions(-) diff --git a/src/blocks/test/e2e/blocks/dynamic-content-frontend.spec.js b/src/blocks/test/e2e/blocks/dynamic-content-frontend.spec.js index a6c533a72..039a42c91 100644 --- a/src/blocks/test/e2e/blocks/dynamic-content-frontend.spec.js +++ b/src/blocks/test/e2e/blocks/dynamic-content-frontend.spec.js @@ -12,24 +12,38 @@ import { test, expect } from '@wordpress/e2e-test-utils-playwright'; * rendered empty. */ test.describe( 'Dynamic Content postContent tag', () => { - const TARGET_CONTENT = 'Otter dynamic target content 2929'; + + // wp-env is persistent, so the fixtures are namespaced per run and torn down + // in afterAll: a fixed token would let leftovers from an earlier run (or a + // retry) win the Query Loop and make the assertions state-dependent. + let token; + let targetContent; let pageId; + // Every record created by this spec, so a retried beforeAll cleans up both + // attempts instead of leaking the first one. + const created = []; + test.beforeAll( async({ requestUtils }) => { + token = `frontier2929${ Date.now() }`; + targetContent = `Otter dynamic target content ${ token }`; + // The Query Loop block has no include/post__in arg, so the loop is - // scoped to the target post via a unique search token in its title. - await requestUtils.createPost({ - title: 'Dynamic content target frontier2929', - content: `

${ TARGET_CONTENT }

`, + // scoped to the target post via the run's search token in its title. + const target = await requestUtils.createPost({ + title: `Dynamic content target ${ token }`, + content: `

${ targetContent }

`, status: 'publish' }); + created.push({ type: 'posts', id: target.id }); + const holder = await requestUtils.createPage({ - title: 'Dynamic content holder 2929', + title: `Dynamic content holder ${ token }`, // The tag is wrapped in a group: postContent runs the_content, which // wraps its output in

, and a nested

would be auto-closed by // the browser parser and land outside the marker element. - content: ` + content: `

Post Content

@@ -37,13 +51,32 @@ test.describe( 'Dynamic Content postContent tag', () => { status: 'publish' }); + created.push({ type: 'pages', id: holder.id }); + pageId = holder.id; }); + test.afterAll( async({ requestUtils }) => { + // Only this spec's own records - other specs run against the same site. + // Best-effort per record: one failed request must not orphan the rest. + while ( created.length ) { + const record = created.pop(); + try { + await requestUtils.rest({ + method: 'DELETE', + path: `/wp/v2/${ record.type }/${ record.id }`, + params: { force: true } + }); + } catch ( error ) { + console.warn( `Could not delete ${ record.type }/${ record.id }:`, error.message ); + } + } + }); + test( 'renders the target post content on the frontend', async({ page }) => { await page.goto( `/?page_id=${ pageId }` ); - await expect( page.locator( '.o-dyn-2929' ) ).toContainText( TARGET_CONTENT ); + await expect( page.locator( '.o-dyn-2929' ) ).toContainText( targetContent ); }); test( 'survives a corrupted $pages loop global without PHP warnings', async({ page }) => { @@ -55,6 +88,6 @@ test.describe( 'Dynamic Content postContent tag', () => { await expect( page.locator( 'body' ) ).not.toContainText( 'preg_match' ); // The tag must still resolve the context post's content. - await expect( page.locator( '.o-dyn-2929' ) ).toContainText( TARGET_CONTENT ); + await expect( page.locator( '.o-dyn-2929' ) ).toContainText( targetContent ); }); }); diff --git a/tests/test-dynamic-content.php b/tests/test-dynamic-content.php index a5c723b18..e9feb86ba 100644 --- a/tests/test-dynamic-content.php +++ b/tests/test-dynamic-content.php @@ -1166,4 +1166,97 @@ function ( $message ) { $this->assertSame( array(), array_values( $page_warnings ) ); $this->assertStringContainsString( 'Test', $result ); } + + /** + * The infinite-loop guard in mark_exceptions() must inspect the context + * post. When the context post nests a postContent tag the guard has to fire + * even though the loop-global post is clean (issue #2929: the guard read the + * loop global, so the nested tag went undetected and recursed). + */ + public function test_post_content_guard_detects_nested_tag_in_context_post() { + $nested_id = $this->factory()->post->create( + array( + 'post_title' => 'Nested', + 'post_content' => 'Before Post Content after', + 'post_status' => 'publish', + ) + ); + + $clean_id = $this->factory()->post->create( + array( + 'post_title' => 'Clean loop global', + 'post_content' => 'Clean loop global content', + 'post_status' => 'publish', + ) + ); + + // Context = the nested post, loop global = the clean post. + $this->go_to( get_permalink( $nested_id ) ); + + $query = new WP_Query( array( 'p' => $clean_id ) ); + while ( $query->have_posts() ) { + $query->the_post(); + } + + $data = array( + 'type' => 'postContent', + 'context' => $nested_id, + ); + $marked = $this->dynamic_content->mark_exceptions( $data ); + $guard_key = $this->dynamic_content->get_exception_key( $data, $nested_id ); + + // Asserted before rendering on purpose: an unfired guard makes + // apply_dynamic_content() recurse until the process dies, so this has to + // fail fast rather than hang. + $this->assertArrayHasKey( $guard_key, $marked ); + + $result = $this->dynamic_content->apply_dynamic_content( '

Post Content

' ); + + wp_reset_postdata(); + wp_delete_post( $nested_id, true ); + wp_delete_post( $clean_id, true ); + + // Guard fired: the tag resolves to an empty string instead of recursing. + $this->assertSame( '

', $result ); + $this->assertStringNotContainsString( 'Clean loop global content', $result ); + } + + /** + * The mirror case: a nested postContent tag in the loop-global post must not + * blank out a clean context post (issue #2929). + */ + public function test_post_content_guard_ignores_nested_tag_in_loop_global() { + $nested_id = $this->factory()->post->create( + array( + 'post_title' => 'Nested loop global', + 'post_content' => 'Before Post Content after', + 'post_status' => 'publish', + ) + ); + + // Context = the clean post from set_up(), loop global = the nested post. + $this->go_to( get_permalink( $this->post_id ) ); + + $query = new WP_Query( array( 'p' => $nested_id ) ); + while ( $query->have_posts() ) { + $query->the_post(); + } + + $data = array( + 'type' => 'postContent', + 'context' => $this->post_id, + ); + $marked = $this->dynamic_content->mark_exceptions( $data ); + $guard_key = $this->dynamic_content->get_exception_key( $data, $this->post_id ); + + $this->assertArrayNotHasKey( $guard_key, $marked ); + + $result = $this->dynamic_content->apply_dynamic_content( '

Post Content

' ); + + wp_reset_postdata(); + wp_delete_post( $nested_id, true ); + + // Guard did not fire: the context post's own content is still rendered. + $this->assertStringContainsString( 'Test', $result ); + } }