WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#14047 closed defect (bug) (invalid)

get_user_by() makes use of the cache even if it does not exists any longer

Reported by: hakre Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.9.2
Component: General Keywords: reporter-feedback close
Focuses: Cc:

Description

This is mostly an error report, and it's quite bloody. But I thought I better report it before it gets lost.

I ran over this while having the following error message:

Fatal error: Call to a member function get() on a non-object in wp-includes\cache.php on line 93

That is the following code:

function wp_cache_get($id, $flag = '') {
	global $wp_object_cache;

	return $wp_object_cache->get($id, $flag);
}

So this API function (!) is blindly using that global variable w/o even checking if it's that what it thinks it is. Well not only that this is dangerous with any global in this case, every other function that relies on the function like get_user_by() does exit with a fatal error. for example after the cache has been shut down.

Attachments (2)

14047.diff (2.8 KB) - added by wojtek.szkutnik 5 years ago.
14047.patch (774 bytes) - added by hakre 5 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 follow-up: @hakre5 years ago

More Info: In my case $GLOBALSwp_object_cache? is still set, but it's value is NULL.

comment:2 in reply to: ↑ 1 @hakre5 years ago

My previous comment is wrong. Correction:

More Info: In my case $GLOBALS['wp_object_cache'] is not set any longer / NULL.

comment:3 @scribu5 years ago

  • Keywords needs-patch added
  • Milestone changed from Unassigned to 3.1

comment:4 @wojtek.szkutnik5 years ago

  • Keywords has-patch gsoc added; needs-patch removed

Patch added. Using is_object and method_exists is as safe as it could get. I'm only wondering if there's any need to check if the method exists if $wp_object_cache is valid?

@wojtek.szkutnik5 years ago

comment:5 @wojtek.szkutnik5 years ago

  • Cc wojtek.szkutnik@… added
  • Keywords needs-testing added

@hakre5 years ago

comment:6 @hakre5 years ago

Under certain circumstance the defensive development approach is good. But I do not think that in wordpress defensive programming is common sense.

In this cache case for example, this will create overhead for 99.999% of all cases.

With my patch I approach the problem from the other side to offer plugin or core developers the possibility to check for cahce existance (for example on shutdowns or so).

comment:7 @nacin5 years ago

  • Keywords reporter-feedback close added; has-patch needs-testing gsoc removed
  • Milestone changed from Awaiting Triage to Awaiting Review

get_user_by() is a pluggable function. Even if it were defined in mu-plugins and used immediately, the object cache should already be available. Can you provide steps to reproduce?

comment:8 @nacin4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.