[wp-trac] [WordPress Trac] #12839: Should sandbox themes on activate to prevent fatal errors

[WordPress Trac]

#12839: Should sandbox themes on activate to prevent fatal errors
-------------------------+-----------------------------
 Reporter:  dd32         |       Owner:
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  Future Release
Component:  Themes       |     Version:  3.0
 Severity:  normal       |  Resolution:
 Keywords:  needs-patch  |     Focuses:
-------------------------+-----------------------------

Comment (by nikolov.tmw):

 Do we have to switch around all of the theme settings before attempting to
 sandbox activate a theme? I'm talking about `theme_mods`,
 `sidebars_widgets`, etc. I don't see a need to do that, since it would add
 more complexity to the sandbox activation and litter the DB with backups
 of these settings(and additional code to handle restoring them in the case
 of a failed activation).

 Is it safe to assume that if `switch_theme()` is called from within
 `'wp_ajax_customize_save'`, then the theme should be healthy(I guess so -
 since the customizer preview dies before loading when doing a theme
 preview)? If not, how would we handle failure? In plugin sandbox
 activation, we rely on a `Location:` header going through in the case of a
 failure - we can't quite use the same approach with AJAX :)

 Where would be a good place for the sandbox testing the theme, files-wise?
 I just followed the general files include sequence and it seems that `wp-
 admin/themes.php` would not be a good place, since the current theme is
 already loaded at that point(and we don't want conflicts between two
 different themes). So I'm guessing that hooking to `'setup_theme'` from
 within `wp-includes/theme.php` and checking to see if we're trying to
 sandbox test a theme would be a good place.

 I also feel like it might be a good idea to keep the current `/wp-
 admin/themes.php?action=activate` action in place and instead add a new
 one(like `/wp-admin/themes.php?action=sandbox_activate`) that we'll link
 to from the UI. The current action would be used in automatic redirects
 after a successful sandbox activation. That way the current theme will
 still be able to run any functions on the `'switch_theme'` action.

 Which brings me to another question - do we have to call the
 `'switch_theme'` action during sandbox activation? I don't know if that's
 even possible to do in a single request and I don't know if it's a good
 idea, even if it is possible(since we don't know what themes could be
 doing on that action and whether they could be removing user data).

 I'd be happy to get the ball rolling on this one, but would need some
 feedback on the above first.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/12839#comment:5>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform