Opened 3 years ago
Closed 3 years ago
#53830 closed defect (bug) (fixed)
Default filters try to create nonce during installation before options table exists
Reported by: | schlessera | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 5.8.1 | Priority: | normal |
Severity: | normal | Version: | 5.7 |
Component: | Upgrade/Install | Keywords: | good-first-bug has-testing-info has-patch commit fixed-major |
Focuses: | Cc: |
Description (last modified by )
Similar to #39047, there's a new instance where a default filter tries to use wp_create_nonce()
during WP installation, which throws an error as the wp_options
table does not yet exist.
The filter is adding the default scripts on 'init'
, and the offending code is found in wp-includes/script-loader.php
:
<?php $user_id = isset( $_GET['user_id'] ) ? (int) $_GET['user_id'] : 0; did_action( 'init' ) && $scripts->localize( 'user-profile', 'userProfileL10n', array( 'user_id' => $user_id, 'nonce' => wp_create_nonce( 'reset-password-for-' . $user_id ), ) );
It was introduced via [50129].
As suggested by @swissspidy, the wp_create_nonce()
call should be wrapped like the following:
<?php ( wp_installing() && ! is_multisite() ) ? '' : wp_create_nonce(..)
Attachments (2)
Change History (13)
#1
@
3 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to Future Release
#2
@
3 years ago
- Description modified (diff)
- Keywords good-first-bug added
- Milestone changed from Future Release to 5.9
#4
@
3 years ago
This can also be reproduced in regular installation:
- Create a
wp-config.php
without the key/salt constants filled in. - Open
/wp-admin/install.php
. - At the bottom of that page, there is a
wp_print_scripts()
call. wp_default_scripts()
callswp_create_nonce()
.wp_create_nonce()
callswp_hash()
.wp_hash()
callswp_salt()
.wp_salt()
tries to save the key/salt values in the database and fails, as the table does not exist yet.
@
3 years ago
I have made patch as @schlessera suggested and tested, its working as expected and remove warnining
#5
@
3 years ago
- Keywords has-testing-info added
Test Report
Env
- WordPress: 5.7.2 and 5.8
- OS: macOS Big Sur 11.5
- Localhost: Local (wp) with PHP 8.0, nginx, MySQL 8.0.16
- Browser: Chrome
Test Steps
- Step 1: Create a new site
- Step 2: Open the
wp-config.php
file in your favorite editor - Step 3: Change the salt keys to empty string
define( 'AUTH_KEY', '' ); define( 'SECURE_AUTH_KEY', '' ); define( 'LOGGED_IN_KEY', '' ); define( 'NONCE_KEY', '' ); define( 'AUTH_SALT', '' ); define( 'SECURE_AUTH_SALT', '' ); define( 'LOGGED_IN_SALT', '' ); define( 'NONCE_SALT', '' );
- Step 4: Open the
wp-admin/install.php
file in your browser (e.g.http://newinstall.local/wp-admin/install.php
where my test site's ishttp://newinstall.local
- Step 4A: If message
Already Installed
appears, delete (drop) the tables in your local site's database. And then refresh the browser.
- Step 4A: If message
Notice the error:
WordPress database error: [Table 'local.wp_options' doesn't exist]
- Step 5: Apply the patch.
- Step 6: Refresh the browser.
Notice the error is gone and the installation can continue.
Results
See 53830-test.gif.
- Reproduced the issue ✅
- Applying 53830.patch resolves the issue ✅
#6
@
3 years ago
- Keywords has-patch commit added; needs-patch removed
@SergeyBiryukov thanks for the testing instructions!
@sanketchodavadiya thanks for the patch!
53830.patch resolves the issue (see testing report) ✅
Code review:
- The patch implemented the suggested strategy ✅
- The strategy is consistent with the other instances ✅
- PHPCS/WPCS passes ✅
composer lint ./src/wp-admin/install.php > @php ./vendor/squizlabs/php_codesniffer/bin/phpcs --report=summary,source './src/wp-admin/install.php' . 1 / 1 (100%)
- Tests pass ✅
npm run test:php -- --stop-on-failure Tests: 12462, Assertions: 57798, Skipped: 21.
Marking for commit
.
#7
@
3 years ago
- Owner set to SergeyBiryukov
- Resolution set to fixed
- Status changed from new to closed
In 51525:
Just noting that this appears to also happen when running unit tests, see this recent test run for example.
Moving to 5.8.1, as this was a recent change in 5.7 and the fix seems simple enough.