[wp-trac] [WordPress Trac] #21622: Validate or sandbox theme file edits before saving them (as is done for plugins)

[WordPress Trac]

#21622: Validate or sandbox theme file edits before saving them (as is done for
plugins)
-------------------------------------+-----------------------------
 Reporter:  eschwartz93              |       Owner:  westonruter
     Type:  enhancement              |      Status:  accepted
 Priority:  high                     |   Milestone:  4.9
Component:  Themes                   |     Version:  2.7.1
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:  administration
-------------------------------------+-----------------------------

Comment (by johnbillion):

 Replying to [comment:30 westonruter]:
 > > * The full file path shouldn't be exposed in the error message. It
 should show the path relative to ABSPATH, for example: `str_replace(
 ABSPATH, '', $error_output )`.
 >
 > There isn't any such path scrubbing in `plugin_sandbox_scrape()`
 previously for when a plugin was edited as I could see.

 Good point. I wonder if it should be introduced there too. At the least,
 it shortens the error message and makes it more readable.

 > The actual user-generated PHP error message will get escaped as it is
 getting printed in `<pre>{{ data.message }}</pre>`. The other messages are
 getting printed unescaped in `<p class="notification-message">{{{
 data.message || data.code }}}</p>` because some (one) of the messages for
 the `file_not_writable` error code has markup in it, the link to the codex
 article.

 I think we should try to avoid this as a matter of best practice.
 Inserting untrusted HTML into the DOM isn't a great idea. The Codex link
 could be moved into the strings available on the front and and appended
 depending on the error code. Not high priority by any means, but we can do
 better.

 > > * Unrelated change in `src/wp-includes/js/wp-a11y.js`.
 >
 > It is related actually. In `theme-plugin-editor.js` there is a call to
 `wp.a11y.speak()` but static analysis was complaining about a missing
 function arg. But the arg is actually optional. So this just updates the
 jsdoc to make it explicit.

 Ah yes. Thanks.

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