[wp-trac] [WordPress Trac] #22252: register_theme_directory() usage can break the current theme

[WordPress Trac]

#22252: register_theme_directory() usage can break the current theme
-----------------------------+--------------------
 Reporter:  johnjamesjacoby  |       Owner:
     Type:  defect (bug)     |      Status:  new
 Priority:  normal           |   Milestone:  3.5
Component:  Themes           |     Version:  trunk
 Severity:  normal           |  Resolution:
 Keywords:  has-patch        |
-----------------------------+--------------------
Description changed by johnjamesjacoby:

Old description:

> '''Problem:'''
>
> It's possible for the active theme and it's roots to become out of sync
> when performing a specific set of actions, resulting in the site white-
> screening because of the active theme directory being incorrect.
>
> Hard to explain, so I've included more information below.
>
> ----
> '''Steps to reproduce:'''
> 1. Activate !BuddyPress.
> 2. Activate bp-default theme.
> 3. Deactivate !BuddyPress.
> 4. Visit Appearance > Themes. (Theme shows as 'Twenty Twelve')
> 5. Visit site: Twenty Twelve appears
> 6. Check DB: stylesheet and template are changed to 'twentytwelve'.
> 7. Check DB: stylesheet_root and template_root still point to
> '/plugins/buddypress/bp-themes/'.
> 8. Reactivate !BuddyPress.
> 9. Visit Appearance > Themes. (Theme shows as 'twentytwelve')
> 10. twentytwelve reports back as broken.
> 11. Visit site: white-screen of doom
> ----
> '''Stack:'''
>
> * wp-admin/themes.php:line 107 calls validate_current_theme()
> * wp-includes/theme.php:719 calls switch_theme()
> * wp-includes/theme.php 682 counts $wp_theme_directories, and skips the
> roots.
> * This results in the _root options pointing to '/plugins/buddypress/bp-
> themes', and the current stylesheet and template being set as
> 'twentytwelve'.
> * If you re-activate !BuddyPress, $wp_theme_directories will now be
> greater than 1, and the _root options will not get updated.
> ----
> '''Possible fixes:'''
> * Remove count( $wp_theme_directories ) check from switch_theme(). This
> forces the theme root to be accurate every time the theme switches,
> regardless of how many directories are ever registered. In my opinion,
> this is the most sensible option. (Patch attached)
> * Improve search_theme_directories() to update 'stylesheet_root' and
> 'template_root' when 'theme_roots' transient doesn't match up. This works
> because we're already looping through the $cached_roots to make sure each
> theme is valid. The problem here is we're depending on the expiration of
> the transient, and doing additional logic each time that happens. (No
> patch attached)
> ----
> The crux of the issue: counting $wp_theme_directories isn't a reliable
> way to guarantee the current theme and its _root's match. This bug
> doesn't occur if more than 1 new theme directory is registered, because
> switch_theme() will always update the theme root in that circumstance.
>
> By forcing switch_theme() to always save the roots, we're able to bypass
> the issue completely.
>
> Conversely, the only reason to store the _root's is when
> register_theme_directory() is used. If it's never used, it's never a
> problem. If it's always used, it's never a problem. It's only a problem
> when switching between it being used once, and then not at all.
>
> This results in two additional options (even when there are never
> multiple theme roots) which sucks, but it always fixes the bug.
>
> If we fix it in search_theme_directories(), we're able to update the
> _root's when they need it, but we're performing that extra logic on each
> iteration when the transients expire.

New description:

 '''Problem:'''

 It's possible for the active theme and it's roots to become out of sync
 when performing a specific set of actions, resulting in the site white-
 screening because of the active theme directory being incorrect.

 Hard to explain, so I've included more information below.

 ----
 '''Steps to reproduce:'''
 1. Activate !BuddyPress.
 2. Activate bp-default theme.
 3. Deactivate !BuddyPress.
 4. Visit Appearance > Themes. (Theme shows as 'Twenty Twelve')
 5. Visit site: Twenty Twelve appears
 6. Check DB: stylesheet and template are changed to 'twentytwelve'.
 7. Check DB: stylesheet_root and template_root still point to
 '/plugins/buddypress/bp-themes/'.
 8. Reactivate !BuddyPress.
 9. Visit Appearance > Themes. (Theme shows as 'twentytwelve')
 10. twentytwelve reports back as broken.
 11. Visit site: white-screen of doom
 ----
 '''Stack:'''

 * wp-admin/themes.php:line 107 calls validate_current_theme()
 * wp-includes/theme.php:719 calls switch_theme()
 * wp-includes/theme.php 682 counts $wp_theme_directories, and skips the
 roots.
 * This results in the _root options pointing to '/plugins/buddypress/bp-
 themes', and the current stylesheet and template being set as
 'twentytwelve'.
 * If you re-activate !BuddyPress, $wp_theme_directories will now be
 greater than 1, and the _root options will not get updated.
 ----
 '''Possible fixes:'''
 * Remove count( $wp_theme_directories ) check from switch_theme(). This
 forces the theme root to be accurate every time the theme switches,
 regardless of how many directories are ever registered. In my opinion,
 this is the most sensible option. (Patch attached)
 * Improve search_theme_directories() to update 'stylesheet_root' and
 'template_root' when 'theme_roots' transient doesn't match up. This works
 because we're already looping through the $cached_roots to make sure each
 theme is valid. The problem here is we're depending on the expiration of
 the transient, and doing additional logic each time that happens. (No
 patch attached)
 ----
 The crux of the issue: counting $wp_theme_directories isn't a reliable way
 to guarantee the current theme and its _root's match. This bug doesn't
 occur if more than 1 additional theme directory is registered, because
 switch_theme() will always update the theme root in that circumstance.

 By forcing switch_theme() to always save the roots, we're able to bypass
 the issue completely.

 Conversely, the only reason to store the _root's is when
 register_theme_directory() is used more than once. If it's only used by
 core to register '/themes', it's never a problem. If it's always used by
 several plugins, it's never a problem. It's only a problem when switching
 between it being used more than once, and then only by core.

 This results in two additional options (even when there are never multiple
 theme roots) which sucks, but it always fixes the bug.

 If we fix it in search_theme_directories(), we're able to update the
 _root's when they need it, but we're performing that extra logic on each
 iteration when the transients expire. Also, because
 search_theme_directories() has some funky sub-directory checks, it's a
 messy bit to try and hack in there.

--

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/22252#comment:8>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software