current_user_can() can run before wp_get_current_user() is defined

[Viper007Bond]

Okay, so I'm writing a plugin and here's the code I have so far:

​http://wordpress.pastebin.com/f3ea57b78

When activating the plugin from the plugins page, I get the following error:

Fatal error: Call to undefined function wp_get_current_user() in [...]\wp-includes\capabilities.php on line 446

No, it's not in a blue box or anything -- it's just that error message. I have to go and rename or delete the plugin file to get my admin area back.

So two issues here:

A) What's causing this PHP error?

B) Why isn't it being caught by the box and dying gracefully? (I assume it's because the error isn't in the plugin's file.)

[Viper007Bond]

The issue is due to this line:

if ( !current_user_can('read') ) return;

current_user_can() is defined at the time this plugin runs, but it appears pluggable.php hasn't been loaded yet.

[westi]

The problem here is you plugin is running code when included.

That is a bad plan.

You should run your setup code on the plugins_loaded or init hook like so:

add_action('plugins_loaded', create_function('','global $WPcomAdminBar; $WPcomAdminBar = new WPcomAdminBar()'));

This ensures that all the WordPress functions are available before your code runs.

[Viper007Bond]

Yeah, I'm being bad in my coding skills (thanks for that slick class start code BTW), but still, shouldn't pluggable.php be loaded before capabilities.php?

[DD32]

I cant see any reason for pluggable.php to be loaded before capabilities.php,

If you follow coding guidelines and use the hook as westi mentioned, there are no issues. current_user_can() shouldnt be needed until all plugins and pluggable.php is loaded anyway.

Closing as invalid.

[Viper007Bond]

Replying to DD32:

I cant see any reason for pluggable.php to be loaded before capabilities.php,

Because capabilities.php is calling a function defined in pluggable.php ?

[DD32]

Replying to Viper007Bond:

Replying to DD32:

I cant see any reason for pluggable.php to be loaded before capabilities.php,

Because capabilities.php is calling a function defined in pluggable.php ?

I meant in normal operation, current_user_can() wouldnt be called until after plugins and pluggable.php had been loaded.

[Viper007Bond]

Replying to DD32:

I meant in normal operation, current_user_can() wouldnt be called until after plugins and pluggable.php had been loaded.

Correct, and I fixed my plugin to do that, but you shouldn't be able to call a defined function properly and get WordPress to make a fatal PHP error as a result. It's bad coding practice in my book. Having the function not work as intended (like returning a null result) is one thing, but literally breaking is another.

current_user_can() should either be defined later (and therefore be an undefined function at the time of plugin loading) or better yet, should not use functions not loaded yet.

So three possible solutions:

• Load pluggable.php before any user script is able to run so that current_user_can() won't throw a PHP error
  
• Put a function_exists() check inside of current_user_can() so that it can't run until everything it needs to operate is loaded
  
• Move capabilities.php to after plugins are loaded (seems like a bad idea)

I hate to get in an open/close war, but WordPress shouldn't throw PHP errors if a plugin calls a defined function correctly, no matter what. It needs abort gracefully.

[DD32]

Replying to Viper007Bond:

• Load pluggable.php before any user script is able to run so that current_user_can() won't throw a PHP error
  

Well that option is useless, It needs to be loaded after plugins to allow plugins to override them.

• Put a function_exists() check inside of current_user_can() so that it can't run until everything it needs to operate is loaded
  

That could work, If function doesnt exist, then user cannot perform operation, Which would backfire on anything requesting the action anyway, the function doesnt know yet if a user can or cant perform the action, returning null instead might be an option, But will still be interprated as a negitive response to most code.

• Move capabilities.php to after plugins are loaded (seems like a bad idea)

That could work, But what about functions in the other set of files loaded before the plugins? What if they require a function in capabilities.php?, or a function in pluggable.php, Should we move those after plugins are loaded too?

At least some of the files need to be loaded before plugins (database/get_option/sanitize_option/etc), All which require another function, which requires another.. Eitherway, You'll find another function which will need a function which is undefined at that stage.

Thats why plugin_loaded/init hooks are provided, To allow for code to be run once WP has loaded all its files, and is ready to work as intended, Until that time comes, Any code in any file(be it WP's or a Plugins) cant expect all functions to behave as intended.

I hate to get in an open/close war

And for that reason, I'll not touch the status of it

[darkdragon]

For what it is worth, I agree with DD32. It is unacceptable to assume that functions will exist without hooking into init hook first. So naturally, anything plugin that doesn't follow this principle has a bug in the plugin and not in WordPress. It is a limitation of the language and therefore seems illogical to assume that functions will be available before PHP knows about it.

Just because a function makes reference to a function that may not exist at that time, doesn't mean it will break anything until the function is called that contains the function in question.

As for fixing the pluggables, there is a pattern that would be useful. I forget what the name is. Hmm, related to Dependency Interjection, but I don't think that is the correct name for the pattern. No matter.

WTF? Returning null in a constructor, that is plain bad practice right there. You should know better than that!

[Viper007Bond]

Replying to darkdragon:

For what it is worth, I agree with DD32. It is unacceptable to assume that functions will exist without hooking into init hook first. So naturally, anything plugin that doesn't follow this principle has a bug in the plugin and not in WordPress. It is a limitation of the language and therefore seems illogical to assume that functions will be available before PHP knows about it.

I don't think there's any disagreement there.

My point though is that I'm not calling a function that doesn't exist, I'm calling one that does and that's causing all of WordPress to break and that there should be some error handling there. One would think that any function defined at the time custom code is run would be expected to work or that otherwise it'd be undefined, but maybe that's just me...

But anyway, I'll re-mark as invalid since the proper practice is to not run anything at plugin load.

WTF? Returning null in a constructor, that is plain bad practice right there. You should know better than that!

Heh, I wasn't actually putting that forward that exact solution, just that perhaps it'd be best for the function to fail gracefully. ;)