[F] Plugins with Admin CP Hooks run on Plugin Updates page

[DougSD]

If a plugin uses an Admin CP hook (admin_page_output_header in this case), it runs when you visit the Plugin Updates page. In my case, the "Fast Menus" plugin by dvb runs. I've been able to reproduce it with my own plugin. I've attached it.

[Matt]

I've noticed that with that plugin too, it's deactivated, but runs where I'm on the updates page.

[Ryan Gordon]

Hm - It's because we have to include it on the ACP Updates page to get the guid identifier. We should use the NO_PLUGINS define (iirc) if we're visiting the version check page.

[DougSD]

What I think we need to do is edit the plugin class so that only active plugins' hooks are added on. Right now, there is no way to stop an un-active plugin from adding hooks if you have the file loaded.

Okay, here's my fix using an edited run_hooks function in the plugin class:

function run_hooks($hook, $arguments="")
{
	global $cache;
	
	if(!is_array($this->hooks[$hook]))
	{
		return $arguments;
	}
	$pluginlist = $cache->read("plugins");
	$this->current_hook = $hook;
	ksort($this->hooks[$hook]);
	foreach($this->hooks[$hook] as $priority => $hooks)
	{
		if(is_array($hooks))
		{
			foreach($hooks as $hook)
			{
				if(is_array($pluginlist))
				{
					$pluginfile = preg_replace("#_(.*)$#", "", $hook['function']);
					if(in_array($pluginfile, $pluginlist['active']))
					{
						$plugin_active = true;
					}
					else
					{
						$plugin_active = false;
					}
				}
				if(!isset($plugin_active) || $plugin_active == true)
				{
					if($hook['file'])
					{
						require_once $hook['file'];
					}
					$oldreturnargs = $returnargs; // why is this line of code here?
						
					$returnargs = call_user_func_array($hook['function'], array(&$arguments));
					
					
					if($returnargs)
					{
						$arguments = $returnargs;
					}
				}
			}
		}
	}
	$this->current_hook = '';
	return $arguments;
}

[frostschutz]

This user has been denied support.

Isn't this a bad idea since it adds overhead to every page? run_hooks() is called everywhere and you want to check wether a plugin is active when it's really only required on one page? Also this check doesn't seem very reliable - it makes assumptions based on function name, which would break some plugins.

Personally I think it's the plugins responsibility to handle such a case. Since the plugin is loaded on the plugins page, all other code (that's not wrapped in a function()) is executed too.

Maybe plugins could get a pluginname_init() or _main() function that is run right after the plugin is loaded, then we could put all our add_hooks and other initialization code there - then there'd no longer have to be any code outside of functions in plugins (except the IN_MYBB check), and the plugins page could load it without running the init function for inactive plugins so hooks won't get added for them. But that'd be a plugin API change that won't make it in 1.4 I guess...

[Ryan Gordon]

I have a fix that will work well. I will post it up in a bit

[Ryan Gordon]

In admin/modules/config/plugins.php find:

if($plugins_list)
	{
		foreach($plugins_list as $plugin_file)
		{
			require_once MYBB_ROOT."inc/plugins/".$plugin_file;
			$codename = str_replace(".php", "", $plugin_file);
			$infofunc = $codename."_info";
			if(!function_exists($infofunc))
			{
				continue;
			}
			$plugininfo = $infofunc();
			$plugininfo['guid'] = trim($plugininfo['guid']);
			
			if($plugininfo['guid'] != "")
			{
				$info[] = $plugininfo['guid'];
				$names[$plugininfo['guid']] = array('name' => $plugininfo['name'], 'version' => $plugininfo['version']);
			}
		}
	}

replace with

if($plugins_list)
	{
		$active_hooks = $plugins->hooks;
		foreach($plugins_list as $plugin_file)
		{
			require_once MYBB_ROOT."inc/plugins/".$plugin_file;
			$codename = str_replace(".php", "", $plugin_file);
			$infofunc = $codename."_info";
			if(!function_exists($infofunc))
			{
				continue;
			}
			$plugininfo = $infofunc();
			$plugininfo['guid'] = trim($plugininfo['guid']);
			
			if($plugininfo['guid'] != "")
			{
				$info[] = $plugininfo['guid'];
				$names[$plugininfo['guid']] = array('name' => $plugininfo['name'], 'version' => $plugininfo['version']);
			}
		}
		$plugins->hooks = $active_hooks;
	}

That should work.

[Ryan Gordon]

Thank you for your bug report.

This bug has been fixed in our internal code repository. Please note that the problem will not be fixed here until these forums are updated.

With regards,
MyBB Group