[wp-trac] [WordPress Trac] #55432: The $plugin parameter of the "plugin_loaded" action can be polluted by individual plugins

[WordPress Trac]

#55432: The $plugin parameter of the "plugin_loaded" action can be polluted by
individual plugins
---------------------------+---------------------
 Reporter:  stevegrunwell  |       Owner:  (none)
     Type:  defect (bug)   |      Status:  new
 Priority:  normal         |   Milestone:  6.0
Component:  Plugins        |     Version:
 Severity:  normal         |  Resolution:
 Keywords:  has-patch      |     Focuses:
---------------------------+---------------------

Comment (by jrf):

 I tend to agree with @peterwilsoncc here: this should be a "wontfix" as
 this can only happen when a plugin is ''doing it wrong''.

 > The issue isn't so much with what the plugin could do so much as that
 the "plugin_loaded" hook does not behave in a predictable or consistent
 manner.

 That's the case, though, with '''''every single filter''''' in WP, as well
 as with '''''every action using WP global vars'''''.

 Think: a filter provides a `string` and expects a string back, the first
 function filtering returns an array, the second function filtering is
 getting a `TypeError` if they don't do any type checking before using the
 received parameter.

 Also see: https://core.trac.wordpress.org/ticket/51525

 As for the proposed solutions:

 > Wrap the include_once call in a closure (or named function) to isolate
 its variable scope

 I agree with @joyously that is bound to cause problems due to global
 variables not being available and not being explicitly approached as
 global variables. It's the kind of BC-break which will break plenty of
 ''not that well-coded/older'' plugins.

 > If you have to make a change here, I vote for renaming the variable.

 That, again would be a BC-break, albeit smaller. This is a documented
 global variable in WordPress, even though only available in this limited
 context, and changing the name of the variable will:
 1) still not prevent people from overwriting the ''new'' name.
 2) may break plugins relying on the `$plugin` variable being available in
 that context.

 > Update the DocBlock to indicate that this argument has type "mixed" and
 cannot be relied upon.

 That's not a solution, that's turning the problem around and making it
 worse. The documented type is correct and should stay documented as
 `string`. Anything else is giving plugins license to deliberately change
 WP global variables.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/55432#comment:7>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform