WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 2 years ago

#41446 new defect (bug)

The PHP notice displayed after the overwriting global $posts by a new empty query.

Reported by: danielpietrasik Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.8
Component: Query Keywords:
Focuses: Cc:
PR Number:

Description

Tested in WordPress 4.8, twentyseventeen theme with default configuration (just installed).

After the overwriting a global variable $posts by a new get_posts() query (which return an empty array (no posts)) there is a PHP notice displayed:

Notice: Undefined offset: 0 in /.../wp-includes/class-wp-query.php on line 3162

The overwriting is not a best choice, but some themes and plugins do it, so we should solve this problem.

The problem is in WP_Query class in rewind_posts() method in wp-includes/class-wp-query.php file.

The original method:

<?php
public function rewind_posts() {
        $this->current_post = -1;
        if ( $this->post_count > 0 ) {
                $this->post = $this->posts[0];
        }
}

The problem: the global query has at least one posts, but a new query has not. So the $this->post_count property is always positive, but the $this->posts[0] is not exists. In this step, the PHP notice is displayed.

An example of solution:

<?php
public function rewind_posts() {
        $this->current_post = -1;
        if ( $this->post_count > 0 && isset( $this->posts[0] ) ) {
                $this->post = $this->posts[0];
        }
}

Right now the method checks that the query has posts and the first post is set and is not null.

Attachments (2)

41446.patch (478 bytes) - added by umangvaghela123 2 years ago.
It is fine if we use this solution for overcome the chance of error.
41446.2.patch (476 bytes) - added by umangvaghela123 2 years ago.
This one is proper solution

Download all attachments as: .zip

Change History (4)

#1 follow-up: @umangvaghela123
2 years ago

It`s is fine If we check !empty condition.

<?php
public function rewind_posts() {
        $this->current_post = -1;
        if ( isset( $this->posts[0] ) && !empty( $this->posts[0] ) && $this->post_count > 0  ) {
                $this->post = $this->posts[0];
        }
}
Last edited 2 years ago by umangvaghela123 (previous) (diff)

@umangvaghela123
2 years ago

It is fine if we use this solution for overcome the chance of error.

@umangvaghela123
2 years ago

This one is proper solution

#2 in reply to: ↑ 1 @danielpietrasik
2 years ago

Using empty() function wouldn't be redundant security?
Of course it is correct, but unnecessary. $this->post_count > 0 will always return false if the variable is empty. Could you give me some example when the $this->post_count > 0 will return true if the variable is empty.

Replying to umangvaghela123:

It`s is fine If we check !empty condition.

<?php
public function rewind_posts() {
        $this->current_post = -1;
        if ( isset( $this->posts[0] ) && !empty( $this->posts[0] ) && $this->post_count > 0  ) {
                $this->post = $this->posts[0];
        }
}
Note: See TracTickets for help on using tickets.