[wp-trac] [WordPress Trac] #29660: Functions in wp_includes/query.php assume non-null return value from get_queried_object

[WordPress Trac]

#29660: Functions in wp_includes/query.php assume non-null return value from
get_queried_object
--------------------------+-----------------------------
 Reporter:  yellyc        |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Future Release
Component:  Query         |     Version:  4.0
 Severity:  normal        |  Resolution:
 Keywords:  2nd-opinion   |     Focuses:
--------------------------+-----------------------------
Changes (by boonebgorges):

 * keywords:  4.6-early has-patch needs-testing => 2nd-opinion


Comment:

 I remain concerned that adding `empty()` checks will paper over legitimate
 bugs.

 The code path described by @Howdy_McGee in #33729 is a legitimate way to
 trigger the PHP notice, but IMHO this is due to an underlying problem with
 the semantics of our `is_*` functions.

 What does it mean to say `$this->is_category()`? Does it mean that a
 category was *requested*? Or does it mean that you're viewing an actual
 category? The way `WP_Query` currently works, the answer depends on when
 you ask the question:

 a. At or before 'pre_get_posts', `is_category` is based solely on the
 results of the rewrite rule, *not* on a database query. So on
 `example.com/category/foo`, when 'foo' does not exist,
 `$this->is_category` is *true* and `$this->is_404` is *false*.
 b. If you ask the question after `WP::handle_404()`, then
 `$this->is_category` is *false* and `$this->is_404` is *true*.

 Which is "correct"? It depends on whether you think the `is_*` flags
 should reflect the intent of the request, or the results of the request.
 It seems to me that `is_404` always refers to the query results - why
 would you _intend_ to query a 404? - so I think the latter is the case.
 But if I'm right about this, then `is_*` queries are pretty much
 unreliable anytime before `'wp'`. See also #14729.

 Alternatively, it would be possible to modify the way that `parse_query()`
 works, so that flags like `is_category` are set back to false if the
 requested object does not exist. But this will mean doing an additional
 database query during every page request. There are a bunch of existing
 tickets related to this: #31355, #27015, #21790. It is a pretty major
 change to the way `WP_Query` works, but it may be required if we're going
 to promise developers that they can use the conditional `is_*` methods at
 'pre_get_posts'.

 In any case, given the above, I'm against papering over the error.
 Developers should be aware that `is_category()` is not accurate at
 'pre_get_posts'. If anything, we should make the error message more
 descriptive.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/29660#comment:21>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform