johnbillion / query-monitor

The developer tools panel for WordPress
https://querymonitor.com
GNU General Public License v2.0
1.58k stars 207 forks source link

Query Monitor triggers infinite loop via `locale` filter when handling `_doing_it_wrong` notices #840

Closed rebeccahum closed 3 months ago

rebeccahum commented 9 months ago

Introduced by the new QM_Collector_Doing_It_Wrong class -- Query Monitor is inadvertently triggering an infinite loop when it attempts to handle _doing_it_wrong notices generated by a premature call (before main WP Query) to is_attachment() when filtering locale.

To reproduce:

function test_locale_filter( $locale ) {
    $meta_locale = false;

    if ( ! is_attachment() ) {
        $id = get_the_ID();

        if ( $id ) {
            $meta_locale = get_post_meta( $id, 'l10n', true );
        }
    }

    if ( $meta_locale ) {
        return $meta_locale;
    }

    return $locale;
}

add_filter( 'locale', 'test_locale_filter', 20, 1 );

You'll see that it OOMs via:

PHP message: Fatal error: Allowed memory size of 536870912 bytes exhausted (tried to allocate 20480 bytes) in /wp/wp-content/mu-plugins/query-monitor/classes/Backtrace.php on line 137

Commenting out https://github.com/johnbillion/query-monitor/blob/0741b15ea0bc05dc9b6fd71af246cf83cbc45f33/collectors/doing_it_wrong.php#L114 fixes the issue.

johnbillion commented 9 months ago

Thanks for the report Rebecca

rebeccahum commented 6 months ago

Hey @johnbillion, just checking if you've had a chance to look into this!

rebeccahum commented 3 months ago

What are your thoughts on maybe adding in action_doing_it_wrong_run() to prevent recursion?

        if ( doing_action( 'doing_it_wrong_run' ) ) {
            return;
        }
johnbillion commented 3 months ago

Thanks for the reminder, coincidentally I've just been discussing this in https://github.com/WordPress/wordpress-develop/pull/6462 . I have a solution in mind which will avoid the recursion inside the collectors for this functionality.