git-updater: Requiring pluggable.php too soon breaks other plugins
Requiring pluggable.php in Bootstrap.php breaks all other plugins that are trying to replace functions in that file.
In my case, I’m using post-smtp to handle email and it detect that the function wp_mail is already defined. This prevent the plugin to do its work.
As WordPress Codex say:
WordPress loads the built-in functions only if they are undefined after all plugins have been loaded.
Loading pluggable.php before all other plugins should not be done.
pluggable.php is also loaded in GU_Trait.php but i didn’t tested if this file is loaded before any other plugins and it should be verified.
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 15 (8 by maintainers)
I just looked at the diff for the last 9 commits on the develop branch and the approach is correct. You removed every
static::$nonceand usedwp_verify_noncewith parameters from submitted forms.Yes, that’s the right mood! 😂
As you say, WPCS gives you a notice everytime you use $_GET, $_POST, or $_REQUEST.
The check that they do is quite simple and you have to recognize where the error can be suppressed (i think with
// phpcs:ignore WordPress.Security.NonceVerification.Recommended) or by checking thenonce.In general, any submission from a form that you create must be verified with the nonce (basically in your case any forms under
/wp-admin/options-general.php?page=git-updater).Any other case must be analyzed. In the case of activation hook, the nonce is verified by WordPress itself and you are reading
$_GET['plugin']after that check is already passed. So no risk of CSRF attack in that function and you can safely ignore WPCS error.The 10.6.0 release has 39 changed files. Next week i can comment some of those changes to help you understand what I would have done.
When it comes to security, always ask for a second opinion… I’m sure what I’m telling you but I’m still human. 😅
Sorry, I mean line 50 in the previous comment. 😅