[wp-trac] [WordPress Trac] #43992: Prevent activation of a plugin if its required PHP version is too high

[WordPress Trac]

#43992: Prevent activation of a plugin if its required PHP version is too high
-------------------------------------------------+-------------------------
 Reporter:  flixos90                             |       Owner:  desrosj
     Type:  task (blessed)                       |      Status:  reviewing
 Priority:  normal                               |   Milestone:  5.2
Component:  Plugins                              |     Version:
 Severity:  major                                |  Resolution:
 Keywords:  needs-unit-tests servehappy dev-     |     Focuses:
  feedback has-patch needs-testing 2nd-opinion   |
-------------------------------------------------+-------------------------

Comment (by afragen):

 Replying to [comment:38 desrosj]:
 > This is testing pretty well for me. After some of the items below are
 solidified I can add some unit tests.
 >
 > At first look, the thought that "we should pass a `$context` of `plugin`
 or `plugin_validation` to the `get_file_data()` call in
 `get_plugin_validation_data()`" came to mind. This would allow extra
 headers to be added and detected. But, if this was added, then a filter
 would need to be added to the end of `get_plugin_validation()`. Otherwise,
 detecting any additional headers would be useless. I can't think of a good
 use case right now, though, so let's leave it as is.

 I only added the headers we need to check. At some point we could add a
 filter to this array if it's necessary. But I also not sure of a use case.

 > In the current state, any empty headers in the `readme.txt` will be
 ignored in favor of the plugin's headers, even if they are defined and
 empty. Just confirming that this is intended.
 >
 > Can you confirm that I am understanding the precedence correctly:
 > - Non empty values defined in `readme.txt`.
 > - Plugin file header values when `readme.txt` does not exist.
 > - When `readme.txt` exists but has empty (or undefined) values for
 either header, the empty values will fall back to the plugin file headers.

 So both `get_file_headers()` and `get_plugin_headers()` will return an
 empty string if the header is missing or has no value.

 Given that `readme.txt` files are required for the plugin directory any
 data existing here takes precedence. If a value exists in `readme.txt` it
 will be used. If a value doesn't exist in `readme.txt` then we look to the
 plugin headers to see if a value exists. If it does then we will use it.
 If no value exists in either place the return value would be an empty
 string.

 Currently we are very permissive and an empty string automatically denotes
 a valid compatibility test. We can always change this in the future via
 the functions below.

 > I am also a little concerned with the function names
 `is_wp_compatible()` and `is_php_compatible()` being too generic. I did
 some brief searching around, and `is_wp_compatible()` seems to be pretty
 common among plugins. I am mostly seeing `is_wp_compatible()` as private
 or protected class methods, but I'd like a few more opinions on whether we
 should adjust the names a bit.

 A search of [https://wpdirectory.net/search/01D6BR9YAZ8W32SXXPXK0H1JPC
 WPDirectory] for `is_wp_compatible` shows instances of the function name
 in only 20 different plugins and all seem to be namespaced.

 A search of [https://wpdirectory.net/search/01D6BRJPDFWXJ3YK752YS3X8W0
 WPDirectory] for `is_php_compatible` shows instances of the function name
 in only 17 different plugins and all seem to be namespaced.

 > For the `unmet_requirements` error, maybe the approach could be:
 > - Add a new function that returns a boolean indicating a plugin can be
 activated.
 > - `validate_plugin_requirements()` returns an array indicating if PHP
 passed, WP passed, and if either fails, a string with the correct
 contextual error message.
 >
 > Something like this could let us have different messages for each
 scenario. Examples: `Plugin does not meet minimum WordPress
 requirements.`, `Plugin does not meet minimum PHP requirements.`, `Plugin
 does not meet minimum WordPress and PHP requirements.`.

 Let me look into this.

 > Some other minor items:
 > - `@uses` [https://make.wordpress.org/core/handbook/best-practices
 /inline-documentation-standards/php/#phpdoc-tags should no longer be used]
 within PHP docblocks. `get_plugin_validation_data()` description needs to
 be revised to be more clear about priority that data is used.
 > - Can the short description for `get_plugin_validation_data()` be more
 specific about what is being validated?

 OK

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