$network_wide is unreliable

[mensmaximus]

This issue may be related to ticket #31104

Scenario:
In a WordPress network an admin decides a plugin must not be network activated. Only a per site or even a site specific activation shall be allowed.

Idea:
Conditional check during plugin activation and die with an error message if $network_wide is true.

Result:
Plugin does not get activated but the custom error message is not displayed

Assume the following function run during activation (it is simplified and the real activation sequence does not have echos!)

function test_activation_hook( $network_wide ) {
	echo '<span>1 | </span>';
	// exit( 1 );
	if ( is_multisite() ) {
		echo '<span>2: | </span>';
		// exit( 2 );
		if ( $network_wide ) {
			echo '<span>3: | </span>';
			// exit( 3 );
		} else {
			echo '<span>4: | </span>';
			// exit( 4 );
		}
	} else {
		echo '<span>5: | </span>';
		// exit( 5 );
	}
	echo '<span>6: | </span>';
	exit( 6 );
}

If you exit after if(is_multisite) the message displayed will be '1 | 2 |'
If you exit after if($network_wide) during a networkwide activation the message displayed is '1 | 2 | 4 | 6' instead of '1 | 2| 3|'
Exiting at 4, 5, or 6 will display '1 | 2| 4|', '1 | 2| 5|' and '1 | 2| 6|' as expected.

After many hours of debuging I realized the function (the action filter 'activate_' . $plugin) gets executed 3 times if you exit it early. And only the first call has the argument $network_wide set to true if it is an network_wide activation.

If you like to exit at 3, that is you do not want a network activation, the flow is therefore as follows:

• The first run (click on network activate) has $network_wide set to true and will exit at 3
• the second run (no idea why) has $network_wide set to false and will pass 1 | 2 | 4 | 6
• the third run (no idea why) has $network_wide set to false and will pass 1 | 2 | 4 | 6

Imho calling the activation function (or the code) three times in case you exit it (better say terminate because you cant end a plugin installiton clean) is wrong at all. However if this is by design then $network_wide must be reliably set to true if it originally was.

[mensmaximus]

To visualize this issue better I have captured some information at the beginning of the function (where exit(1) is located):

Captured during a successful network activation

***** activate_wpmu-activation-test/index.php: 2016-04-03 10:27:49 *****
(
    [stage] => 1
    [network_wide] => 1
    [GET] => Array
        (
            [action] => activate
            [plugin] => wpmu-activation-test/index.php
            [plugin_status] => all
            [paged] => 1
            [s] => 
            [_wpnonce] => ed6fce4af2
            [networkwide] => 1
        )

)

Captured while exiting at exit(3) to avoid network activation of the plugin

***** activate_wpmu-activation-test/index.php: 2016-04-03 10:25:46 *****
(
    [stage] => 1
    [network_wide] => 1
    [GET] => Array
        (
            [action] => activate
            [plugin] => wpmu-activation-test/index.php
            [plugin_status] => all
            [paged] => 1
            [s] => 
            [_wpnonce] => ed6fce4af2
            [networkwide] => 1
        )

)

***** activate_wpmu-activation-test/index.php: 2016-04-03 10:25:48 *****
(
    [stage] => 1
    [network_wide] => 
    [GET] => Array
        (
            [action] => error_scrape
            [plugin] => wpmu-activation-test/index.php
            [_wpnonce] => c9063923af
        )

)
***** activate_wpmu-activation-test/index.php: 2016-04-03 10:25:48 *****
(
    [stage] => 1
    [network_wide] => 
    [GET] => Array
        (
            [action] => error_scrape
            [plugin] => wpmu-activation-test/index.php
            [_wpnonce] => c9063923af
        )

)

[mensmaximus]

Well now I understand the issue arises from calling plugin_sandbox_scrape() in /wp-admin/plugins.php. Just for a test I set

$_GET['networkwide'] = 1

within plugin_sandbox_scrape() and replaced

if ( $network_wide )

with

if ( $_GET['networkwide'] )

in my activation test (the back compat for plugins). As a result the message I want to show is displayed. However this is not a (good) solution and still I think calling plugin_sandbox_scrape() twice is wrong.

[mensmaximus]

And I think there is one more bug in /wp-admin/plugins.php within the 'error_scrape' case from the switch statement. Line 166 shows

do_action( "activate_{$plugin}" );

but I think this should be

do_action( "activate_{$plugin}", is_network_admin() );

like within activate_plugin() in /wp-admin/includes/plugin.php where in shows in line 573

do_action( 'activate_' . $plugin, $network_wide );

[mensmaximus]

After some more investigation I can definitely say passing true to do_action( "activate_{$plugin}" ); in plugins.php line 166 solves the described problem.

