Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#35013 closed defect (bug) (fixed)

WP4.4 function handle_404 yelds a fatal error on line 613 when trying to clone $wp_query->post if it's not an object

Reported by: jdmweb's profile jdmweb Owned by: swissspidy's profile swissspidy
Milestone: 4.4.1 Priority: normal
Severity: normal Version: 4.4
Component: Bootstrap/Load Keywords: has-patch commit fixed-major
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Hello.

Here's the problem:
Fatal error: __clone method called on non-object in /var/www/html/lesmenuires/wp-includes/class-wp.php on line 612

So I went on having a look at what this function does, and this particular line, here's what I have:
$p = clone $wp_query->post;

I've just updated a multi site install to WordPress 4.4, and that is a core WordPress file so you should have the same code than me when looking at this file on the WP4.4 branch.

I must be in the context where $wp_query->post hasn't been defined yet, which therefore triggers the error.

I've tried a little fix like so:
$p = is_object($wp_query->post) ? clone $wp_query->post : null;
And that fixed my issue. But as this is a WordPress core file, I can't leave it like this that's why I'm reporting this to you today.

To give you a bit more context if that helps:

I've been running this multi site install for several years now, I've followed nearly every version upgrade and never had any issue before this upgrade.

An element that might be important: I'm the context of a file in my theme with no active plugins (it's a special API). This file is called with an absolute path, and includes wp-blog-header.php to activate WordPress.

Here's the complete stack trace with comments:

Fatal error: __clone method called on non-object in /var/www/html/lesmenuires/wp-includes/class-wp.php on line 612
Call Stack
#	Time	Memory	Function	Location
1	0.0030	797888	{main}( )	../noe-api.php:0 //This is the inclusion of my special file
2	0.0045	805128	require_once( '/var/www/html/lesmenuires/wp-blog-header.php' )	../noe-api.php:16 //Before this line, nothing special happens, then I include wp-blog-header.php
3	0.8151	56045320	wp( )	../wp-blog-header.php:14
4	0.8151	56045448	WP->main( )	../functions.php:947
5	0.8173	56095592	WP->handle_404( )	../class-wp.php:677 //Fatal

Thanking you very much.

Regards.

PS: I have echoed a var_dump of $wp_query just before this line to give you a little more context if that's of any help:

