is_string check in validate_file

[IwanLuijksQuestMedia]

When creating a plugin there is the possibility to get an object in validate_file's first parameter, since there is no check on the parameters this function accepts, it triggers the following warning. Worse: the actual plugin activation process keeps adding the plugin to the active_plugins list in the *_options table in result.

Warning: strpos() expects parameter 1 to be string, object given in /<path>/wp-includes/functions.php on line 3345
Call Stack:
0.0009 937456 1. {main}() /<path>/wp-admin/plugins.php:0
0.0013 1053776 2. require_once('/<path>/wp-admin/admin.php') /<path>/wp-admin/plugins.php:10
0.0014 1079792 3. require_once('/<path>/wp-load.php') /<path>/wp-admin/admin.php:30
0.0015 1100288 4. require_once('/<path>/wp-config.php') /<path>/wp-load.php:29
0.0019 1240920 5. require_once('/<path>/wp-settings.php') /<path>/wp-config.php:92
0.0969 26021992 6. wp_get_active_and_valid_plugins() /<path>/wp-settings.php:212
0.0971 26023184 7. validate_file() /<path>/wp-includes/load.php:506
0.0971 26023392 8. strpos() /<path>/wp-includes/functions.php:3345

@@ -3342,6 +3342,9 @@
  * @return int 0 means nothing is wrong, greater than 0 means something was wrong.
  */
 function validate_file( $file, $allowed_files = '' ) {
+       if (!is_string($file) || ($allowed_files !== '' && !is_array($allowed_files)))
+               return 4;
+
        if ( false !== strpos( $file, '..' ) )
                return 1;

[IwanLuijksQuestMedia]

This happens when in the plugin itself you use the variable $plugin, as in:

<?php
$plugin = \SomeQM\Plugin::instantiate();
$plugin->expose_hooks();

While the following doesn't trigger the errors:

<?php
$plugin2 = \SomeQM\Plugin::instantiate();
$plugin2->expose_hooks();

This is really bad side-effect behaviour, and should be fixed, or at least fully documented somewhere.

[juliobox]

Hello

So you're telling us that when you badly use a function, there is some PHP Warnings? Don't you say! :)
This is not even a WordPress bug, because you can not reproduce it without a plugin/script badly written, am i wrong?

The documentation is already there :

 * @param string $file File path.
 * @param array $allowed_files List of allowed files.
 * @return int 0 means nothing is wrong, greater than 0 means something was wrong.
 */
function validate_file( $file, $allowed_files = '' ) {

1st param is "string" as far as i can read.
So, i don't think that each function have to check each type of the given parameters, just use it correctly, unless it works in a clean WP install.

Thank you

[SergeyBiryukov]

Replying to IwanLuijksQuestMedia:

This happens when in the plugin itself you use the variable $plugin

Confirmed. $plugin = array() in a plugin file causes a bunch of notices on activation:

Notice: Array to string conversion in S:\home\wordpress\trunk\wp-includes\functions.php on line 3346
Notice: Array to string conversion in S:\home\wordpress\trunk\wp-includes\functions.php on line 3349
Notice: Array to string conversion in S:\home\wordpress\trunk\wp-includes\functions.php on line 3355
Notice: Array to string conversion in S:\home\wordpress\trunk\wp-includes\load.php on line 507

Notice: Array to string conversion in S:\home\wordpress\trunk\wp-includes\functions.php on line 3346
Notice: Array to string conversion in S:\home\wordpress\trunk\wp-includes\functions.php on line 3349
Notice: Array to string conversion in S:\home\wordpress\trunk\wp-includes\functions.php on line 3355
Warning: Illegal offset type in S:\home\wordpress\trunk\wp-admin\includes\plugin.php on line 874

It also leads to a corrupted active_plugins option, which can only be fixed by a manual reset:

Array(
    [0] => akismet/akismet.php
    [1] => debug-bar/debug-bar.php
    [2] => hello.php
    [3] => Array()
)

This is caused by accidentally overriding the local $plugin variable in activate_plugin():
tags/3.9/src/wp-admin/includes/plugin.php#L536​.

Appears to be introduced in [4811], which changed the calling sequence. Also related: #6308.

28102.patch​ fixes this by using a more unique name, $_wp_plugin_file, to restore the previous value.

We could probably just replace all instances of $plugin in activate_plugin() with $_wp_plugin_file, but $plugin seems more readable.

[SergeyBiryukov]

28102.2.patch​ adds a better comment.

[SergeyBiryukov]

In 28644:

Avoid stomping of the $plugin variable in activate_plugin().

fixes #28102.

[juliobox]

Same problem with $_wp_plugin_file = array(); now. So?

[SergeyBiryukov]

The idea is to use a variable with much smaller chances of accidental stomping than $plugin.

[nacin]

Thanks for the clear code comment in [28644].

[SergeyBiryukov]

Follow-up: #31104