Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch WSODs and provide a means for recovery for end users #3

Closed
wants to merge 73 commits into from

Conversation

@felixarntz
Copy link
Member

commented Jul 30, 2018

This is the PR accompanying https://core.trac.wordpress.org/ticket/44458.

This is only for development, discussion should continue to happen on the Trac ticket as usual.

@felixarntz

This comment has been minimized.

Copy link
Member Author

commented Jul 30, 2018

The initial commit here is based on the state of 44458.6.diff.

@felixarntz felixarntz changed the title Port-over 44458.6.diff from Trac. Catch WSODs and provide a means for recovery for end users Jul 30, 2018

schlessera added 13 commits Aug 19, 2018
Add redirection on protected endpoints
When we are on a protected endpoint, we redirect to the same page after we paused a plugin.

This has two effects:

* The protected endpoints are always immediately available, they don't show the error message on first encounter.
* The flow is exactly the same, whether only one plugin or multiple plugins are broken. So, if you update PHP and ten plugins break at once, you still get into the admin backend with one click, and the ten plugins will be paused already.
src/wp-admin/plugins.php Outdated Show resolved Hide resolved
schlessera added 3 commits Oct 1, 2018
@felixarntz

This comment has been minimized.

Copy link
Member Author

commented Oct 1, 2018

I'm not entirely sure about dbe5bc0 yet (see https://core.trac.wordpress.org/ticket/44458#comment:33), we might need to discuss this further.

felixarntz added 7 commits Oct 2, 2018
felixarntz and others added 22 commits Jan 7, 2019
Revert "Skip pausing of security-related plugins to avoid opening up …
…an attack vector"

This reverts commit 5d38162 as it
requires another discussion.
* @return string Error message HTML output.
*/
protected function get_error_message_markup() {
if ( ! function_exists( '__' ) ) {

This comment has been minimized.

Copy link
@spacedmonkey

spacedmonkey Jan 9, 2019

Why not call wp_load_translations_early(); here?

This comment has been minimized.

Copy link
@schlessera

schlessera Jan 9, 2019

Member

Not sure, but that sounds like it might be too risky for a shutdown handler...

This comment has been minimized.

Copy link
@felixarntz

felixarntz Jan 9, 2019

Author Member

I agree with @schlessera. We might not even have the database initialized and thus might not even know which locale to use. Stubbing __() is not risky in that regard.

This comment has been minimized.

Copy link
@spacedmonkey

spacedmonkey Jan 9, 2019

But that is the point of the function, it doesn’t call the database or cache, but does it best to load translations. It loads in wp_db class super early. This function is designed exactly for this...

This comment has been minimized.

Copy link
@spacedmonkey

spacedmonkey Jan 9, 2019

Stubbing __() is safe, but it also means that the translation do not work at all. Trying to support translations if possible.

@TimothyBJacobs

This comment has been minimized.

Copy link
Collaborator

commented Jan 9, 2019

I opened #4 as requested to implement an opt-out mechanism via a plugin header.

@felixarntz

This comment has been minimized.

Copy link
Member Author

commented Jan 9, 2019

This has been fixed (see https://core.trac.wordpress.org/ticket/44458#comment:56). Further work should happen through separate Trac tickets and patches or PRs.

@felixarntz felixarntz closed this Jan 9, 2019

@felixarntz felixarntz deleted the 44458 branch Jan 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.