#54800 closed defect (bug) (fixed)
Nonce creation causes DB access errors when initializing multisite networks
Reported by: | schlessera | Owned by: | peterwilsoncc |
---|---|---|---|
Milestone: | 6.0 | Priority: | normal |
Severity: | normal | Version: | 5.0 |
Component: | Script Loader | Keywords: | has-patch has-testing-info fixed-major |
Focuses: | Cc: |
Description
The default script assets to be enqueued via the script loader use nonces for the assets that integrate with the WP REST API.
The nonce creation through wp_create_nonce()
fails with a database error when it is called on a site that has not fully been initialized yet, as the table where the nonce is supposed to be stored does not yet exist.
WordPress database error Table 'wp_cli_test.wp_sitemeta' doesn't exist for query SELECT meta_value FROM wp_sitemeta WHERE meta_key = 'nonce_key' AND site_id = 1 made by include('phar:///home/alain/bin/wp/php/boot-phar.php'), include('phar:///home/alain/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php'), WP_CLI\bootstrap, WP_CLI\Bootstrap\LaunchRunner->process, WP_CLI\Runner->start, WP_CLI\Runner->load_wordpress, require('wp-settings.php'), do_action('init'), WP_Hook->do_action, WP_Hook->apply_filters, register_block_core_file, register_block_type_from_metadata, register_block_script_handle, wp_register_script, wp_scripts, WP_Scripts->__construct, WP_Scripts->init, do_action_ref_array('wp_default_scripts'), WP_Hook->do_action, WP_Hook->apply_filters, wp_default_scripts, wp_create_nonce, wp_hash, wp_salt, get_site_option, get_network_option
There is already some safeguarding in place around these nonces, but it is not enough, as it specifically excludes the case of a multisite setup.
The ! is_multisite()
condition should be removed from these safeguards to ensure the error does never surface.
This is a follow-up ticket to #54634.
Change History (24)
This ticket was mentioned in PR #2147 on WordPress/wordpress-develop by schlessera.
3 years ago
#1
- Keywords has-patch added
This ticket was mentioned in Slack in #core by schlessera. View the logs.
3 years ago
This ticket was mentioned in Slack in #core by schlessera. View the logs.
3 years ago
#4
@
3 years ago
The root cause here was introduced with the Gutenberg 10.5 merge: [50824]
The files that added block registration caused scripts to be enqueued unconditionally. This in turn triggers the wp_default_scripts
hook and function, which includes REST API assets that require a nonce.
This ticket was mentioned in Slack in #cli by schlessera. View the logs.
3 years ago
#7
@
3 years ago
- Keywords reporter-feedback added
[50824] has been identified as the root cause, though this changeset was introduced in 5.8, not in 5.9.
I'm hesitant to move backport the patch to 5.9 without understanding root cause introduced in 5.9 and testing more broadly.
@schlessera some questions to help with decision making:
- Is this error happening on 5.8 also? (should be if the changeset is when the issue was introduced into Core)
- Is this error on all PHP versions?
- With this error, does the upgrade or site fail? (severity)
#8
@
3 years ago
- Keywords needs-testing-info added
Also, @schlessera can you provide the step-by-step instructions on how to reproduce the reported errors? This information helps testers create Test Reports to confirm original error and fix resolution. Thank you!
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
#10
@
3 years ago
As per today's bug scrub, let's keep this ticket in the milestone until WP 5.9 RC3 tomorrow.
This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.
3 years ago
peterwilsoncc commented on PR #2147:
3 years ago
#12
These changes look good to me and prevent the issue when installing. To test via WP-CLI I was running the commands:
wp db --yes reset wp core multisite-install --title='wpdevms' --admin_user=admin --admin_email=admin@example.com --admin_password=password --url=http://wp-dev.local/
During the install process, define( 'MULTISITE', true )
is included in my wp-config file. No salts are defined in code, causing WP to fall back to the defaults.
@schlessera I've merged an up to date version of trunk in to this PR to get the tests running on Core in the current state.
#13
@
3 years ago
Testing this with @costdev today, we came to the conclusion these database errors were added later and during the WP 5.9 cycle.
The linked pull request appears to fix the error.
--- Reproduction notes ---
The easiest method to reproduce is via WP CLI.
wp-config.php
- Ensure the file includes
define( 'MULTISITE', true );
- Ensure the file does not include any of the
*_KEY
or*_SALT
definitions - Ensure debug logging is enabled
Run the following commands via WP CLI (changing the host name as appropriate)
wp db --yes reset wp core multisite-install --title='wpdevms' --admin_user=admin --admin_email=admin@example.com --admin_password=password --url=http://wp-dev.local/
--- Without the patch applied ---
Multiple errors are logged during the installation process as the wp_create_nonce()
is called. Each error is similar to the log entry below but the meta_key
value can vary:
WordPress database error Table 'wordpress.wp_sitemeta' doesn't exist for query SELECT meta_value FROM wp_sitemeta WHERE meta_key = 'nonce_key' AND site_id = 1 made by include('phar:///usr/local/src/wp-cli/bin/wp/php/boot-phar.php'), include('phar:///usr/local/src/wp-cli/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php'), WP_CLI\bootstrap, WP_CLI\Bootstrap\LaunchRunner->process, WP_CLI\Runner->start, WP_CLI\Runner->load_wordpress, require('wp-settings.php'), do_action('init'), WP_Hook->do_action, WP_Hook->apply_filters, register_block_core_file, register_block_type_from_metadata, register_block_script_handle, wp_register_script, wp_scripts, WP_Scripts->__construct, WP_Scripts->init, do_action_ref_array('wp_default_scripts'), WP_Hook->do_action, WP_Hook->apply_filters, wp_default_scripts, wp_create_nonce, wp_hash, wp_salt, get_site_option, get_network_option
--- With the patch applied ---
No nonce related errors are thrown during the installation process.
#14
@
3 years ago
- Keywords has-testing-info added; reporter-feedback needs-testing-info removed
- Milestone changed from 5.9 to 5.9.1
- Priority changed from high to normal
- Severity changed from major to normal
- Version changed from 5.8 to 5.0
Adding a few notes from #core slack discussions:
@peterwilsoncc identified [51501] as the possible source that introduced the nonce table errors in 5.9
I ended up running bisect twice as it doesn’t quite make sense to me as to why [51501] would be introducing the nonce table erorrs.
Following up the earlier notes for the database errors thrown during multisite installs (reported in #54801 and #54800). The errors during
wp core multisite-install
are new for 5.9 so it’s preferred to focus on those.
Multiple errors when attempting to set the same via
set_site_transient()
These errors appear to be coming from to
wp_is_block_theme()
calls within the following locations:
create_initial_post_types()
_add_default_theme_supports()
wp_enable_block_templates()
The nonce errors as reported due to the conditions
is_installing()
&&! is_multisite()
when setting up the default scripts, this appears to have been added during the commit I mentioned above.
Knowns:
- [51501] highlighted the problem during
wp_installing()
as 3 new functions eventually callget_theme_roots()
which then callsget_transient()
for single site orget_site_transient()
for multisite.get_transient()
has checks forwp_installing()
whereasget_site_transient()
does not.- Peter opened #54849 to address this problem possibly in 5.9.1.
- Root cause of the errors can be resolved in #54849.
- Reporting of the errors (this ticket) is due to what @schlessera identified and fixed with PR 2147.
Next steps:
- Lowering the severity level and priority as the reporting of the errors does not cause the upgrade or site to fail.
- PR 2147 is ready for commit to
trunk
.
- What about backporting to 5.9? Thinking this needs to move to 5.9.1 to go with #54849. Why?
- The reporting of the errors were not introduced during RC or beta and are not blockers for a final release.
- The severity of does not seem to be a blocker for 5.9 final release.
- The root cause resolution (see #54849) is in 5.9.1 milestone.
I welcome discussion on whether to backport to 5.9 during RC or push this to 5.9.1. Please note, if backport doesn't happen in time for RC3 today, backporting would require another RC release.
#15
@
3 years ago
- Owner set to peterwilsoncc
- Resolution set to fixed
- Status changed from new to closed
In 52602:
#16
@
3 years ago
- Keywords fixed-major added
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for merging in to the 5.9 branch for 5.9.1.
Please do not merge to the 5.9 branch prior to the release of WordPress 5.9.
peterwilsoncc commented on PR #2147:
3 years ago
#17
#18
@
3 years ago
comment:21:ticket:39047 has some context for adding the ! is_multisite()
checks removed in [52602].
Some history here:
This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.
3 years ago
#20
follow-up:
↓ 21
@
3 years ago
@SergeyBiryukov Are you noting the history for information purposes or are you after some changes prior to merging in to the 5.9 branch? If the latter, are you able to be more specific?
#21
in reply to:
↑ 20
;
follow-up:
↓ 22
@
3 years ago
Replying to peterwilsoncc:
Are you noting the history for information purposes or are you after some changes prior to merging in to the 5.9 branch?
I think I wanted to see if comment:21:ticket:39047 is still relevant, but didn't get around to it, so I guess it's just for information purposes at this point :)
#22
in reply to:
↑ 21
@
3 years ago
Replying to SergeyBiryukov:
Replying to peterwilsoncc:
Are you noting the history for information purposes or are you after some changes prior to merging in to the 5.9 branch?
I think I wanted to see if comment:21:ticket:39047 is still relevant, but didn't get around to it, so I guess it's just for information purposes at this point :)
I think it's still somewhat valid, but I think leaving [52602] in place for trunk is a good idea. I don't see this as being something worth back-porting to 5.9.x though, it's not a regression or major bug.
There's three cases here:
- Single Site:
wp_installing()
truthful, actually installing, nonces unavailable. - Multisite Site creation:
wp_installing()
truthful, but nonces available due to Site being installed. With [52602] in place, any pages where a site is being created, the nonces which are no longer being generated here are now unavailable, even though they can be generated. Does that cause a problem? Unsure. - Multisite Network Creation:
wp_installing()
truthful, but nonces unavailable due to Network not being created yet.
Somewhat importantly, that last one is not exposed via the WordPress UI's as far as I know? so it was never really accounted for.
I guess in part, wp_installing()
is really being overloaded for multiple different things, so doesn't always make total sense.
Perhaps the fix actually needs to be made lower in the stack, when wp_installing()
functions that trigger DB access should check the table exists before accessing it?
#23
@
3 years ago
- Milestone changed from 5.9.1 to 6.0
- Resolution set to fixed
- Status changed from reopened to closed
I agree with Dion's comment above:
I think it's still somewhat valid, but I think leaving [52602] in place for trunk is a good idea. I don't see this as being something worth back-porting to 5.9.x though, it's not a regression or major bug.
Closing as fixed and changing the milestone to 6.0
#24
@
2 years ago
Somewhat importantly, that last one is not exposed via the WordPress UI's as far as I know?
Yea, think there is no way to install a multisite network directly. The only way is to install a single site first, then upgrade it (afaik).
This bug seems specific to WP CLI. Maybe worth another look there if installing a multisite network directly doesn't interfere somewhere else.
This PR adapts the safeguards around the
wp_create_nonce()
calls in thewp-includes/script-loader.php
file to ensure they are never called during a WordPress installation process.The calls were already safeguarded but only on non-multisite setups, which has turned out to be not enough.
Trac ticket: #54800