hook_suffix is undefined in WP_Screen class

[splendorstudio]

Hi There,

There is an bug in WP_Screen class in 213 line which we are getting via $id = $GLOBALS['hook_suffix']; hook_suffix global, You can see this issue via some functions but,
I encountered with this issue via convert_to_screen() in ajax situation. Please let me know if there is any question.

[SergeyBiryukov]

Related: #22039, #29933.

Trac ticket: https://core.trac.wordpress.org/ticket/49089

[htdat]

When working on plugin Edit Flow, we came across this error ​https://github.com/Automattic/Edit-Flow/issues/654

Here is what we've got so far:

To replicate this issue with core

• Add this file under wp-content/mu-plugins or wp-content/plugins ​https://gist.github.com/htdat/06d77ea326d53cb1ae1e6b68c96aa0c3
• Visit wp-admin/edit.php
• Try to edit a post with "Quick Edit" to trigger the ajax action
• Check PHP error log (debug.log)

Error with track trace:

[12-Jul-2021 04:28:47 UTC] PHP Notice: Undefined index: hook_suffix in /var/www/html/wp-admin/includes/class-wp-screen.php on line 223
[12-Jul-2021 04:28:47 UTC] PHP Stack trace:
[12-Jul-2021 04:28:47 UTC] PHP 1. {main}() /var/www/html/wp-admin/admin-ajax.php:0
[12-Jul-2021 04:28:47 UTC] PHP 2. do_action() /var/www/html/wp-admin/admin-ajax.php:187
[12-Jul-2021 04:28:47 UTC] PHP 3. WP_Hook->do_action() /var/www/html/wp-includes/plugin.php:484
[12-Jul-2021 04:28:47 UTC] PHP 4. WP_Hook->apply_filters() /var/www/html/wp-includes/class-wp-hook.php:316
[12-Jul-2021 04:28:47 UTC] PHP 5. {closure:/var/www/html/wp-content/mu-plugins/trac-49089.php:11-24}() /var/www/html/wp-includes/class-wp-hook.php:292
[12-Jul-2021 04:28:47 UTC] PHP 6. WP_List_Table->construct() /var/www/html/wp-content/mu-plugins/trac-49089.php:14
[12-Jul-2021 04:28:47 UTC] PHP 7. convert_to_screen() /var/www/html/wp-admin/includes/class-wp-list-table.php:149
[12-Jul-2021 04:28:47 UTC] PHP 8. WP_Screen::get() /var/www/html/wp-admin/includes/template.php:2571

Root cause

The error happens around this line ​http://github.com/WordPress/WordPress/blob/9f91305af2bd18c914096cc5e5cc1d6882163200/wp-admin/includes/class-wp-screen.php#L223-L223

                        $id = $GLOBALS['hook_suffix'];

admin-ajax.php requests do not load file wp-admin/admin.php, which is loaded when admins browse wp-admin pages.
Then global $hook_suffix is not added and defined, hence, the PHP error Undefined index: hook_suffix happens.

A side note here: I searched hook_suffix and $hook_suffix across WordPress core and found out that this global $hook_suffix var is set up in file wp-admin/admin.php:

• ​http://github.com/WordPress/WordPress/blob/9f91305af2bd18c914096cc5e5cc1d6882163200/wp-admin/admin.php#L203-L210

Solution

I think the ultimate solution here is to change the way we handle $GLOBALS['hook_suffix'] and property id of class WP_List_Table.

That's why I proposed assigning a value for property id with the wp_ajax_ prefix for this case in PR: ​https://github.com/WordPress/wordpress-develop/pull/1490

[htdat]

Another note here (thanks to @mikeyarce): This notice Undefined index will be converted into a warning in PHP8 and that might cause a bigger headache:

Ref: ​https://www.php.net/manual/en/migration80.incompatible.php:

A number of notices have been converted into warnings:
...
Attempting to read an undefined array key.

[mukesh27]

@htdat I face some what similar error in PHP 8.1. Here is more information: ​https://github.com/WordPress/wordpress-develop/pull/3152#issuecomment-1246552921

Avoids a E_NOTICE: Undefined index: hook_suffix in wp-admin/includes/class-wp-screen.php:224 notice.

Trac ticket: https://core.trac.wordpress.org/ticket/49089

[dd32]

Just noting that this can also happen when using WP_List_Table through admin-post.php, not just admin-ajax.php.

One solution is for List Tables to pass the screen property upstream to WP_List_Table so it doesn't have to guess what the screen is.

class Example_List_Table extends WP_List_Table {

	public function __construct( $details ) {
		$this->details = $details;

		parent::__construct(
			array(
				'screen' => 'toplevel_page_example' . $this->details['slug']
			)
		);
	}

Another is to simply only use $hook_suffix when it's known and valid. As in ​PR #3415.

[dd32]

#29933 was marked as a duplicate.

[dd32]

#40571 was marked as a duplicate.

[dd32]

#56052 was marked as a duplicate.

@dd32 thanks for the PR. We have similar issue in https://core.trac.wordpress.org/ticket/16502 we end up with this error. ​https://github.com/WordPress/wordpress-develop/pull/3152#issuecomment-1246552921.

if ( $hook_name ) {
    $id = $hook_name;
} elseif ( wp_doing_ajax() && isset( $_REQUEST['action'] ) ) {
    $id = 'wp_ajax_' . esc_attr( $_REQUEST['action'] );
} elseif ( isset( $GLOBALS['hook_suffix'] ) ) {
    $id = $GLOBALS['hook_suffix'];
} else {
    $id = ''; // Or just let it fail here.
}

[costdev]

As this patch also unblocks a cleaner solution for #16502 (which I'd like to try to get back into 6.1 if possible), I'm milestoning this for 6.1 so that Core Tech leads can take a look.

Pinging @mike, @davidbaumwald, @jeffpaul for your thoughts. The patch for this ticket is relatively minor.

Additional props to @mukesh27 for putting this on my radar.

[davidbaumwald]

In 54414:

Administration: Guard against undefined $GLOBALS['hook_suffix'] in WP_Screen::get().

When initially defaulting the screen $id in WP_Screen::get(), if the $hook_name parameter is not supplied, an else fallback uses $GLOBALS['hook_suffix']. However, in some cases, hook_suffix doesn't exist in the global scope. This produces an "Undefined index" notice on < PHP 8, and a warning in >= PHP 8.

This change ensures $GLOBALS['hook_suffix'] has a value before using it as a fallback for the screen ID.

Props splendorstudio, SergeyBiryukov, htdat, mukesh27, dd32, costdev.
Fixes #49089.

Merged into core in https://core.trac.wordpress.org/changeset/54414.

Thanks for the PR @htdat! This was merged into core for the upcoming version 6.1 in https://core.trac.wordpress.org/changeset/54414.