How to Confirm $_REQUEST['plugin'] is for the intended plugin, in uninstall.php

General Discussion Coding Help
1

Most of my plugins have an uninstall.php file.
This file runs when the plugin is deleted.

One of the security checks in said file needs to check if the request is actually for said plugin.

if ( ! defined( 'WP_UNINSTALL_PLUGIN' )
		|| empty( $_REQUEST )
		|| ! isset( $_REQUEST['plugin'] )
		|| ! isset( $_REQUEST['action'] )
		|| 'name-of-plugin-folder/name-of-plugin-main-file.php' !== $_REQUEST['plugin']
		|| 'delete-plugin' !== $_REQUEST['action']
		|| ! check_ajax_referer( 'updates', '_ajax_nonce' )
		|| ! current_user_can( 'activate_plugins' )
	) {

		exit;

	}

The problem with this is that’s hardcoded and when users rename the plugin, they can’t delete it anymore.

In all other files/actions like activation hook or deactivation hook you can use a Constant populated with plugin_basename( __FILE__ ).

But of course this will not work in uninstall file, since no other file will run and thus you can’t define a constant, i.g. in the plugin main file, and use it in the uninstall.php

If using plugin_basename( __FILE__ ) inside the uninstall, it will not have the same value as is in $_REQUEST['plugin'].
It returns plugin-folder-name/uninstall.php, but the request value is plugin-folder-name/plugin-main-file.php

so…

  • You can’t define a constant in main plugin file because that file is not even fired when uninstall is done.
  • You can’t detect plugin base name as it returns wrong name inside the very uninstall.php
  • But you need to check request is matching…

How do check if Request is indeed for my plugin in said file?
It seems not possible, unless hardcoding it, which then cause issues if a user renames the plugin folder name and tries to delete it after. They’ll have to keep the exact folder name I hardcode, and hardcode is bad anyway.

Would it be enough to just check for folder name (meaning, strip off the part after / from plugin_basename( __FILE__ ), then compare if that string is within the Request?
Or would it even be OK to fully omit plugin-folder/plugin-main.php !== $_REQUEST['plugin'] check?
(Note, most plugins I see do not check even for nonce in that file, but as far I know we should check if the request is really for our plugin, even when deleting it.)

Perhaps I am overthinking this one (too)

2

If your plugin is OOP, you can do away with the uninstall.php file and just add your deletion method like this to your main class. This keeps it fully encapsulated and you don’t need to check if it’s the right plugin.

class YourClass {

	public function __construct() {
		
		// Do other stuff...
		
		// Static class must use __CLASS__ here.
		register_uninstall_hook(__FILE__,    [__CLASS__, 'uninstall_plugin']);
	}

	// Note this is a static method.
	public static function uninstall_plugin() {
		if (!current_user_can('delete_plugins')) {
			return;
		}

		// Do your deletion stuff here.

	}

}
3 Likes
3

Anyone who renames a plugin must be assumed to have sufficient technical knowledge to resolve this issue for themselves.

When you provide a plugin, you provide something that worked when you zipped it. If the user makes any code changes they’re on their own.

TL;DR you’re overthinking it.

3 Likes
4

Ok thanks
So I’ll use variable where possible and hardcode in this case + perhaps add a notice to the readme just in case someone actually runs into this and wonders how to resolve it

Thanks!

1 Like
5

Oha… cool
Trying that first (for some reason I didn’t use it… not sure anymore why thou)

6

Aside: you can avoid such issues by always calling your main plugin file index.php

So the plugin identifier is always (folder)/index.php - and it’s easy to grab the folder.

7

Not sure to understand this… the request is wired to the full plugin folder/name
But uninstall file runs without index or main file

You suggest to include the main file once?

8

Here’s a full example that shows how to handle activation, deactivation, uninstall right in your primary class in a consistent way. Because it’s all inside your class, there’s no need to check if the request is for your plugin… if the method is invoked, it will only ever be for your plugin.

class YourClass {

	public function __construct() {
		
		// Do other stuff...
		
		// Register hooks for activation, deactivation, and uninstallation.
		register_uninstall_hook(__FILE__,    [__CLASS__, 'uninstall_plugin']); // Static method
		register_activation_hook(__FILE__,   [$this, 'activate_plugin']);
		register_deactivation_hook(__FILE__, [$this, 'deactivate_plugin']);
	}

	// Activation
	public function activate_plugin() {
		if (!current_user_can('activate_plugins')) {
			return;
		}

		// Do stuff here...
	}

	// Deactivation
	public function deactivate_plugin() {
		if (!current_user_can('activate_plugins')) {
			return;
		}

		// Do whatever here...

	}

	// Deletion
	public static function uninstall_plugin() { // <---- Static method
		if (!current_user_can('delete_plugins')) {
			return;
		}

		// Do your deletion stuff here.

	}

}
2 Likes
9

Thanks all!
Solved by using register_uninstall_hook instead of uninstall.php file.

Allows to make it all dynamic.

Will have to update a bunch of plugins now :stuck_out_tongue:

3 Likes
10

Might as well add the Update Manager client file at the same time. :laughing:

What. Too soon?

2 Likes
11

Not quite what I meant - but you’ve solved it the other way now so don’t worry about it!

1 Like
12

never too soon for a pun :stuck_out_tongue:

2 Likes
13

@anon66243189 We should add @anon71687268 example to the developer docs. It’s a good example how to handle activation, etc. Just a thought.

2 Likes
14

It will be part of the better plugin boilerplate as soon I push the update, thus as well included in the https://www.generateplugins.com generator.

I just did not have time yet to push it…

Once we have a doc (read: Plugin Handbook), we can then link to that, and also make examples directly within the doc. We could do a sort of “easiest/best practices” section for that, inside the (future) plugin handbook.

1 Like
15

This topic was automatically closed 2 days after the last reply. New replies are no longer allowed.