The question now is how can we determine whether the plugin activation was started on the network admin screen? is_network_admin() is not working in case error_scrape because WP_NETWORK_ADMIN is not defined and $GLOBALS['current_screen']->in_admin( 'network' ) is empty. I even tried to pass is_network_admin() as an argument to $iframe_url in /wp-admin/plugins.php line 438.

[mensmaximus]

No feedback? Really nobody? Not a single statement for 3 month? Just me talking to myself?

[earnjam]

If I'm reading this correctly, you want to add support in core for plugin authors to prevent network activation of their plugins? Right now this could be done manually within the code of the plugin, but you would like to facilitate this by supporting it directly within core?

I could see maybe something like the opposite of the Network: true header.

So you could have 3 options for plugins on multisite:

1. Network activation only
2. Individual site activation only
3. Either network or individual site activations

Right now 3 is the default and 1 is possible if the Network: true header is set. I could see declaring Network: false as serving that purpose.

I could work on a patch for this ticket, but want to make sure I understand your goal.

[mensmaximus]

Replying to earnjam:

Right now 3 is the default and 1 is possible if the Network: true header is set. I could see declaring Network: false as serving that purpose.

This is what the ticket is about and what the title says. $network_wide is not set consistently and therefore you cant rely on it. Please read my opening post again. You can exit the code after the if ($network_wide) but you can't send a admin message to display a warning like "this plugin cant be activated networkwide". The problem stems from the error checking as explained in my comments 2, 3 and 6. The $network_wide argument passed to the plugin activation gets lost if the activation process raises an error.

[mensmaximus]

The best solution I can come up with is the attached patch. The only place I could find to check for is_network_admin() is before error_scrape is called. I have added the value to the GET parameters so I can fetch it and pass it to activate_{plugin}. In my tests this works well and has no negative impact.

[flixos90]

The patch appears solid to me. What we need to think about is whether the is_network_admin() check will be reliable enough to know that we're network-activating the plugin. I think it is in this case - but maybe we can find a more accurate solution.

[jeremyfelt]

I think I've wrapped my head around what's possible and what's missing here. Thanks for your patience @mensmaximus!

First, I don't think there's a way to short-circuit network activation of a plugin and show a custom or configurable error message in our current state. When the plugin's activation hook fires, WordPress expects to get null back. If anything else is output, then a predetermined path to an error condition occurs.

• If echo 'error'; is used, an error explaining that unexpected characters were output will appear. The plugin will still be activated with just a warning to be cautious.
• If exit() is used, a fatal error will occur during activation and a different error will be showing.

It's when this fatal error is show that the plugin goes through additional validations and the plugin activation hook is run again with varying $network_wide values. If the plugin is network activated with no error, the activation hook fires once.

It's possible to work around the activation by using the activated_plugin hook to reverse activation immediately after. Something like this:

function remove_this_plugin( $plugin ) {
	$current = get_site_option( 'active_sitewide_plugins', array() );
	unset( $current[ $plugin ] );
	update_site_option( 'active_sitewide_plugins', $current );
}

register_activation_hook( __FILE__, 'test_activation_hook' );
function test_activation_hook( $network_wide ) {
	if ( $network_wide ) {
		add_action( 'activated_plugin', 'remove_this_plugin' );
	}
}

This works, but provides no feedback to the user that the plugin activation was not successful.

It seems like there should be room for a filter that allows a plugin to set the WP_Error message in a way that a proper error will be displayed. I'm not yet sure if I'm missing anything in thinking that way.

I'm going to push this back to Future Release. We should continue discussing possible approaches.

[mensmaximus]

Thank you for the feedback. However I don't understand the final statement to push it back again. My patch is working as expected. If I exit the activation while network_wide was set the activation is stopped and my custom error message ist shown.

[jeremyfelt]

Replying to mensmaximus:

Thank you for the feedback. However I don't understand the final statement to push it back again. My patch is working as expected. If I exit the activation while network_wide was set the activation is stopped and my custom error message ist shown.

I'm not yet sure that working around the generation of a fatal error is the proper way to handle this. It may be, but pushing it back provides some time to think about other possible approaches.

[mensmaximus]

Now as 4.7 is launched I would really like to see my patch or any other reliable solution to make it into 4.7.1.

Still I do not understand what you are saying @jeremyfelt. My solution does not produce any fatal errors. My patch just fixes the missing $network_wide parameter. This is neither an enhancement nor a new feature. This is a bug fix.

I really would like to avoid patching 72 wordpress multisite installations after each update for a proper error handling :-).

[mensmaximus]

Original Patch updated for WP 4.7

[swissspidy]

Now as 4.7 is launched I would really like to see my patch or any other reliable solution to make it into 4.7.1.

4.7.1 is specifically for regressions introduced in 4.7. Tickets like this one would get into a major release. No agreement could be reached for 4.7, that's why it was pushed back.

