WordPress.org

Make WordPress Core

Opened 8 weeks ago

Last modified 3 weeks ago

#53824 new defect (bug)

Fatal error when in action pre_get_posts call *_template()

Reported by: Adiasz Owned by:
Milestone: 5.8.2 Priority: normal
Severity: normal Version: 5.8
Component: Query Keywords: has-patch
Focuses: Cc:

Description

code:

add_action('pre_get_posts', function($query) {
    // when is on page
    get_page_template();
});

breaks WP core in
"Allowed memory size of 134217728 bytes exhausted" in wp-includes\class-wp-query.php

steps:

  1. Add upper action to functions.php
  2. Enter to any WP page
  3. Fatal ERROR

It breaks WP in each function:
get_single_template()
get_category_template()
get_post_type_archive_template()
get_taxonomy_template()
get_archive_template()
get_home_template()

when function is fired in 'pre_get_posts' hook and this is current WP page.

Attachments (2)

53824.diff (546 bytes) - added by kapilpaul 8 weeks ago.
Created patch.
53824.2.diff (487 bytes) - added by kapilpaul 7 weeks ago.
using did_action to check.

Download all attachments as: .zip

Change History (10)

#1 @SergeyBiryukov
8 weeks ago

  • Component changed from General to Query
  • Milestone changed from Awaiting Review to 5.8.1

Hi there, welcome to WordPress Trac! Thanks for the report.

I was able to reproduce the issue. It appears to be introduced in [51003]:

  • The code in question calls get_page_template() on the pre_get_posts action.
  • get_page_template() calls get_query_template().
  • get_query_template() calls locate_block_template().
  • locate_block_template() calls resolve_block_template().
  • resolve_block_template() calls get_block_templates().
  • get_block_templates() creates a new WP_Query() instance.
  • WP_Query::get_posts() runs the pre_get_posts action again, leading to endless recursion and a fatal error.

Moving to 5.8.1 to see if we can figure out a solution.

As a workaround, you should be able to check whether this is the main query before calling get_page_template(). This seems to work as expected in my testing:

add_action( 'pre_get_posts', function( $query ) {
	if ( $query->is_main_query() ) {
		echo get_page_template();
	}
});

@kapilpaul
8 weeks ago

Created patch.

This ticket was mentioned in PR #1530 on WordPress/wordpress-develop by kapilpaul.


8 weeks ago

  • Keywords has-patch added

This PR will fix fatal error when in action pre get posts call *_template()

Tested with below templates-
get_single_template()
get_category_template()
get_post_type_archive_template()
get_taxonomy_template()
get_archive_template()
get_home_template()

Trac Ticket: https://core.trac.wordpress.org/ticket/53824

#3 @kapilpaul
8 weeks ago

@SergeyBiryukov I am also able to reproduce this issue. I have tried to solve this (patch) by using did_action.

#4 @peterwilsoncc
7 weeks ago

I've added a brief review on the pull request but the PR bot didn't pick it up here for some reason.

There may be a few solutions available for this bug, it's just a matter of finding the most appropriate.

@kapilpaul
7 weeks ago

using did_action to check.

#5 @kapilpaul
7 weeks ago

@peterwilsoncc Thanks for explaining this. I have updated the PR and attached a new patch in here. Hope it helps.

#6 @prbot
7 weeks ago

johnbillion commented on PR #1530:

If this goes in, the docblock needs to move inside the condition too.

#7 @peterwilsoncc
7 weeks ago

Adding some thoughts I've had overnight:

  • Within get_query_template(), locate_block_template() should probably check for theme support. If nothing else, it saves on a database query.
  • Half thought: A part of me is wondering if pre_get_posts should be running for templates, it seems a different purpose so might be better name pre_get_templates (backcompat break)
  • I'm a little nervous about not always running pre_get_posts as this is where plugins remove potentially damaging data (EDD for example)

#8 @desrosj
3 weeks ago

  • Milestone changed from 5.8.1 to 5.8.2

I'd like for this to get a bit more discussion and testing in trunk before backporting and including in a minor release.

Since the most recent feedback has not been discussed further and 5.8.1 RC is due out in less than 24 hours, I'm going to punt this to 5.8.2. If any committers disagree and are willing & able to shepherd this in before then, they're more than welcome!

Note: See TracTickets for help on using tickets.