File Editor: Make fatal error detection more robust for edits to PHP files

[westonruter]

Follow-up on #21622.

I found that on sites that have HTTP Basic auth enabled, the loopback request to do a fatal error scrape can fail, causing a bad PHP file edit to not be rolled back and instead appear as successful. So we should pass basic auth in error scraping loopback requests in addition to cookies.

Additionally, there are a couple other things to make it more robust:

1. Set a timeout for the loopback requests to make sure that the spawning process runs longer than the spawned processes.
2. Output needle on success too as a way confirm request went through successfully.
3. Check response code and if not 200 then rollback the file change.

See ​https://wordpress.slack.com/archives/C02RQBWTW/p1507181594000012

[pixolin]

Hello,

I'm testing 4.9-beta1-41785 and get following reproducable issue:

I go to Appearance => Editor, Select functions.php of theme Twenty Seventeen (or any other), delete a closing bracket and Update File, my change gets confirmed with a message "File edited successfully.". If I then leave the editor and open any other webpage in front- or backend, I receive a

Fatal error: syntax error, unexpected ';' in /Users/bego/MAMP/dev/wp-content/themes/twentyseventeen/functions.php on line 16

While this is the expected result due to the code change, it breaks the website and can only be fixed by chaning the file via FTP. The fatal error was not detected in the theme editor.

If I go back in browsing history, I can see the file editor and replace the missing bracket, but get an error message "save error" (which looks like a correct error message, as the website is broken).

[westonruter]

@pixolin Thanks for the details, and that this happened to you locally. The problem is probably that your WP install is unable to successfully make the loopback request. This will be fixed with the proposed fixes I have above, but it will then mean that you wouldn't be able to make any PHP change at all because it would not be able to make a successful loopback request to check for the fatal error. So in that case we'll probably need to show a notice saying that it was not able to communicate with the site, and that the PHP file change must be uploaded via FTP instead.

[westonruter]

@pixolin Would you test 42102.0.diff​?

[pixolin]

Replying to westonruter:

@pixolin Would you test 42102.0.diff​?

Tested the patch: works perfectly'''

After intentionally adding a syntax error and saving the changes I get an error message that my "code changes were rolled back due to an error" pointing exactly at the wrong syntax. I can revert the change, save and then continue to work. Thank you.

---

Ups. Didn't want to close the issue – just signaling, it works for me. Sorry.

[westonruter]

Pass along HTTP Basic credentials in loopback requests

[westonruter]

In 41805:

File Editor: Increase robustness of fatal error checking when saving PHP file edits.

• Increase PHP execution time limit prior to issuing loopback requests where are themselves given timeouts to ensure PHP file can be reverted.
• Output scrape messages on success and failure so that absence of either can also be flagged as an error condition.
• Forward browser's HTTP Basic Auth credentials in loopback requests to admin and home URL.
• Display more helpful message when loopback request fails.

Amends [41721].
See #21622.
Fixes #42102.