Still I do not understand what you are saying @jeremyfelt. My solution does not produce any fatal errors. My patch just fixes the missing $network_wide parameter. This is neither an enhancement nor a new feature. This is a bug fix.

Nobody said your solution would produce any fatal errors, but that the patch works around the handling of fatal errors. Quote:

I'm not yet sure that working around the generation of a fatal error is the proper way to handle this.

A quick note on the patch:

• Patches should ideally be made from the root directory of the develop repository, so that the path reads like src/wp-admin/plugins.php, not .mensmaximus.com/htdocs/wp-admin/plugins.php or plugins. like in the latest patch. Otherwise the patch cannot be applied easily.
• $_REQUEST[ 'network_wide' ] is not being sanitized. Since you only want a boolean value, that line could be simply changed to do_action( "activate_{$plugin}", isset( $_REQUEST['network_wide'] ) );

Further:

• The activate_plugin hook really needs to be fixed. At one point the $network_wide argument passed and at another point it is not — without any documentation about it.
• Sharing the same concern as Felix:

What we need to think about is whether the is_network_admin() check will be reliable enough to know that we're network-activating the plugin. I think it is in this case - but maybe we can find a more accurate solution.

Adding needs-unit-tests to see if we can add unit tests for this. Not sure how testable the code is though.

[mensmaximus]

@swissspidy thank you for the explanation. I understand I have to wait until the core development team takes care of it.

[KestutisIT]

It looks like that there is no solution currently at all, because I cannot put do_action from no reason at some random point of code, and I must use:

<?php
 register_activation_hook($this->coreConf->getPluginPathWithFilename(), array(&$this, 'activate'));

and in the:

public function activate()
{
// there is NO way to check is that is single site, or network activation
}

So the only hack might be to put:

<?php
if(is_network_admin())
{
  register_activation_hook($this->coreConf->getPluginPathWithFilename(), array(&$this, 'networkActivate'));
} else is_admin())
{
  register_activation_hook($this->coreConf->getPluginPathWithFilename(), array(&$this, 'activate'));
}

And this is critical, because this do not allow normal activation of network plugin at all, without leaving ability for plugin then being activated for single site only in multisite mode.

[KestutisIT]

Replying to KestutisIT:

It looks like that there is no solution currently at all, because I cannot put do_action from no reason at some random point of code, and I must use:

<?php
 register_activation_hook($this->coreConf->getPluginPathWithFilename(), array(&$this, 'activate'));

and in the:

public function activate()
{
// there is NO way to check is that is single site, or network activation
}

So the only hack might be to put:

<?php
if(is_network_admin())
{
  register_activation_hook($this->coreConf->getPluginPathWithFilename(), array(&$this, 'networkActivate'));
} else is_admin())
{
  register_activation_hook($this->coreConf->getPluginPathWithFilename(), array(&$this, 'activate'));
}

And this is critical, because this do not allow normal activation of network plugin at all, without leaving ability for plugin then being activated for single site only in multisite mode.

And even my hack with is_network_admin() doesn't work here, because if I do debug test for network-install click, it says that is a regular admin page, not a network. So that works only after plugin install, when I browse plugin pages, but not during plugin activation. So there is absolutely NO SOLUTION AT THIS MOMENT.

[jeremyfelt]

Replying to KestutisIT:

and in the:

public function activate()
{
// there is NO way to check is that is single site, or network activation
}

$network_wide is available in activate() here and can be used to detect if this is a network activation.

The issue described by this ticket is that there's no clean way to avoid network activation when $network_wide is seen as true.

[SergeyBiryukov]

#47646 was marked as a duplicate.

[KestutisIT]

3 years and no progress made. I believe the severity of this ticket has to be set to "blocker". As there is NO WAY further develop plugins, as multisite is now one of the main parts of the WP Core.
Also nobody published any workaround.

I described the situation more clear in my ticket:
https://core.trac.wordpress.org/ticket/47646

[KestutisIT]

Just an additional suggestion to this is that we should have two hooks in the future, instead of playing with IF/ELSE statements in the same hook, meaning:

<?php
register_activation_hook(..);

which would be called only on local activations, doesn't matter if the site is based on multisite or not,

and

<?php
register_network_activation_hook(..);

which would be called only after call from 'network plugins admin' with 'network activate click'.

That would make WordPress much more user friendly and developers would be able to write more cleaner and do make less bugs in the code.

[earnjam]

blocker should be reserved for issues that make WordPress completely unusable and completely prevent a release from occurring. This does not meet that criteria.

[KestutisIT]

It definitelly not 'normal'. There is NO WAY to develop plugins at all in WordPress at this point for WordPress network. I'd say it is 'blocker'. But if @earnjam claims that it is regarding WordPress core only, not the WP plugins, then it should remain as 'critical'. In any case - 3 years way too long for this, meaning that no network plugins can be developed.