WordPress.org

Make WordPress Core

Opened 23 months ago

Closed 23 months ago

Last modified 22 months ago

#25690 closed defect (bug) (fixed)

PHP Fatal error: Call to undefined function wp_get_current_user() when upgrading to version 3.7

Reported by: ascom Owned by: nacin
Milestone: 3.7.1 Priority: normal
Severity: blocker Version: 3.7
Component: Upgrade/Install Keywords:
Focuses: Cc:

Description

When I just upgraded the WordPress on my server, it brought me to /wp-admin/upgrade.php. It was blank. When I checked my server log:

[Thu Oct 24 20:43:04 2013] [error] [client 96.xxx.xxx.xxx] FastCGI: server "/usr/lib/cgi-bin/php5-fcgi" stderr: PHP message: PHP Fatal error:  Call to undefined function wp_get_current_user() in /var/www/wp-includes/user.php on line 215

I haven't tried to reproduce it. Thinking it was something with the updater, I downloaded and extracted the update manually (didn't fix it). I fixed it by adding require_once("pluggable.php"); to the top of
/wp-includes/capabilities.php. Then it upgraded my database successfully.

Attachments (2)

25690.diff (443 bytes) - added by nacin 23 months ago.
25690.ternary.diff (470 bytes) - added by georgestephanis 23 months ago.

Download all attachments as: .zip

Change History (23)

comment:1 @dd3223 months ago

This is caused by a plugin, can you revert your changes, deactivate all your plugins, and reactivate them to find the culprit?

There's at least one plugin I've seen so far, Reed Write which calls get_posts() upon inclusion.

Leaving open to find the cause and decide if it's a regression is just a bad doing it wrong scenario

comment:2 @ascom23 months ago

I do not have the plugin Reed Write. When reinstalling 3.7, it doesn't hit the error. I've noticed that for 3.6.1 -> 3.7. I think someone forgot to do the correct includes, because it looks like a mistake in the core (edit:)when updating the database.

Last edited 23 months ago by ascom (previous) (diff)

comment:3 @dd3223 months ago

I wasn't able to duplicate on a clean install + upgrade of 3.6.1.

comment:4 @usermrpapa23 months ago

I had Next Gen Gallery cause the same issue on one upgrade.

comment:5 @kpdesign23 months ago

Seeing this report from the forums for Plugin Organizer plugin: http://wordpress.org/support/topic/httpwordpressorgpluginsplugin-organizer-breaks-37?replies=1 and a couple more posts in the plugin's support forum: http://wordpress.org/support/plugin/plugin-organizer

comment:6 @dd3223 months ago

The particular error is pretty common from plugins who run code on inclusion, rather than on plugins_loaded or init.
Basically plugins that run code on inclusion are _diong_it_wrong() and WILL break on a future release.

These plugins appear to be running get_posts() upon inclusion, which is completely wrong to be doing (also, CPT's are not a great place to store options...) which in turn needs to figure out user details - of which, come from pluggable functions.. which are not included until after Plugins are loaded.

comment:7 @georgestephanis23 months ago

Related commits: r25689 and the big one of r25669 that swapped out the $user_ID global to get_current_user_id()

As Dion said, this is revealing bad practices in plugins and themes. Still, it'd be nice for a safety net. I'll try to pitch a diff tomorrow.

comment:8 @SergeyBiryukov23 months ago

  • Milestone changed from Awaiting Review to 3.7.1
  • Version changed from 3.6.1 to 3.7

comment:9 @nacin23 months ago

We don't need to revert everything in [25669]. Let's figure out what is possible to call prior to pluggable being loaded.

Looks to me like wp_insert_attachment(), wp_insert_post(), get_posts_by_author_sql() (which is a helper), and WP_Query itself.

I think we should patch get_current_user_id() to simply return 0 when that function does not exist. I don't love the idea of slowing down that function, but I don't think we use it extensively (it's not like calling it in a loop is going to be desired, the value doesn't change). At worst we can convert it to using $current_user directly, though I think we should just add a function_exists() check and call it a day. What a mess.

@nacin23 months ago

comment:10 @dd3223 months ago

25690.diff is also a thought that came to me while talking to georgestephanis, so +1 on that approach.

comment:11 follow-up: @georgestephanis23 months ago

I'm not quite sold on a ternary here, but figured I'd kick it in. I think I prefer the original patch more, though, as it's more understandable to read, and we'd have a spot to add a doing_it_wrong() call.

comment:12 in reply to: ↑ 11 @nacin23 months ago

Replying to georgestephanis:

I'm not quite sold on a ternary here, but figured I'd kick it in. I think I prefer the original patch more, though, as it's more understandable to read, and we'd have a spot to add a doing_it_wrong() call.

Yeah, that's just more work than is necessary.

comment:13 @nacin23 months ago

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

In 25929:

Have get_current_user_id() return 0 when pluggable.php is not yet included (which brings the ability to set or get the current user).

fixes #25690 for trunk.

comment:14 @nacin23 months ago

In 25930:

Have get_current_user_id() return 0 when pluggable.php is not yet included (which brings the ability to set or get the current user).

Merges [25929] to the 3.7 branch.

fixes #25690.

comment:15 @ascom23 months ago

Here are some plugins that I use that I am suspicious of:
WP SEO by Yoast - PHP Warnings on upgrading plugins
Plugin Organizer - from kpdesign's comment
YARPP (probably not)

Now I can't think of any more...

comment:16 @johnbillion23 months ago

We should put a _doing_it_wrong() in here too, but I'm not sure how useful it would be in practice if developers aren't already spotting other warnings and notices generated by their plugins.

comment:17 @jrf23 months ago

@ascom FYI on the warning on upgrading plugins (and themes and core) in WP SEO by Yoast

This is caused by the WP 3.7 change in the 'upgrader_process_complete' hook which was passing 3 variables in WP 3.6 and only two in WP 3.7.

The 'error' in the plugin is not blocking anything else from working so is totally not critical. [I created a fix](https://github.com/Yoast/wordpress-seo/issues/328) for this last Thursday which has already been committed to the plugin core. Just waiting a few more days to see if any other issues surface before releasing the plugin update.

comment:18 @markoheijnen23 months ago

Heard that today and seems like a wrong move to remove the 3rd argument.

comment:19 @johnbillion22 months ago

Changed in r25806 by the looks of things.

comment:20 @dd3222 months ago

Yeah, the 'upgrader_process_complete' hook was kind of unmarked internal use only, Didn't realise there were plugins using it yet, It was added in 3.6 for LP support but wasn't actually properly implemented, resulting in having to change it in 3.7.

comment:21 @jrf22 months ago

@dd32 WP 3.6 had just come out when I was working on that bit of WPSEO - checking for meta description tag in the header file of the current theme - and I wanted to make it as lean as possible. The upgrader_process_complete hook with a check for theme and action=upgrade seemed the leanest way to do it when a theme would be upgraded. The alternative for pre-WP3.6 was abusing a filter which is something I'd rather not do.

Anyway, a new version of WP SEO has been released by now which includes a fix.

Note: See TracTickets for help on using tickets.