Themes should be sandboxed on activation to prevent fatal errors

[dd32]

I've just made a child theme of TwentyTen by copying the folder over, renaming, and adding a Template: header to the style.css file.

Upon activation, I've been thrown to a page with a fatal error due to redefining a twentyten function.

Ideally, theme activations should be passed through a sandbox style activation the same as plugins.

[ocean90]

Related: #14752

[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.

[dd32]

We don't need to worry about any theme settings really, that's all either handled up the stack, or something we don't need to worry about.

The way the plugins sandbox works is as follows:

• You hit the Activation URL
• A redirect is issued to a failed-activation step
• The plugin is included, activation functions are run
• The plugin is marked as activated in the database
• A redirect is issued to the successful-activation step, overriding the previous failure step.

That works pretty well, and could also be done for themes. The problem we face today is that with the REST API and other AJAX endpoints, the HTTP redirection flow can't really be used as it's probably not available (Can't perform redirects like that during a REST API call for example, nor from the CLI)

One option that has been brought up for the upgraders at least, is to perform a HTTP callback to various locations (Front page, Admin page, REST API) to ensure that none of those pages are fataling and are still accessible. That has other downsides though (failing requests, Load-balanced/proxied sites, etc) not being handled well.

The way forward isn't exactly "do this", it's rather, "here's the issues, now we need to find a solution that takes it all into account". Even if it doesn't protect against the REST API/CLI in the first iteration, it's a step forward.

[nikolov.tmw]

Replying to dd32:

We don't need to worry about any theme settings really, that's all either handled up the stack, or something we don't need to worry about.

I thought that's the case, but wanted to double-check.

The way the plugins sandbox works is as follows:

• You hit the Activation URL
• A redirect is issued to a failed-activation step
• The plugin is included, activation functions are run
• The plugin is marked as activated in the database
• A redirect is issued to the successful-activation step, overriding the previous failure step.

That works pretty well, and could also be done for themes.

However, I anticipate that a single request won't be a viable option for themes. The problem I see is that while multiple plugins can be active, we can't(shouldn't) have multiple themes active in the same request. Hence why we first do a request to the sandboxing URL and if that succeeds, we do a request to the activation URL, where all of the flips and switches are currently being taken care of.

One option that has been brought up for the upgraders at least, is to perform a HTTP callback to various locations (Front page, Admin page, REST API) to ensure that none of those pages are fataling and are still accessible. That has other downsides though (failing requests, Load-balanced/proxied sites, etc) not being handled well.

I did think about HTTP callbacks as well, but there are definitely cases where those would fail(besides the points you brought, it can also fail if you're working on a remote server that doesn't have a DNS record, ie using your hosts file).

Ok, I'll work on a patch in the following week or so. It definitely won't be able to handle either the REST API, or the CLI, but it will work from the UI. Once we have that in place, we can think of possible ways to make the activation work for the API and CLI.

[melchoyce]

#41073 was marked as a duplicate.

[swissspidy]

#42523 was marked as a duplicate.

[Howdy_McGee]

This is already done with Plugins and I agree that it should be extended to Themes.