WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#22162 closed defect (bug) (fixed)

WP_Query must convert posts to WP_Post objects after filters are run

Reported by: nacin Owned by: ryan
Milestone: 3.5 Priority: normal
Severity: critical Version: 3.5
Component: Query Keywords: has-patch
Focuses: Cc:

Description

WP_Query sets all found posts to WP_Post objects with this:

if ( $this->posts )
	$this->posts = array_map( 'get_post', $this->posts );

This takes place before the post_results and the_posts filters, which may actually set new posts. One use case would be to set posts from cache (which may still be stdClass objects) after bypassing the original query using the posts_request or post_request_ids filters (which is why we needed to add if ( $this->posts ) to begin with).

For more, see the advanced post caching plugin, a newer version of which is running on WP.com. The symptom they saw there is the queried object ends up an stdClass, and then in nav-menu-template.php — _wp_menu_item_classes_by_context(), specifically — $queried_object->ancestors is assumed to be an array. In reality, it will be null, as the stdClass object has no magic. This generates a warning.

#14426 was just re-opened for similar gotchas.

The solution: WP_Query must convert posts to WP_Post objects after filters are run. This second call could even only run when filters are not suppressed, I imagine.

Attachments (4)

22162.diff (783 bytes) - added by ryan 6 years ago.
array_map if count changes
22162.2.diff (417 bytes) - added by ryan 6 years ago.
is_a in sanitize_post loop
22162.3.diff (699 bytes) - added by ryan 6 years ago.
Replace sanitize loop with get_post array_map
22162.4.diff (578 bytes) - added by ryan 6 years ago.
Wrap the_preview in get_post() to ensure WP_Post

Download all attachments as: .zip

Change History (15)

#1 @scribu
6 years ago

  • Cc scribu added

#2 @scribu
6 years ago

  • Version set to trunk

#3 @ryan
6 years ago

Arguably we'd want to run them both before and after. Maybe we should check the count before and after the filters run and and re-map the array if the count changed.

#4 @scribu
6 years ago

Maybe we should check the count before and after the filters run and and re-map the array if the count changed.

I don't think that's a good heuristic. Maybe some plugin just replaces one post with another.

Always running array_map() shouldn't be too expensive.

@ryan
6 years ago

array_map if count changes

#5 @ryan
6 years ago

It's probably good enough.

Alternatively, since we already do a foreach sanitize_post() loop, do an is_a check and upgrade stdClass objects.

@ryan
6 years ago

is_a in sanitize_post loop

#6 @scribu
6 years ago

  • Keywords has-patch added

+1 for 22162.2.diff.

#7 @ryan
6 years ago

nacin mentioned that we should probably wrap the_preview in get_post().

We also talked briefly about posts_results and whether there should be a re-map after it as well. Decided to skip that until we see problems with it in the wild.

#8 follow-up: @nacin
6 years ago

Is the sanitize loop even needed? get_post() should call filter().

@ryan
6 years ago

Replace sanitize loop with get_post array_map

#9 in reply to: ↑ 8 @ryan
6 years ago

Replying to nacin:

Is the sanitize loop even needed? get_post() should call filter().

Indeed. That loop can be lost with another array_map of get_post used instead. That will ensure both sanitization and WP_Post.

#10 @ryan
6 years ago

In [22238]:

Replace the sanitize loop at the end of WP_Query::get_posts() with an array_map of get_post(). get_post() will ensure each object in the loop is sanitized and is of the type WP_Post. see #22162

@ryan
6 years ago

Wrap the_preview in get_post() to ensure WP_Post

#11 @ryan
6 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [22244]:

Pass the result of the the_preview filter through get_post() to ensure the post is filtered and of type WP_Post. fixes #22162

Note: See TracTickets for help on using tickets.