Critical error if page_for_posts is set to non existing page ID

[wplindavantol]

Description
If page_for_posts points to a non-existing page ID, get_posts() will fail and cause a critical error in line 406 (nav-menu.php). So probably a check is needed to prevent this. Tested with default theme and no active plugins.

Critical error location
Dashboard --> Appearance --> Menus

Error log

PHP Fatal error:  Uncaught Error: Attempt to assign property "posts_page" on null in /.../wp-admin/includes/nav-menu.php:406
Stack trace:
#0 /.../wp-admin/includes/template.php(1577): wp_nav_menu_item_post_type_meta_box()
#1 /.../wp-admin/nav-menus.php(984): do_accordion_sections()
#2 {main}
   thrown in /.../wp-admin/includes/nav-menu.php on line 406, referer: https://.../wp-admin/nav-menus.php? 
   action=locations

nav-menu.php

<?php
  // Insert Posts Page.
     $posts_page = 'page' === get_option( 'show_on_front' ) ? (int) get_option('page_for_posts' ) : 0;

     if ( ! empty( $posts_page ) ) {
           $posts_page_obj             = get_post( $posts_page );
           $posts_page_obj->posts_page = true;

           $important_pages[]   = $posts_page_obj;
           $suppress_page_ids[] = $posts_page_obj->ID;
     }

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

[hbhalodia]

Hi @SergeyBiryukov, As per @wplindavantol description, I tried to reproduce the same error on my system. I got to reproduce the same error. What I followed is to provide the non-existing ID for this part of code logic and successfully encountered the error. Though I have added the patch for this ticket, I am bit confuse here,

• I have tried to reproduce the error by setting the homepage and posts page in settings→reading and then deleted the page which was assigned to posts page. When I revisited the menu, it defaults taken the next page ID but does not add the error.
• I think when we intentionally add the non-exisiting ID, then only the error is reproducible.

@wplindavantol Can you please provide the steps how you have reporoduced the error without intenionally passing the non-existing Id. Thanks.

Ps: Though I have added the patch, here is the PR for the same - ​https://github.com/WordPress/wordpress-develop/pull/4472

[wplindavantol]

@hbhalodia The problem existed before I was asked to look into it. I tried to reproduce it with a possible scenario, but I couldn't reproduce it.

I have created a new user. With this user I created a page, which I set as a 'blog' page. Then I deleted this user, without linking the content to another user. But that didn't cause this problem.

[hbhalodia]

Yes @wplindavantol, By default I was also not able to reproduce the issue without explicitly adding the non-existing ID. When I add any ID which I know that is not exists, it's reproducing that way only.

[josklever]

When I investigated this issue together with @wplindavantol we've found ​other occurrences of this issue.

So it's probably a good idea to check if the post exists and if not set the value to 0, like:

if ( is_null($posts_page_obj) ) { update_option('page_for_posts', 0); }

Disclaimer: I'm not a developer, so this needs to be checked/tested.

[audrasjb]

Thanks for the patch @hbhalodia however there's a logic issue in the PR: if $front_page_obj or $posts_page_obj is null we should fallback to the else statement.

[hbhalodia]

@audrasjb,

Got it, $front_page_obj = get_post( $front_page );, this check needs to be checked in the first if statement. If any of the condition fails should be fallbacking to the else statement. Something like below,

if ( ! empty( $front_page ) ) {
    $front_page_obj = get_post( $front_page );
}

if ( $front_page_obj ) {
    $front_page_obj->front_or_home = true;

    $important_pages[]   = $front_page_obj;
    $suppress_page_ids[] = $front_page_obj->ID;
} else {
 ....
}

And for the $posts_page_obj there was no default else statement previously. Should we need to add the same as we added for $front_page_obj.

Let me know so that I can edit the patch correctly.

Thanks.

[audrasjb]

Yeah this is exactly what I meant 👍

[hbhalodia]

Thanks, @audrasjb Would updated the patch accordingly. I am adding the patch file here and if looks good, would update the PR too.

[hbhalodia]

Patch for ticket - 58345

[oglekler]

New patch needs testing.

[azaozz]

patch-58345-2.patch makes sense and seems to work here. As @hbhalodia I'm also a bit surprised about how these two options can point to non-existing posts, but seems it is not impossible :)

[azaozz]

In 56236:

Menus: Fix critical errors when the page_on_front and/or page_for_posts options contain references to non-existing posts.

props: hbhalodia, wplindavantol, josklever, audrasjb.
Fixes: #58345.