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

[Blueprints] Prevent plugin activation error if plugin redirects during activation or produces an output #2066

Merged
merged 29 commits into from
Dec 19, 2024
Merged
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6fed4da
Prevent plugin activation error if plugin redirects during activation
bgrgicak Dec 10, 2024
2ca6335
Merge branch 'trunk' into fix/1979-plugin-activation-fails-on-redirect
bgrgicak Dec 13, 2024
2f45f25
[Blueprints] Prevent WSOD when autologin is enabled and a plugin logs…
adamziel Dec 13, 2024
3d69181
Merge branch 'prevent-autologin-white-screen-of-death' into fix/1979-…
bgrgicak Dec 17, 2024
d1ee3c9
Use options to validate if a plugin is active
bgrgicak Dec 17, 2024
1747574
Explain why we had to split into two runs
bgrgicak Dec 17, 2024
75cd7b4
Merge branch 'trunk' into fix/1979-plugin-activation-fails-on-redirect
bgrgicak Dec 17, 2024
a46d412
Merge branch 'trunk' into fix/1979-plugin-activation-fails-on-redirect
bgrgicak Dec 17, 2024
7204067
Revert "[Blueprints] Prevent WSOD when autologin is enabled and a plu…
bgrgicak Dec 17, 2024
7aa4329
Always return activation errors
bgrgicak Dec 17, 2024
aaf683d
Merge branch 'trunk' into fix/1979-plugin-activation-fails-on-redirect
bgrgicak Dec 17, 2024
d255141
Merge branch 'trunk' into fix/1979-plugin-activation-fails-on-redirect
bgrgicak Dec 18, 2024
d351a63
Update packages/playground/blueprints/src/lib/steps/activate-plugin.ts
bgrgicak Dec 18, 2024
cf2a580
Remove plugin dir path only from the beginning of a plugin path
bgrgicak Dec 18, 2024
a3eccc6
Return error if plugin wasn't found
bgrgicak Dec 18, 2024
8627934
Update error message
bgrgicak Dec 18, 2024
fd4cb60
Fix linter errror
bgrgicak Dec 18, 2024
7414381
Move activate plugin error logging before is active check
bgrgicak Dec 18, 2024
2dd659e
Add test for double plugin activation
bgrgicak Dec 18, 2024
9b148b2
Ensure all supported plugin paths can be activated
bgrgicak Dec 18, 2024
622099d
Clear up is active check
bgrgicak Dec 19, 2024
d7e7827
Add trailing slash activation test
bgrgicak Dec 19, 2024
56cd875
Apply suggestions from code review
bgrgicak Dec 19, 2024
2efc8e2
Check if the plugin exists in the filesystem
bgrgicak Dec 19, 2024
dde935a
Check if the current relative plugin path is a directory before addin…
bgrgicak Dec 19, 2024
c0dd231
Use resolves.not.toThrow in tests
bgrgicak Dec 19, 2024
0c09fc0
Explain why a WP_Error during plugin activation doesn't mean the plug…
bgrgicak Dec 19, 2024
e27c6fb
Remove .php check
bgrgicak Dec 19, 2024
bb24b49
Use rtrim and only is_dir
bgrgicak Dec 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion packages/playground/blueprints/src/lib/steps/activate-plugin.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { phpVar } from '@php-wasm/util';
import { StepHandler } from '.';
import { logger } from '@php-wasm/logger';
import { PHPResponse } from '@php-wasm/universal';

/**
* @inheritDoc activatePlugin
Expand Down Expand Up @@ -74,7 +75,25 @@ export const activatePlugin: StepHandler<ActivatePluginStep> = async (
throw new Exception( 'Unable to activate plugin' );
`,
});
if (result.text !== 'Plugin activated successfully') {

/**
* A valid plugin activation response is either
* a 200 status code response with the text 'Plugin activated successfully'
* or a redirect status code (300-399) response with the text ''
*
* Some plugins redirect on activation.
Copy link
Collaborator

@adamziel adamziel Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we know it's not an error-related redirection? I wonder how wp-cli solves this. Not that we should load it just to active a plugin, but maybe there's some inspiration there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WP-CLI checks is_wp_error https://github.com/wp-cli/extension-command/blob/main/src/Plugin_Command.php#L384

I updated my code to also cover scenarios when plugins create output. It contains a explanation why I think we shouldn't rely on the is_wp_error check https://github.com/WordPress/wordpress-playground/pull/2066/files#diff-4a02b123bb6311fff3ec089ca1ade75c75f1af35aaa4f1a717f14d444fc9b485R81-R84.

*/
function isValidActivationResponse(result: PHPResponse) {
return (
(result.httpStatusCode === 200 &&
result.text === 'Plugin activated successfully') ||
(result.httpStatusCode >= 300 &&
result.httpStatusCode < 400 &&
result.text === '')
);
}

if (!isValidActivationResponse(result)) {
logger.debug(result);
throw new Error(
`Plugin ${pluginPath} could not be activated – WordPress exited with no error. ` +
Expand Down
Loading