-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Explicitly require the hash
extension.
#8138
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
@dd32 I would like to ask you to double check those stats you pulled about the |
PHP 7.4 is the first version to properly bundle the hash extension, so prior to PHP 7.4, it is technically possible to compile PHP without the hash extension. |
@johnbillion In it's current form, the error will only show the first extension that is missing. If an install is missing both required extensions, I think it would be helpful to include that in the WP_Error. Does something like this work? // Add a warning when required PHP extensions are missing.
$required_php_extensions = array( 'json', 'hash' );
$missing_php_extensions = array();
foreach ( $required_php_extensions as $required_php_extension ) {
if ( ! extension_loaded( $required_php_extension ) ) {
$missing_php_extensions[] = $required_php_extension;
}
}
if ( ! empty( $missing_php_extensions ) ) {
$php_extension_error = new WP_Error();
foreach ( $missing_php_extensions as $missing_php_extension ) {
$php_extension_error->add(
"php_not_compatible_{$missing_php_extension}",
sprintf(
/* translators: 1: WordPress version number, 2: The PHP extension name needed. */
__( 'The update cannot be installed because WordPress %1$s requires the %2$s PHP extension.' ),
$wp_version,
strtoupper( $missing_php_extension )
)
);
}
return $php_extension_error;
} |
@peterwilsoncc Good point, I'll take a look |
Edit: This isn't true, the |
@johnbillion It could be done on the API by only presenting the 6.8 update if the hash extension is available. The main consideration there would be how to handle two situations:
Each of these will remain a problem, even if a two step approach is taken (prep in 6.8, implement in 6.9) so the API approach will be needed in some form regardless. I'm sure patches to meta are welcome but I was unable to find where to create a PR. in the meta repo. It's possible the API is in the private systems repo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tests well, I was unable to reach the upgrade-core path as noted inline.
As the variable is a global, should it have a wp_
prefix too?
Edit: To test, I added elphaba
as a required extension to avoid the need to recompile PHP.
…php_extensions` global.
…too, in case a site bypasses the install or upgrade checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with this.
To test, I created a plugin that replaced the update API to present a locally hosted zip file. I then modified the required extensions to include a fictional extension as I didn't want to recompile PHP. That was included in the zip file.
Zip file: https://www.dropbox.com/s/dlq4f3dzjp7h2i7/wordpress-fake-ext.zip?dl=0
Hijack plugin: https://gist.github.com/peterwilsoncc/183592b397403e2220c3e511b9dc445f
I tested upgrading via:
- dashboard update: failed due to required extension
- wp cli update: failed due to required extension
- autoupdate: failed -- I did not get an email but presume it's due to the failed extension
I tested installation via WP CLI and it failed too due to the required extension.
This marks
hash
as a required extension and implements checks for required extensions in the same manner as PHP and MySQL version checks during installation, upgrade, and regular runtime. Thehash
extension is required for the following tickets:If a site doesn't have
hash
(orjson
) installed then this prevents the site from updating to 6.8 from a prior version becauseupdate-core.php
gets put into place during the update routine. More details at https://meta.trac.wordpress.org/ticket/7900 .Effects
If a version of PHP is being used that does not have the
hash
extension installed:hash
PHP extension", whether the update is performed via the Dashboard -> Updates menu or via thewp core update
command in WP-CLI. The site will remain at the prior version and will remain functional because at the point of this error the only file that has been overwritten isupdate-core.php
. If the user installs the missing extension they can proceed with the update.hash
PHP extension" error in the same way it would for an unmet PHP version requirement.hash
PHP extension", and installation won't proceed. This is the same behaviour as it would be for an unmet PHP version requirement.Testing steps
This is best tested by manually adjusting the
$required_php_extensions
array inwp-includes/version.php
to include an extension that doesn't exist on your installation. My preferred method is adding extensions named after fruit, but Pete prefers characters from musicals. Do what feels right for you.$required_php_extensions
present. Testing steps to follow.Testing on GitHub Actions
I've opened a separate PR at #8285 which combines this branch with #8252 in order to test the upgrade routine on GitHub Actions with an additional extension requirement that is not met. The
wp core update
step successfully fails with the appropriate error, preventing the upgrade from continuing. After the error the site remains operational at the previous version because no core files have been overwritten at that point except update-core.php. This is the same behaviour as an unmet PHP or MySQL requirement.