wp_setup_nav_menu_item() throws PHP warning when using virtual menu-items

[apedog]

I believe this bug was introduced in 47211

TLDR

wp_setup_nav_menu_item() calls get_post() but does no sanity checking before calling get_post_states() with what might be a null. Adding a simple if ( $menu_post !== null ) before calling get_post_states() resolves this issue.

I've added a patch.

Details

I have an old plugin (pre-dating above commit) that renders custom dynamically-generated drop-down menus (eg. recent posts, yearly archives).
The plugin is somewhat based on ​https://www.ibenic.com/understanding-wordpress-menu-items/ but is a bit more involved. It's a stable and simple plugin.

The way the plugin works is it adds a single menu-item meta-box in the Menu editor page. However on the front it dynamically generates submenu items that don't show on the Edit Menu page. On the front the plugin creates virtual/fake WP_Post objects at runtime that are fed into Walker_Nav_Menu_Checklist. Therefore these objects have no corresponding $menu_post in the database.

Since WordPress 5.4, core throws multiple Attempt to read property "post_status|ID" on null notices on every page load. These notices have been upgraded to warnings in PHP 8.

wp-admin/includes/template.php:2288
get_post_states()
wp-includes/nav-menu.php:839
wp_setup_nav_menu_item()
wp-includes/nav-menu.php:839
array_map()
wp-includes/nav-menu.php:727
wp_get_nav_menu_items()
wp-admin/nav-menus.php:596

[apedog]

Perhaps a similar sanity check could be added to get_post_states() if parameter $post is null.

This refreshes https://core.trac.wordpress.org/attachment/ticket/58932/changeset-58932.diff submitted by ​apedog.

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

[joemcgill]

Thanks @apedog. This change makes sense to me since get_post() can return null on failure. I've refreshed the diff in this PR so we can make sure tests are passing. It would be great to add an additional unit test that would demonstrate the bug.

https://core.trac.wordpress.org/ticket/58932

The function get_post_states() requires a WP_Post object as an arg. But get_post() can return null - so we need multiple layers of defense here.

First, get_post_states() needs to validate that it has a WP_Post object before trying to treat $post as an object.

Second, the call to get_post_states() in wp-includes/nav-menu.php needs to be conditional on get_post() returning a WP_Post object.

This also includes a new test for get_post_states() to make sure it does the right thing when given something other than a WP_Post object.

[josephscott]

I've got a PR at ​https://github.com/WordPress/wordpress-develop/pull/10706 that addresses this.

• Adds a test for get_post_states() that checks to see if it does the right thing when given something other than a WP_Post object
• Adds a check to get_post_states() that makes sure $post is an object before using it as one, in cases where it gets something else it now returns an empty array
• Defense in depth, add a return value check from get_post() in wp-includes/nav-menu.php so that we only call get_post_states() when we have a WP_Post object

There are other existing calls to get_post_states(), but they appear to safe because of other checks that happen before that point.

This provides more defensive layers for existing and future code that makes use of get_post_states().

[josephscott]

New diff from ​https://github.com/WordPress/wordpress-develop/pull/10706

Sorry, there's a merge conflict now due to r61463.

[westonruter]

In 61465:

Menus: Ensure a WP_Post instance gets passed to get_post_states() in wp_setup_nav_menu_item().

The get_post_states() function is also hardened to short-circuit in case a non-WP_Post is passed. A test is added to verify this.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/10706

Follow-up to [47211].

Props apedog, josephscott, joemcgill, westonruter.
See #49374.
Fixes #58932.