object(WP_Query)[165]
  public 'query_vars' => 
    array
      'page' => int 0
      'name' => string 'api' (length=3)
      'error' => string '' (length=0)
      'm' => string '' (length=0)
      'p' => int 0
      'post_parent' => string '' (length=0)
      'subpost' => string '' (length=0)
      'subpost_id' => string '' (length=0)
      'attachment' => string '' (length=0)
      'attachment_id' => int 0
      'static' => string '' (length=0)
      'pagename' => string '' (length=0)
      'page_id' => int 0
      'second' => string '' (length=0)
      'minute' => string '' (length=0)
      'hour' => string '' (length=0)
      'day' => int 0
      'monthnum' => int 0
      'year' => int 0
      'w' => int 0
      'category_name' => string '' (length=0)
      'tag' => string '' (length=0)
      'cat' => string '' (length=0)
      'tag_id' => string '' (length=0)
      'author' => string '' (length=0)
      'author_name' => string '' (length=0)
      'feed' => string '' (length=0)
      'tb' => string '' (length=0)
      'paged' => int 0
      'comments_popup' => string '' (length=0)
      'meta_key' => string '' (length=0)
      'meta_value' => string '' (length=0)
      'preview' => string '' (length=0)
      's' => string '' (length=0)
      'sentence' => string '' (length=0)
      'title' => string '' (length=0)
      'fields' => string '' (length=0)
      'menu_order' => string '' (length=0)
      'category__in' => 
        array
          empty
      'category__not_in' => 
        array
          empty
      'category__and' => 
        array
          empty
      'post__in' => 
        array
          empty
      'post__not_in' => 
        array
          empty
      'post_name__in' => 
        array
          empty
      'tag__in' => 
        array
          empty
      'tag__not_in' => 
        array
          empty
      'tag__and' => 
        array
          empty
      'tag_slug__in' => 
        array
          empty
      'tag_slug__and' => 
        array
          empty
      'post_parent__in' => 
        array
          empty
      'post_parent__not_in' => 
        array
          empty
      'author__in' => 
        array
          empty
      'author__not_in' => 
        array
          empty
      'ignore_sticky_posts' => boolean false
      'suppress_filters' => boolean false
      'cache_results' => boolean true
      'update_post_term_cache' => boolean true
      'update_post_meta_cache' => boolean true
      'post_type' => string '' (length=0)
      'posts_per_page' => int 10
      'nopaging' => boolean false
      'comments_per_page' => string '50' (length=2)
      'no_found_rows' => boolean false
      'order' => string 'DESC' (length=4)
  public 'tax_query' => null
  public 'meta_query' => 
    object(WP_Meta_Query)[4624]
      public 'queries' => 
        array
          empty
      public 'relation' => null
      public 'meta_table' => null
      public 'meta_id_column' => null
      public 'primary_table' => null
      public 'primary_id_column' => null
      protected 'table_aliases' => 
        array
          empty
      protected 'clauses' => 
        array
          empty
      protected 'has_or_relation' => boolean false
  public 'date_query' => boolean false
  public 'post_count' => int 0
  public 'current_post' => int -1
  public 'in_the_loop' => boolean false
  public 'comment_count' => int 0
  public 'current_comment' => int -1
  public 'found_posts' => int 0
  public 'max_num_pages' => int 0
  public 'max_num_comment_pages' => int 0
  public 'is_single' => boolean true
  public 'is_preview' => boolean false
  public 'is_page' => boolean false
  public 'is_archive' => boolean false
  public 'is_date' => boolean false
  public 'is_year' => boolean false
  public 'is_month' => boolean false
  public 'is_day' => boolean false
  public 'is_time' => boolean false
  public 'is_author' => boolean false
  public 'is_category' => boolean false
  public 'is_tag' => boolean false
  public 'is_tax' => boolean false
  public 'is_search' => boolean false
  public 'is_feed' => boolean false
  public 'is_comment_feed' => boolean false
  public 'is_trackback' => boolean false
  public 'is_home' => boolean false
  public 'is_404' => boolean false
  public 'is_embed' => boolean false
  public 'is_comments_popup' => boolean false
  public 'is_paged' => boolean false
  public 'is_admin' => boolean true
  public 'is_attachment' => boolean false
  public 'is_singular' => boolean true
  public 'is_robots' => boolean false
  public 'is_posts_page' => boolean false
  public 'is_post_type_archive' => boolean false
  private 'query_vars_hash' => string '49a108f3c6538dd0a5cc33d9f5f8e501' (length=32)
  private 'query_vars_changed' => boolean false
  public 'thumbnails_cached' => boolean false
  public 'updated_term_meta_cache' => boolean false
  public 'updated_comment_meta_cache' => boolean false
  private 'stopwords' => null
  private 'compat_fields' => 
    array
      0 => string 'query_vars_hash' (length=15)
      1 => string 'query_vars_changed' (length=18)
  private 'compat_methods' => 
    array
      0 => string 'init_query_flags' (length=16)
      1 => string 'parse_tax_query' (length=15)
  public 'query' => 
    array
      'page' => string '' (length=0)
      'name' => string 'api' (length=3)
  public 'request' => string 'SELECT   lm_posts.* FROM lm_posts  WHERE 1=1  AND lm_posts.post_name = 'api' AND lm_posts.post_type = 'post'  ORDER BY lm_posts.post_date DESC ' (length=143)
  public 'posts' => 
    array
      empty

Attachments (2)

35013.patch (574 bytes) - added by igmoweb 9 years ago.
Patch
35013-unit-tests.patch (594 bytes) - added by igmoweb 9 years ago.
Unit Tests

Download all attachments as: .zip

Change History (16)

