Make WordPress Core

Opened 15 months ago

Last modified 11 months ago

#59522 new defect (bug)

Bug handling multisites where main site URL and main blog URL are different

Reported by: robdxw's profile robdxw Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.3
Component: Networks and Sites Keywords: has-patch needs-testing
Focuses: multisite Cc:

Description

WordPress 6.3 and up appear to break handling of multisites where the BLOG_ID_CURRENT_SITE and the domain and path for the network in the wp_site table do not point at the same blog. It treats the the site configured in wp_site as if it is also the main blog in the network (over-riding BLOG_ID_CURRENT_SITE). This results in image paths being broken on the site set in wp_site if it is not in the root directory.

How to recreate the bug:

  1. In WordPress 6.2 or below, create a directory-based multisite, with 2 sites in the network, site 1 at just http://localhost and site 2 at http://localhost/foo. In wp-config.php, set both SITE_ID_CURRENT_SITE and BLOG_ID_CURRENT_SITE to 1. In the wp_site table (which should only contain 1 entry, with ID 1), set the domain to localhost and the path to /foo/.
  2. Upload an image to the media library of http://localhost/foo. It should display correctly in the library, and have a URL of http://localhost/onsblog/wp-content/uploads/sites/foo/[year]/[month]/[your-filename].
  3. In the network site list, at http://localhost/foo/network/sites.php, localhost should be marked as the main site, reflecting the BLOG_ID_CURRENT_SITE constant.
  4. Upgrade to WordPress 6.3 or 6.3.1.
  5. In the media library of http://localhost/foo, the uploaded image will no longer display. If you view the image properties, it will report an image URL of http://localhost/onsblog/wp-content/uploads/[year]/[month]/[your-filename], i.e. the subsite part of the path is missing.
  6. In the network site list, localhost/foo is now marked as the main site, which does not reflect the BLOG_ID_CURRENT_SITE setting.

This bug also results in all images uploaded via the classic editor prior to 6.3 404-ing in the frontend on the affected subsite post-upgrade.

This behaviour was introduced in this commit: https://github.com/WordPress/WordPress/commit/05f19b1ed76af6d60855a1d44b8c15a166c5e36f, presumably unintentionally given the commit message. This line specifically is the issue: https://github.com/WordPress/WordPress/commit/05f19b1ed76af6d60855a1d44b8c15a166c5e36f#diff-254cbb80547c44f4b3a7ef37746bb5eb9f42bf29f0de5215034a8d1015928fecR242.

When the WP_Network object is populated via the WP_Network::get_instance() method, the $id property is populated via a WPDB query: https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wp-network.php#L106. This returns the id from the database as a string (normally '1'), which is then passed into WP_Network::__construct(), resulting in the id property being populated as a string. Therefore this check:

( defined( 'SITE_ID_CURRENT_SITE' ) && (int) SITE_ID_CURRENT_SITE === $this->id )

will always return false, as $this->id will be a string. Either $this->id should also be cast as an integer, or the WP_Network::__construct() method should perform type enforcement, e.g. by calling the same class's __set() method to set properties.

Change History (2)

This ticket was mentioned in PR #5369 on WordPress/wordpress-develop by RobjS.


15 months ago
#1

  • Keywords has-patch added

Before: when the WP_Network object was populated via WP_Network::get_instance(), the WPDB query would return id as a string, and the constructor would then set the id property as that string. However, elsewhere the class assumes the id will always be an integer, and strictly typed comparisons (e.g. l.242) were therefore returning an unexpected result. This resulted in multi-sites where the network URL (set via the wp_site table) did not point to the same blog as the BLOG_ID_CURRENT_SITE breaking in WordPress 6.3 and up.

Now: the constructor uses the __set() method to ensure that properties are always cast to the expected type.

#2 @robdxw
11 months ago

  • Keywords needs-testing added
Note: See TracTickets for help on using tickets.