[wp-trac] [WordPress Trac] #57580: Avoid errors from null parameters in add_submenu_page()

[WordPress Trac]

#57580: Avoid errors from null parameters in add_submenu_page()
------------------------------------------+-----------------------------
 Reporter:  ipajen                        |       Owner:  (none)
     Type:  enhancement                   |      Status:  reopened
 Priority:  normal                        |   Milestone:  Future Release
Component:  General                       |     Version:
 Severity:  normal                        |  Resolution:
 Keywords:  PHP81 has-patch dev-feedback  |     Focuses:  administration
------------------------------------------+-----------------------------

Comment (by nprwp):

 If you are going to make the parameter only accept a real string, which
 could be sensible if you want the callee to check for nulls, although
 personally I'd wish PHP could allow you to overload the function with a
 version that does accept null, checks for it, then calls the original just
 for obeying 'be liberal in what you accept, be conservative in what you
 emit' principle (but alas, there's no support for that), it results in an
 indirect problem too, which I think should be changed.

 There's a way in which this interacts with another part of wordpress that
 causes quite a few plugins (including paid ones that are hard to update)
 to fail by using wordpress in a way that seems intended, given the
 documentation of this other part.

 I'm referring to the function {{{plugins_url()}}}, which is used by
 plugins to serve files or otherwise figure out where they're installed,
 which may be important to use php's file writing/reading functions do do
 stuff.

 {{{
 plugins_url( null, __FILE__ )
 }}}

 is the typical way you see this function called, which then indirectly
 calls {{{wp_is_stream}}}. The function signature looks like this:

 {{{
 /**
  * Retrieves a URL within the plugins or mu-plugins directory.
  *
  * Defaults to the plugins directory URL if no arguments are supplied.
  *
  * @since 2.6.0
  *
  * @param string $path   Optional. Extra path appended to the end of the
 URL, including
  *                       the relative directory if $plugin is supplied.
 Default empty.
  * @param string $plugin Optional. A full path to a file inside a plugin
 or mu-plugin.
  *                       The URL will be relative to its directory.
 Default empty.
  *                       Typically this is done by passing `__FILE__` as
 the argument.
  * @return string Plugins URL link with optional paths appended.
  */
 }}}


 Notice how each parameter says "optional"? This usually, in terms of type
 hints, in a (strongly) typed language with no ability for optional
 parameters, as is the case with php7, which is still supported by
 wordpress, the normal way to do it is to pass NULLs for values you don't
 need if you need to pass parameter !#2, but not parameter !#1. This is
 ''very'' common for this function; as if your plugin has a simple file
 structure, it typically doesn't have a subdirectory for downloadable files
 but uses the regular path.

 Inverting the order of the parameters (which was badly chosen) is not
 really an option because it'd break everyone's code. So you're left with
 only the option to ignore it and let plugins break everywhere or fix it
 for them.

 This ability to pass both NULL and a value is represented as {{{string?}}}
 which is different from {{{string}}} in that it **accepts nulls**. Yet,
 you're effectively asking for a strict {{{string}}} in plugins_url while
 denoting the param as 'optional'; but it works for PHP8 in one way and for
 PHP7 in another.

 Should this function gain a null check that casts the value to 'empty
 string' for its first parameter? A lot of plugins pass null and it appears
 to be a sensible thing to do for compatibility reasons, which then gets
 them snafu'd (and the user a bunch of warning messages that mess up their
 site) for choosing to use {{{NULL}}} instead of {{{''}}} when sites update
 to PHP8.

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