WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#4712 closed defect (bug) (fixed)

get_pages filter may be applied twice

Reported by: DD32 Owned by: markjaquith
Milestone: 2.3.1 Priority: normal
Severity: normal Version: 2.2
Component: Template Keywords: get_pages
Focuses: Cc:

Description

This is a bit hard to explain; so hang in there while reading.

Under certain conditions(WP Object cache being enabled), the get_pages filter is applied twice:

First, We get the pages from the database, And filter it.

$pages = $wpdb->get_results($query);
	$pages = apply_filters('get_pages', $pages, $r);

Thats fine, Next, We locate any children, and cache it:

if ( $child_of || $hierarchical )
		$pages = & get_page_children($child_of, $pages);

	$cache[ $key ] = $pages;
	wp_cache_set( 'get_pages', $cache, 'page' );

Thats also fine, So we've got the results, we've filtered it, and now we've put it into the cache as filtered.

Now, Next time the function runs:

	$key = md5( serialize( $r ) );
	if ( $cache = wp_cache_get( 'get_pages', 'page' ) )
		if ( isset( $cache[ $key ] ) )
			return apply_filters('get_pages', $cache[ $key ], $r );

We pull the cached _filtered_ results, and then we filter it again.

In general this isnt an issue, But if you've got a plugin adding pages to the get_pages function, AND the cache is enabled, then it ends up that the page will be added twice, Once when pulling from the DB, And then it'll be added again when its pulled from the cache later.

I've attached a patch to remove the filtering upon a cache retrieval, This has the downside that any plugin which modifies the get_pages output will have to deal with its output being cached.

I've also attached a patch to update the cache before filtering, So the filtering allways takes place.

Attachments (2)

4712.delay_filter_after_cache.diff (646 bytes) - added by DD32 8 years ago.
delay the fitlering until after caching
4712.remove_cached_filter.2.diff (462 bytes) - added by DD32 8 years ago.
remove the get_pages filter on the cached result

Download all attachments as: .zip

Change History (10)

@DD328 years ago

delay the fitlering until after caching

@DD328 years ago

remove the get_pages filter on the cached result

comment:1 @DD328 years ago

There was an error in the "4712.remove_cached_filter.diff" patch, replaced with .2

comment:2 @markjaquith8 years ago

  • Keywords get_pages added
  • Milestone set to 2.3 (trunk)
  • Owner changed from anonymous to markjaquith
  • Status changed from new to assigned
I've also attached a patch to update the cache before filtering, So the filtering allways takes place.

That sounds like the most flexible solution with least chance of breakage.

(FYI: I deleted your old patch)

comment:3 @markjaquith8 years ago

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

(In [5855]) Avoid running get_posts filter twice. Props DD32. fixes #4712

comment:4 @epper7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

If you have a plugin adding pages to the get_pages() function by the 'get_pages' filter AND the get_pages() function itself doesn't find any page, the result should infact be only the pages added by the plugin via the filter.

The problem is that with the change discussed in this ticket (and now live in wordpress 2.3) this cannot appen because of these lines: (in wp 2.2 this was ok)

1187 	    if ( empty($pages) )
1188 	        return array();

On empty result set from the database, the empty result is returned and is not filtered.

I don't really know wordpress code like you all but I think that this simple patch could help to solve the problem:

1187 	    if ( empty($pages) )
1188 	        return apply_filters('get_pages',array(),$r);

With this change, my plugin, which allows to add private pages (with a special parameter) to the wp_list_pages output, now works correctly.

comment:5 @foolswisdom7 years ago

  • Milestone changed from 2.3 to 2.4

comment:6 @ryan7 years ago

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

(In [6204]) Apply filteres when returning empty array from get_pages. Props epper. fixes #4712 for trunk

comment:7 @ryan7 years ago

(In [6205]) Apply filteres when returning empty array from get_pages. Props epper. fixes #4712 for 2.3

comment:8 @ryan7 years ago

  • Milestone changed from 2.4 to 2.3.1
Note: See TracTickets for help on using tickets.