WordPress.org

Make WordPress Core

Opened 13 months ago

Closed 11 months ago

Last modified 8 months ago

#46097 closed defect (bug) (invalid)

WSOD protection: network paused plugin cannot be resumed from the non-network plugins screen

Reported by: pbiron Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.1
Component: Site Health Keywords: servehappy has-patch needs-testing
Focuses: Cc:

Description (last modified by pbiron)

As reported in ticket:46060#comment:3

if a network active plugin is paused and an admin goes to the plugins screen for one of the sites in the network (not the network plugins screen), the "Resume" link for the network paused plugin doesn't actually resume the plugin. My guess is either the "resume" logic should check whether the paused plugin is network active or not and act accordingly, or that link should go to the network plugins screen to resume the plugin.

And here's a little more info that may be related:

  • if the network active plugin is paused while visiting a non-network admin screen and whatever caused the fatal is fixed before visiting the network plugins screen, then the plugin is shown as paused (i.e., has the peach background and the "Network Resume" link is there) but it does not have the paused status. Hence, when you try to resume it (even from the network plugins screen it seems to still error.
  • note: the plugin I used to test this was one that defined a main plugin class and when trying to resume, it errors with a "can't redefine class XYZ_Plugin" message...so PHP still seems to have loaded the main class even tho it is paused.

Attachments (1)

46097.diff (633 bytes) - added by afragen 13 months ago.

Download all attachments as: .zip

Change History (18)

#1 @pbiron
13 months ago

  • Description modified (diff)

#2 @afragen
13 months ago

  • Component changed from General to Plugins
  • Keywords servehappy needs-patch added
  • Version set to trunk

#3 @afragen
13 months ago

I did a little digging.

I think this happens because the resume redirect uses the site admin_url() and not the network_admin_url().

Perhaps the solution would be to not show the Resume for network active plugins when not in the network screen. Somewhere in https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-plugins-list-table.php#L670

Last edited 13 months ago by afragen (previous) (diff)

#4 follow-up: @flixos90
13 months ago

@afragen

I think this happens because the resume redirect uses the site admin_url() and not the network_admin_url().

Are you sure? Afaict it uses self_admin_url() which should use the correct version.

I'm not sure yet why this doesn't work. Regarding the approach itself, I think it makes sense to allow resuming a plugin for one specific site even if it is network-active. The error that caused it originally could have occurred due to a specific incompatibility with another plugin that is only active on that site.

#5 in reply to: ↑ 4 @afragen
13 months ago

Replying to flixos90:

@afragen

I think this happens because the resume redirect uses the site admin_url() and not the network_admin_url().

Are you sure? Afaict it uses self_admin_url() which should use the correct version.

I'm not sure yet why this doesn't work. Regarding the approach itself, I think it makes sense to allow resuming a plugin for one specific site even if it is network-active. The error that caused it originally could have occurred due to a specific incompatibility with another plugin that is only active on that site.

I'm pretty sure. I was stepping through the code the other day and that's what I found. As the site is a subsite it returned admin_url(). I'll check again later to confirm.

#6 @afragen
13 months ago

@flixos90 step through from https://core.trac.wordpress.org/browser/trunk/src/wp-admin/plugins.php#L404.

It seems that because the resume link initiates from a subsite the admin_url() is returned for self_admin_url().

@afragen
13 months ago

#7 @afragen
13 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

#8 @pbiron
13 months ago

I'll try to test the patch by tomorrow.

#9 @pbiron
13 months ago

Now I can't recreate the conditions reported in the description of this ticket :-( That is, it now seems to work even without the patch.

@afragen where you able to replicate the problem before producing your patch?

#10 @afragen
13 months ago

@pbiron Honestly I cheated. I edited the code to show the ‘Resume’ link and then followed in xDebug. I didn’t have a plugin set up to fatal. I’ll see if I can recreate the problem legitimately.

My thought was that the plugin wouldn’t be resumed from wp-admin/plugins.php only from wp-admin/network/plugins.php.

#11 @pbiron
13 months ago

I first noticed the problem while working on a network active plugin on my local machine (hit save too early :-) Maybe, the problem only happens when the plugin is paused at a certain point during a request (i.e., attached to hook that runs at a certain time)? And I don't remember exactly what part of my code I mucking with at the time (i.e., when it would have executed).

I'll try to recreate the conditions.

#12 follow-up: @afragen
13 months ago

@pbiron per #46130

Additionally, after discussion with @flixos90, we decided to drop Network Activated plugins from pausing. There are a number of things that WordPress loads between MU/Network Activated plugins being loaded and regular plugins being loaded. Most importantly to us, the cookie constants. Also from @flixos90, pausing network plugins open their whole own permission/scope can of worms and is multisite is generally run by more technical users.

It appears this may not be an issue.

#13 in reply to: ↑ 12 @pbiron
13 months ago

Replying to afragen:

It appears this may not be an issue.

Also, given the Slack discussion a little while ago about possibly reverting WSOD protection from 5.1, whether it is an issue or not might not matter immediately.

But, I wouldn't close this ticket just yet.

#14 @pento
13 months ago

  • Milestone changed from Awaiting Review to 5.2

#15 @flixos90
12 months ago

  • Milestone changed from 5.2 to 5.3

#16 @flixos90
11 months ago

  • Milestone 5.3 deleted
  • Resolution set to invalid
  • Status changed from new to closed

This ticket is based on the old fatal error recovery mode implementation and will be covered as part of #46130.

#17 @spacedmonkey
8 months ago

  • Component changed from Plugins to Site Health
Note: See TracTickets for help on using tickets.