Make WordPress Core

Opened 2 weeks ago

Last modified 8 days ago

#54800 reopened defect (bug)

Nonce creation causes DB access errors when initializing multisite networks

Reported by: schlessera Owned by: peterwilsoncc
Milestone: 5.9.1 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 (18)

This ticket was mentioned in PR #2147 on WordPress/wordpress-develop by schlessera.


2 weeks ago

  • Keywords has-patch added

This PR adapts the safeguards around the wp_create_nonce() calls in the wp-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

This ticket was mentioned in Slack in #core by schlessera. View the logs.


2 weeks ago

This ticket was mentioned in Slack in #core by schlessera. View the logs.


2 weeks ago

#4 @schlessera
2 weeks 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.

Last edited 8 days ago by SergeyBiryukov (previous) (diff)

#5 @johnbillion
2 weeks ago

  • Version changed from trunk to 5.8

This ticket was mentioned in Slack in #cli by schlessera. View the logs.


13 days ago

#7 @hellofromTonya
9 days 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 @hellofromTonya
9 days 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.


9 days ago

#10 @audrasjb
9 days 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.


9 days ago

#12 @prbot
9 days ago

peterwilsoncc commented on PR #2147:

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 @peterwilsoncc
9 days 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 @hellofromTonya
8 days 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 call get_theme_roots() which then calls get_transient() for single site or get_site_transient() for multisite. get_transient() has checks for wp_installing() whereas get_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.
  • The code for this fix is from 5.0 (see #45065 and changeset [44114]).

Next steps:

  • Lowering the severity level and priority as the reporting of the errors does not cause the upgrade or site to fail.
  • 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 @peterwilsoncc
8 days ago

  • Owner set to peterwilsoncc
  • Resolution set to fixed
  • Status changed from new to closed

In 52602:

Script loader: Prevent DB errors during Multisite install.

Prevent the script loader from attempting to create nonces during the installation process for Multisite configurations.

Prior to this fix, multiple "Table does not exist" errors were thrown during installation if MULTISITE was defined in the wp-config.php file but the salt constants were not defined. Without the salts defined in PHP, WP was attempting to use the database fallbacks prior to table creation.

Props schlessera, johnbillion, hellofromTonya, audrasjb, costdev.
Fixes #54800.

#16 @peterwilsoncc
8 days 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.

#18 @SergeyBiryukov
8 days ago

comment:21:ticket:39047 has some context for adding the ! is_multisite() checks removed in [52602].

Some history here:

  • [39684] added a wp_installing() && ! is_multisite() check for the wp-api nonce.
  • [44114] used the same check in a new wp_default_packages_inline_scripts() function.
  • [51525] used the same check for the user-profile nonce.
Note: See TracTickets for help on using tickets.