Make WordPress Core

Opened 12 years ago

Closed 12 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's profile bananastalktome Owned by: nacin's profile nacin
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Cache API Keywords: has-patch
Focuses: Cc:

Description

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 12 years ago.
cache_delete_fix.patch (757 bytes) - added by bananastalktome 12 years ago.

Download all attachments as: .zip

Change History (9)

#1 @nacin
12 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
12 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
12 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
12 years ago

Ah, ok. Well a request for just that is in the unit-tests trac (#UT110). I'm working on tests for this now, and hopefully putting up a patch soon.

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

#5 @nacin
12 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
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Another issue found from the unit tests for these functions (http://unit-tests.trac.wordpress.org/ticket/110), 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?)

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

#7 @nacin
12 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.