[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:  (none)
     Type:  task (blessed)                       |      Status:  new
 Priority:  normal                               |   Milestone:  5.2
Component:  Plugins                              |     Version:
 Severity:  major                                |  Resolution:
 Keywords:  needs-unit-tests servehappy dev-     |     Focuses:
  feedback has-patch needs-testing               |
-------------------------------------------------+-------------------------

Comment (by afragen):

 [attachment:"43992.15.diff"] based upon feedback below. Comments inline.

 Replying to [comment:33 TimothyBlynJacobs]:
 > Looking at .14
 >
 > 1. Patch applies cleanly.
 > 2. The `get_plugin_validation_data()` header should be updated to
 reference `get_file_data` instead of the readme parser.
 done
 > 3. The `$validation_headers` is invalid. It has duplicated array keys.
 While less clean, the array keys should probably be separate for the
 `tested|required` and the `tested|required up to` header variants and then
 merged back together with whichever variant is better.
 Shortened array to only 2 essential components.
 > 4. I think the `null === $plugin_data` check needs to either be relaxed
 or removed. I'd think the plugin headers should take priority and thus
 that code branch should always run, but not sure, either way the expected
 precedence should be documented. If just opting to relax the checks, it
 should also verify that the headers are not empty strings.
 We can discuss the precedence. As indicated in the comment blocks the
 precedence if for data in `readme.txt` with a fallback to new plugin
 headers.
 > 5. When calling `get_plugin_data`, can we disable `$markup` and
 `$translate`?
 done
 > 6. Document the return type. Should we return the tested up to values?
 It seems like they aren't used anywhere else in the patch and aren't
 provided by the plugin header.
 return type is documented as `array`, extraneous headers are no longer
 queried.
 > 7. Consider making the return type less ambiguous by returning
 `requires_wp` instead of just `requires`.
 `$requires` is parameter passed into the `is_php_compatible()` or
 `is_wp_compatible()` functions. Using a more generic parameter simply made
 the function and code more similar.
 > 8. I'd update `validate_plugin_requirements()` to return
 `true|WP_Error`. The error would detail which requirements weren't met.
 I think we'll have to agree to disagree. Returning a boolean for makes
 more sense to me as there is no error to be generated. If the validation
 fails, a WP_Error is generated.
 > 9. In concert with 7 update the error message in `activate_plugin` to be
 more specific. Depending on the implementation of
 `validate_plugin_requirements` it might make sense to just return that
 WP_Error object. Otherwise, if `activate_plugin` returns its own WP_Error,
 I think the error code could be more specific. Maybe `unmet_requirements`?
 I changed this to `unmet_requirements`

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