#1 @SergeyBiryukov
9 years ago

  • Description modified (diff)

#2 @SergeyBiryukov
9 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.4.1

Hi jdmweb, welcome to Trac!

Thanks for the report. Introduced in [34476].

@igmoweb
9 years ago

Patch

@igmoweb
9 years ago

Unit Tests

#3 follow-up: @igmoweb
9 years ago

Nice catch @jdmweb

Here's a patch and a Unit Test

Cheers.

#4 in reply to: ↑ 3 @jdmweb
9 years ago

Replying to igmoweb:

Nice catch @jdmweb

Here's a patch and a Unit Test

Cheers.

Hi @igmoweb

Thank you for the patch.

I'm new to track. Am I supposed to do something with this patch and unit test? Or do I just wait for it to be released with a new minor WP version?

Thanking you.

Regards.

#5 @igmoweb
9 years ago

I guess it will be released in 4.4.1 as it's a fatal error but the patch has to be checked by someone with more permissions in the Trac, like a WordPress Core Commiter. Otherwise you can ask about the ticket in #core channel in Slack, there's a meeting every Wednesday: https://make.wordpress.org/core/weekly-developer-chats/

Cheers.

#6 follow-up: @swissspidy
9 years ago

  • Keywords has-patch commit added; needs-patch removed

We can't unit test fatal errors, but 35013.patch (with curly braces though :)) looks good.

#7 in reply to: ↑ 6 ; follow-up: @igmoweb
9 years ago

Replying to swissspidy:

We can't unit test fatal errors, but 35013.patch (with curly braces though :)) looks good.

Thanks @swissspidy. I still don't use curly braces on ifs with single lines due to old PHP Coding Standards. Do we need to add them in the patch?

#8 in reply to: ↑ 7 ; follow-up: @swissspidy
9 years ago

  • Owner set to swissspidy
  • Status changed from new to assigned

Replying to igmoweb:

Replying to swissspidy:
Thanks @swissspidy. I still don't use curly braces on ifs with single lines due to old PHP Coding Standards. Do we need to add them in the patch?

We use them everywhere for new code that we introduce, as per the WordPress PHP Coding Standards.

Since this is a rather small change, we can manually add them when committing. I'd probably do it like this:


if ($wp_query->post instanceof WP_Post ) ) {
        $p = clone $wp_query->post;
}

The reason for this is that is_a is deprecated before PHP 5.3.0 and throws E_STRICT warnings. See: http://php.net/manual/en/function.is-a.php#refsect1-function.is-a-changelog

#9 in reply to: ↑ 8 @SergeyBiryukov
9 years ago

Replying to swissspidy:

Since this is a rather small change, we can manually add them when committing. I'd probably do it like this:


if ($wp_query->post instanceof WP_Post ) ) {
        $p = clone $wp_query->post;
}

+1, would be consistent with [31188].

#10 @swissspidy
9 years ago

In 35994:

In WP::handle_404(), make sure $wp_query->post is a WP_Post object before cloning it.

Props igmoweb for initial patch.
See #35013.

#11 @swissspidy
9 years ago

  • Keywords fixed-major added

#12 follow-up: @dd32
9 years ago

Any reason this is using instanceof instead of just checking it's non-falsey? Is there any benefit to being explicit?

eg:

if ( $wp_query->post ) { 
    $p = clone $wp_query->post; 
} 

#13 in reply to: ↑ 12 @igmoweb
9 years ago

Replying to dd32:

Any reason this is using instanceof instead of just checking it's non-falsey? Is there any benefit to being explicit?

eg:

if ( $wp_query->post ) { 
    $p = clone $wp_query->post; 
} 

Because there are some lines later making reference to WP_Post attributes like $p->post_content.

#14 @dd32
9 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 36064:

In WP::handle_404(), make sure $wp_query->post is a WP_Post object before cloning it.

Merges [35994] to the 4.4 branch.
Props igmoweb, swissspidy.
Fixes #35013.

Note: See TracTickets for help on using tickets.