WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#27199 closed defect (bug) (fixed)

When site_name is missing from sitemeta table, it cannot be updated by Network > Settings screen

Reported by: JPry Owned by:
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.1
Component: Options, Meta APIs Keywords: has-patch
Focuses: multisite Cc:

Description

While it is uncommon, if the site_name meta_key is missing from the sitemeta table, it is not possible to update the site_name key using the Network Admin > Settings page. This is because a default value (the domain name) is set and cached.

This may be related to #16676, although I don't believe that change set introduced the problem.

Steps to reproduce:

  1. Set up WordPress Multisite.
  2. In the database, remove the site_name meta_key from the sitemeta table:
DELETE FROM `wp_sitemeta` WHERE `meta_key` = 'site_name';
  1. Go to the WordPress Dashboard, and navigate to Network Admin > Settings
  2. Change the value for "Network Title"
  3. Click the "Save Changes" button

When the page reloads, you will see that the Network Title has not been updated. You can confirm in the database that the site_name key has not been added back to the sitemeta table.

Here's what I believe is happening inside WordPress that causes this behavior:

  1. ms-settings.php calls the get_current_site_name() function. In 3.8.1 this is at line 112.
  2. get_current_site_name() attempts to determine the current site name (obviously). In doing that, it first checks the cache. If there's no cached value, it checks the sitemeta table. Finally, if there's no matching value in the sitemeta table, it defaults to the domain name of the network.
  3. Regardless of how the current site name was derived in step 2 above, the value is cached.
  4. When updating the site_name option, update_site_option() is called first.
  5. update_site_option() uses get_site_option() to determine the current value of the option, for comparison purposes.
  6. get_site_option() is allowed to use cache, and so it retrieves the cached value that was set inside get_current_site_name()
  7. update_site_option() has a value to use for comparison: the domain name of the site. Unfortunately, that value is not false. This means that update_site_option() will attempt to update the database instead of adding a new option.
  8. update_site_option() fails (returning a value of false) because nothing was updated, due to the fact that the value is not present in the database.

Attachments (1)

27199.patch (917 bytes) - added by JPry 6 years ago.

Download all attachments as: .zip

Change History (10)

@JPry
6 years ago

#1 @JPry
6 years ago

  • Keywords has-patch added

#2 follow-up: @jeremyfelt
6 years ago

  • Milestone changed from Awaiting Review to 3.9

Hey @jPry, thanks for the report and the patch!

I've run into this before once because I deleted that meta key, definitely weird behavior.

I think the approach of 27199.patch makes sense. It should be extremely rare that this fallback will ever be needed and if we are not able to find a value in the initial boot process, then caching an assumed one only covers the issue.

That said, this could affect other uses of get_site_option( 'site-name' ) as false will be returned until the meta key is restored. There are only a couple places in core that use this.

I wonder if we can replace all uses of get_site_option( 'site_name' ) with get_current_site_name() instead, as that would use the fallback properly.

Related #16676, which reduced the setting of site name cache by a bit in [18604].

#3 in reply to: ↑ 2 ; follow-up: @jeremyfelt
6 years ago

Replying to jeremyfelt:

I wonder if we can replace all uses of get_site_option( 'site_name' ) with get_current_site_name() instead, as that would use the fallback properly.

In addition to the patch on this ticket, not instead. :)

#4 @JPry
6 years ago

This plugin is a way around this bug without needing to modify core files.

#5 in reply to: ↑ 3 ; follow-up: @JPry
6 years ago

Replying to jeremyfelt:

Replying to jeremyfelt:

I wonder if we can replace all uses of get_site_option( 'site_name' ) with get_current_site_name() instead, as that would use the fallback properly.

In addition to the patch on this ticket, not instead. :)

I think that could work, but there is something to consider: The get_current_site_name() function is expecting to receive the $current_site object, and it returns the modified object. This means we have two possible ways to approach this:

  • We could declare the global $current_site object first, and then pass it to get_current_site_name() every time it is called.
  • We could declare the global $current_site object inside the get_current_site_name() function, perhaps only if $current_site is not passed to the function.

Jeremy, what do you think about those options?

#6 in reply to: ↑ 5 @jeremyfelt
6 years ago

Replying to JPry:

  • We could declare the global $current_site object inside the get_current_site_name() function, perhaps only if $current_site is not passed to the function.

I would go for this option. We reduced the use of the $current_site global quite a bit in #25158, so it would be nice if we could keep it simple and grab from get_current_site() inside get_current_site_name() if something has not been passed. The naming of the function kind of throws things off as is since there isn't much "current" about it.

#7 follow-up: @jeremyfelt
6 years ago

Ok. This should be resolved now.

As of [27359], core does not use get_current_site_name() for anything. The replacement behavior in get_network_by_path() does not yet set the $network_id:site_name cache, which should mean that it is generated the first time that get_site_option( 'site-name' ) is used.

I tested briefly and I am able to delete a network's site_name and then correct it as expected in Network Settings.

#8 in reply to: ↑ 7 ; follow-up: @JPry
6 years ago

Replying to jeremyfelt:

Ok. This should be resolved now.

As of [27359], core does not use get_current_site_name() for anything. The replacement behavior in get_network_by_path() does not yet set the $network_id:site_name cache, which should mean that it is generated the first time that get_site_option( 'site-name' ) is used.

I tested briefly and I am able to delete a network's site_name and then correct it as expected in Network Settings.

That sounds like a better solution to me. Shall we just close this ticket then?

#9 in reply to: ↑ 8 @jeremyfelt
6 years ago

  • Resolution set to fixed
  • Status changed from new to closed
  • Version changed from trunk to 3.1

Replying to JPry:

That sounds like a better solution to me. Shall we just close this ticket then?

Yep, I think we can.

Noting that this was mostly introduced in [18604].

Fixed in [27359]

Note: See TracTickets for help on using tickets.