Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#21327 closed defect (bug) (fixed)

No default value is set for $group in WP_Object_Cache for the decr method

Reported by: bananastalktome Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Cache API Keywords: has-patch
Focuses: Cc:


Unlike incr and other methods on WP_Object_Cache, decr does not do a check to see if $group is empty to set it to 'default'. This is problematic when using wp_cache_decr() since the result is that set cache with wp_cache_set (which has $group set as 'default') is not decremented.

Patch included, but not sure if this should be classified 'normal' or 'major'. Unit tests still appear to pass, but more testing probably useful.

Attachments (2)

cache_decr_fix.patch (457 bytes) - added by bananastalktome 9 years ago.
cache_delete_fix.patch (757 bytes) - added by bananastalktome 9 years ago.

Download all attachments as: .zip

Change History (9)

#1 @nacin
9 years ago

  • Milestone changed from Awaiting Review to 3.5

Even low severity would be fine — this really only occurs if you use the method improperly by explicitly passing false, null, "", etc., which I imagine would result in a notice when it is used as a key.

#2 @bananastalktome
9 years ago

@nacin the problem is more with the wrapper wp_cache_decr, which passes an empty string as the group param to the decr method on global $wp_object_cache. So, for example, wp_cache_decr("dog", 1) would result in the grouping being set as an empty string rather than "default". I might be wrong, so please correct me if I am :)

#3 @nacin
9 years ago

Ah ha. Well, still only happens if you use wp_cache_decr() without a group. Always been broken, and not used in core. Good candidate for unit tests + patch.

#4 @bananastalktome
9 years ago

Ah, ok. Well a request for just that is in the unit-tests trac (http://unit-tests.trac.wordpress.org/ticket/110). I'm working on tests for this now, and hopefully putting up a patch soon.

Version 0, edited 9 years ago by bananastalktome (next)

#5 @nacin
9 years ago

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

In [21294]:

If $group is empty in WP_Object_Cache::decr(), consider it to be 'default'.

This is consistent with the rest of the cache methods wrapped by
functions; the functions pass an empty string by default, hence
the need for this check.

props bananastalktome.
fixes #21327.

#6 @bananastalktome
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Another issue found from the unit tests for these functions (#UT110), which is that the profiles of wp_cache_delete and WP_Object_Cache::delete() are different. wp_cache_delete is missing the optional third force parameter.

Patch added to add this as well (or should this be a separate ticket?)

Last edited 9 years ago by bananastalktome (previous) (diff)

#7 @nacin
9 years ago

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

wp_cache_delete() doesn't have a force parameter because there is nothing to "force". Same with wp_cache_get(). These parameters are only helpful with persistent caching backends.

Note: See TracTickets for help on using tickets.