Make WordPress Core

#59677 closed defect (bug) (fixed)

Incorrect theme returned when switching to blog in multisite subsite

Reported by: codex-m's profile codex-m Owned by: flixos90's profile flixos90
Milestone: 6.4 Priority: normal
Severity: major Version: 6.4
Component: Networks and Sites Keywords: has-patch has-unit-tests dev-reviewed
Focuses: multisite, performance Cc:

Description

I think I have found issue with the coming WordPress 6.4. I tested with latest WordPress 6.4 RC1.

Environment

PHP Version: 8.0
Operating system: Ubuntu 20.04

Test code to reproduce the issue (e.g. save this as reproduce.php):

<?php
ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
ini_set('error_reporting', E_ALL);

require_once('wp-load.php');

switch_to_blog(2);
$template_path = get_template_directory();
$stylesheet_path = get_stylesheet_directory();
restore_current_blog();

echo "PARENT THEME: {$template_path}";
echo "<br />";
echo "CHILD THEME: {$stylesheet_path}";

How to reproduce the issue:

1.) Setup a fresh WordPress multisite.
2.) Create a new subsite (BLOG ID: 2)
3.) Activate a non-default theme (with parent and child theme) subsite BLOG ID: 2. For example, in this testing I use Astra parent and child theme).

4.) Run the test script on the main site by calling it as:

https://yourmultisite.tld/reproduce.php

Test result:

1.) WordPress 6.3.2 OUTPUT:

PARENT THEME: /home/emerson/sourcecode/subdomaintest/wp-content/themes/astra
CHILD THEME: /home/emerson/sourcecode/subdomaintest/wp-content/themes/astra-child

2.) Latest WordPress 6.4 RC1 OUTPUT:

PARENT THEME: /home/emerson/sourcecode/subdomaintest/wp-content/themes/twentytwentythree
CHILD THEME: /home/emerson/sourcecode/subdomaintest/wp-content/themes/twentytwentythree

Test summary:

I expected to see the actual theme used in the subsite (blog ID: 2) instead I got the currently activated main site theme (incorrect).

Solution

I think the issue is that in WordPress 6.4, the result of these functions (get_template_directory() & get_stylesheet_directory()) are now stored into global variables for performance reasons. However the issue is that in multisite - these can be set first before the blog is switched thus preventing to return the correct value once the blog is switched.

I think we can fix this by checking if the call is made in multisite and if the blog is already switched. Once switched, we unset these global variables so it will return the correct values.

This is only my suggestion to fix this issue. Please attached patch. Thanks!

Attachments (3)

patch.diff (789 bytes) - added by codex-m 10 months ago.
Suggested fix
patch2.diff (785 bytes) - added by codex-m 10 months ago.
Revised - fixed some PHP notice.
patch3.diff (827 bytes) - added by codex-m 10 months ago.
Remove redundant is_multisite check and function exist check

Download all attachments as: .zip

Change History (14)

@codex-m
10 months ago

Suggested fix

@codex-m
10 months ago

Revised - fixed some PHP notice.

@codex-m
10 months ago

Remove redundant is_multisite check and function exist check

#1 @hellofromTonya
10 months ago

  • Milestone changed from Awaiting Review to 6.4

Moving into 6.4 milestone for investigation.

@clarkeemily @flixos90 @joemcgill for awareness.

#2 @joemcgill
10 months ago

  • Focuses performance added

This ticket was mentioned in PR #5532 on WordPress/wordpress-develop by @jeremyfelt.


10 months ago
#3

  • Keywords has-patch has-unit-tests added

This nulls the global wp_template_path and wp_stylesheet_path data whenever a site is switched or restored so that the next call to get_stylesheet_directory() or get_template_directory() will provide fresh data.

See https://core.trac.wordpress.org/ticket/59677

Trac ticket: https://core.trac.wordpress.org/ticket/59677

#4 @jeremyfelt
10 months ago

It seems like @codex-m has the right idea in patch3.diff, but I think we can move the nulling of that data to switch_to_blog() and restore_current_blog() so that it's only cleared once instead of every time get_stylesheet_directory() or get_template_directory() is called.

I've added an attempt at this in PR#5532 with some tests that clarify what's happening.

#5 @flixos90
10 months ago

  • Owner set to flixos90
  • Status changed from new to reviewing

Thank you for the report @codex-m, great catch. And thanks for the PR @jeremyfelt. I think that solution looks good, unsetting these globals when switching the site should be the most efficient solution to that bug.

#6 @flixos90
10 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 56974:

Multisite: Ensure that switching sites resets the current theme directory globals.

The globals introduced in [56635] to cache the current theme directories in memory were not considering switching sites in a multisite network. This changeset addresses the bug including test coverage.

Props codex-m, jeremyfelt.
Fixes #59677.
See #18298.

#7 @flixos90
10 months ago

  • Keywords dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backporting [56974] to the 6.4 branch.

@costdev @joemcgill Could one of you maybe give this a second sign-off?

#9 @codex-m
10 months ago

Thank you guys! Looks good to me. I will do another quick check with our plugins compatibility for WP 6.4 once this is available https://core.svn.wordpress.org/branches/6.4/

Cheers.

#10 @joemcgill
10 months ago

  • Keywords dev-reviewed added; dev-feedback removed

[56974] looks good to backport in my opinion. Thanks @flixos90!

#11 @flixos90
10 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 56986:

Multisite: Ensure that switching sites resets the current theme directory globals.

The globals introduced in [56635] to cache the current theme directories in memory were not considering switching sites in a multisite network. This changeset addresses the bug including test coverage.

Props codex-m, jeremyfelt, costdev, joemcgill.
Merges [56974] to the 6.4 branch.
Fixes #59677.
See #18298.

Note: See TracTickets for help on using tickets.