From 6fed4dae0160e95fa4f649c3528c39fd2bd7f23d Mon Sep 17 00:00:00 2001 From: Bero Date: Tue, 10 Dec 2024 10:51:46 +0100 Subject: [PATCH 01/23] Prevent plugin activation error if plugin redirects during activation --- .../src/lib/steps/activate-plugin.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 0d08958340..ca279733ad 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -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 @@ -74,7 +75,25 @@ export const activatePlugin: StepHandler = 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. + */ + 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. ` + From 2f45f25675de70fc6571aafb4f1a1d0116c9fade Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Fri, 13 Dec 2024 16:34:14 +0100 Subject: [PATCH 02/23] [Blueprints] Prevent WSOD when autologin is enabled and a plugin logs a notice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR ensures the autologin step won't cause a perpetual white screen of death when one of the plugins displays a notice. The autologin flow consists of a bunch of checks, setcookie() call, a header('Location') call, and an `exit;`. However, when the headers are already sent, the cookies won't be set, the redirect won't work, but the script will still. This, effectively, prevents loading any WordPress page. This PR adds a `headers_sent()` check to stop the autologin step early in those scenarios when we can't finish the login. ## Testing instructions * CI has a new E2E test – confirm everything is green * Try this Blueprint. It should open a white screen with errors. Now change the landing page to "/". It should open the homepage with a few errors at the top. Before this PR, you'd see a blank page instead of the homepage. ```json { "plugins": [ "pronamic-ideal" ], "preferredVersions": { "php": "8.2", "wp": "6.7" }, "features": { "networking": true }, "login": true, "landingPage": "/wp-admin/update-core.php", "steps": [ { "step": "defineWpConfigConsts", "consts": { "WP_DEBUG": true, "WP_DEBUG_DISPLAY": true } }, { "step": "setSiteLanguage", "language": "nl_NL" } ] } ``` --- .../website/playwright/e2e/blueprints.spec.ts | 42 +++++++++++++++++++ packages/playground/wordpress/src/index.ts | 30 +++++++++++++ 2 files changed, 72 insertions(+) diff --git a/packages/playground/website/playwright/e2e/blueprints.spec.ts b/packages/playground/website/playwright/e2e/blueprints.spec.ts index 45eadef70d..f944baac40 100644 --- a/packages/playground/website/playwright/e2e/blueprints.spec.ts +++ b/packages/playground/website/playwright/e2e/blueprints.spec.ts @@ -449,3 +449,45 @@ test('should correctly redirect to a multisite wp-admin url', async ({ await expect(wordpress.locator('body')).toContainText('Escritorio'); }); }); + +test('WordPress homepage loads when mu-plugin prints a notice', async ({ + wordpress, + website, + page, +}) => { + // Load a blueprint that enables debug mode and adds a mu-plugin that prints a notice + const blueprint = { + landingPage: '/', + preferredVersions: { + wp: '6.7', + php: '8.0', + }, + steps: [ + { + step: 'defineWpConfigConsts', + consts: { + WP_DEBUG: true, + WP_DEBUG_DISPLAY: true, + }, + }, + { + step: 'writeFile', + path: '/wordpress/wp-content/mu-plugins/000-print-notice.php', + data: `ID, $user->user_login ); wp_set_auth_cookie( $user->ID ); do_action( 'wp_login', $user->user_login, $user ); + setcookie('playground_auto_login_already_happened', '1'); + /** + * Confirm that nothing in WordPress, plugins, or filters have finalized + * the headers sending phase. See the comment above for more context. + */ + if (headers_sent()) { + _doing_it_wrong('playground_auto_login', 'Headers already sent, the Playground runtime will not auto-login the user', '1.0.0'); + return; + } + /** * Reload page to ensure the user is logged in correctly. * WordPress uses cookies to determine if the user is logged in, From d1ee3c9805f833c7e57e440dd974b7da5c25e914 Mon Sep 17 00:00:00 2001 From: Bero Date: Tue, 17 Dec 2024 08:49:18 +0100 Subject: [PATCH 03/23] Use options to validate if a plugin is active --- .../src/lib/steps/activate-plugin.spec.ts | 23 +++++++ .../src/lib/steps/activate-plugin.ts | 65 ++++++++++++------- 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts index 2a059c1c9c..ba9ce895aa 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts @@ -84,4 +84,27 @@ describe('Blueprint step activatePlugin()', () => { expect(php.fileExists(createdFilePath)).toBe(true); }); + + it('should succeed when the plugin redirects during activation', async () => { + const docroot = php.documentRoot; + php.writeFile( + `${docroot}/wp-content/plugins/test-plugin.php`, + ` { + await activatePlugin(php, { + pluginPath: 'test-plugin.php', + }); + }).not.toThrow(); + }); }); diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index ca279733ad..25ba7186b0 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -1,8 +1,6 @@ import { phpVar } from '@php-wasm/util'; import { StepHandler } from '.'; import { logger } from '@php-wasm/logger'; -import { PHPResponse } from '@php-wasm/universal'; - /** * @inheritDoc activatePlugin * @example @@ -40,7 +38,7 @@ export const activatePlugin: StepHandler = async ( progress?.tracker.setCaption(`Activating ${pluginName || pluginPath}`); const docroot = await playground.documentRoot; - const result = await playground.run({ + const activatePluginResult = await playground.run({ code: ` = async ( } } - if ( null === $response ) { - die('Plugin activated successfully'); - } else if ( is_wp_error( $response ) ) { - throw new Exception( $response->get_error_message() ); + if (is_wp_error($response)) { + throw new Error($response->get_error_message()); } - - throw new Exception( 'Unable to activate plugin' ); `, }); /** - * 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 '' + * Instead of checking the plugin activation response, + * check if the plugin is active by looking at the active plugins list. + * + * Relying on the plugin activation response is not reliable because if the plugin activation + * produces any output, it will be threaded as an error. + * See WordPress source code for more details: + * https://github.com/WordPress/wordpress-develop/blob/6.7/src/wp-admin/includes/plugin.php#L733 * - * Some plugins redirect on activation. + * Because some plugins can create an output, we need to use output buffering + * to ensure the 'true' response is not polluted by other outputs. + * If the plugin activation fails, we will return the buffered output as it might + * contain more information about the failure. */ - function isValidActivationResponse(result: PHPResponse) { - return ( - (result.httpStatusCode === 200 && - result.text === 'Plugin activated successfully') || - (result.httpStatusCode >= 300 && - result.httpStatusCode < 400 && - result.text === '') - ); - } + const relativePluginPath = pluginPath.replace( + docroot + '/wp-content/plugins/', + '' + ); + const isActiveCheckResult = await playground.run({ + code: ` Date: Tue, 17 Dec 2024 08:55:32 +0100 Subject: [PATCH 04/23] Explain why we had to split into two runs --- .../blueprints/src/lib/steps/activate-plugin.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 25ba7186b0..ddbd8e6ad5 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -74,6 +74,10 @@ export const activatePlugin: StepHandler = async ( * Instead of checking the plugin activation response, * check if the plugin is active by looking at the active plugins list. * + * We have to split the activation and the check into two PHP runs + * because some plugins might redirect during activation, + * which would prevent any output that happens after activation from being returned. + * * Relying on the plugin activation response is not reliable because if the plugin activation * produces any output, it will be threaded as an error. * See WordPress source code for more details: @@ -84,16 +88,14 @@ export const activatePlugin: StepHandler = async ( * If the plugin activation fails, we will return the buffered output as it might * contain more information about the failure. */ - const relativePluginPath = pluginPath.replace( - docroot + '/wp-content/plugins/', - '' - ); const isActiveCheckResult = await playground.run({ code: ` Date: Tue, 17 Dec 2024 10:14:03 +0100 Subject: [PATCH 05/23] Revert "[Blueprints] Prevent WSOD when autologin is enabled and a plugin logs a notice" This reverts commit 2f45f25675de70fc6571aafb4f1a1d0116c9fade. --- .../website/playwright/e2e/blueprints.spec.ts | 42 ------------------- packages/playground/wordpress/src/index.ts | 30 ------------- 2 files changed, 72 deletions(-) diff --git a/packages/playground/website/playwright/e2e/blueprints.spec.ts b/packages/playground/website/playwright/e2e/blueprints.spec.ts index f944baac40..45eadef70d 100644 --- a/packages/playground/website/playwright/e2e/blueprints.spec.ts +++ b/packages/playground/website/playwright/e2e/blueprints.spec.ts @@ -449,45 +449,3 @@ test('should correctly redirect to a multisite wp-admin url', async ({ await expect(wordpress.locator('body')).toContainText('Escritorio'); }); }); - -test('WordPress homepage loads when mu-plugin prints a notice', async ({ - wordpress, - website, - page, -}) => { - // Load a blueprint that enables debug mode and adds a mu-plugin that prints a notice - const blueprint = { - landingPage: '/', - preferredVersions: { - wp: '6.7', - php: '8.0', - }, - steps: [ - { - step: 'defineWpConfigConsts', - consts: { - WP_DEBUG: true, - WP_DEBUG_DISPLAY: true, - }, - }, - { - step: 'writeFile', - path: '/wordpress/wp-content/mu-plugins/000-print-notice.php', - data: `ID, $user->user_login ); wp_set_auth_cookie( $user->ID ); do_action( 'wp_login', $user->user_login, $user ); - setcookie('playground_auto_login_already_happened', '1'); - /** - * Confirm that nothing in WordPress, plugins, or filters have finalized - * the headers sending phase. See the comment above for more context. - */ - if (headers_sent()) { - _doing_it_wrong('playground_auto_login', 'Headers already sent, the Playground runtime will not auto-login the user', '1.0.0'); - return; - } - /** * Reload page to ensure the user is logged in correctly. * WordPress uses cookies to determine if the user is logged in, From 7aa4329ac16aad648c4932f03fbaa7013efa44ae Mon Sep 17 00:00:00 2001 From: Bero Date: Tue, 17 Dec 2024 10:16:12 +0100 Subject: [PATCH 06/23] Always return activation errors --- .../src/lib/steps/activate-plugin.spec.ts | 18 ++++++++++++++++++ .../src/lib/steps/activate-plugin.ts | 6 ++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts index ba9ce895aa..a9e350e4d5 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts @@ -107,4 +107,22 @@ describe('Blueprint step activatePlugin()', () => { }); }).not.toThrow(); }); + + it('should activate the plugin if it produces output during activation', async () => { + const docroot = php.documentRoot; + php.writeFile( + `${docroot}/wp-content/plugins/test-plugin.php`, + ` { + await activatePlugin(php, { + pluginPath: 'test-plugin.php', + }); + }).not.toThrow(); + }); }); diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index ddbd8e6ad5..7af8a21b29 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -65,7 +65,7 @@ export const activatePlugin: StepHandler = async ( } if (is_wp_error($response)) { - throw new Error($response->get_error_message()); + die($response->get_error_message()); } `, }); @@ -107,8 +107,10 @@ export const activatePlugin: StepHandler = async ( `, }); + if (activatePluginResult.text) { + logger.warn(activatePluginResult.text); + } if (isActiveCheckResult.text !== 'true') { - logger.debug(activatePluginResult.text); if (isActiveCheckResult.text !== 'false') { logger.debug(isActiveCheckResult.text); } From d351a63315ee3dc94c9e43a79f6ca6825b8f6d95 Mon Sep 17 00:00:00 2001 From: Bero Date: Wed, 18 Dec 2024 07:41:56 +0100 Subject: [PATCH 07/23] Update packages/playground/blueprints/src/lib/steps/activate-plugin.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adam Zieliński --- packages/playground/blueprints/src/lib/steps/activate-plugin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 7af8a21b29..93d8e10551 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -79,7 +79,7 @@ export const activatePlugin: StepHandler = async ( * which would prevent any output that happens after activation from being returned. * * Relying on the plugin activation response is not reliable because if the plugin activation - * produces any output, it will be threaded as an error. + * produces any output, WordPress will assume it's an activation error. * See WordPress source code for more details: * https://github.com/WordPress/wordpress-develop/blob/6.7/src/wp-admin/includes/plugin.php#L733 * From cf2a5800b8bd65b88919703423b245cd17a4e080 Mon Sep 17 00:00:00 2001 From: Bero Date: Wed, 18 Dec 2024 08:01:12 +0100 Subject: [PATCH 08/23] Remove plugin dir path only from the beginning of a plugin path --- .../src/lib/steps/activate-plugin.ts | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 93d8e10551..0dbbe211a8 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -41,13 +41,13 @@ export const activatePlugin: StepHandler = async ( const activatePluginResult = await playground.run({ code: ` 'Administrator') )[0]->ID ); - $plugin_path = ${phpVar(pluginPath)}; + $plugin_path = getenv('PLUGIN_PATH'); $response = false; if (!is_dir($plugin_path)) { $response = activate_plugin($plugin_path); @@ -68,6 +68,10 @@ export const activatePlugin: StepHandler = async ( die($response->get_error_message()); } `, + env: { + PLUGIN_PATH: pluginPath, + DOCROOT: docroot, + }, }); /** @@ -91,11 +95,18 @@ export const activatePlugin: StepHandler = async ( const isActiveCheckResult = await playground.run({ code: ` = async ( die('true'); } } - die(ob_get_flush() ?? 'false'); + die(ob_get_flush() ?: 'false'); `, + env: { + DOCROOT: docroot, + PLUGIN_PATH: pluginPath, + }, }); if (activatePluginResult.text) { From a3eccc695dfc1beda227d2b670fb24da259549e7 Mon Sep 17 00:00:00 2001 From: Bero Date: Wed, 18 Dec 2024 08:23:42 +0100 Subject: [PATCH 09/23] Return error if plugin wasn't found --- packages/playground/blueprints/src/lib/steps/activate-plugin.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 0dbbe211a8..26b729a62b 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -66,6 +66,8 @@ export const activatePlugin: StepHandler = async ( if (is_wp_error($response)) { die($response->get_error_message()); + } else if (false === $response) { + die("The plugin $plugin_path does not exist."); } `, env: { From 862793498f750afca299b85cba20118483cc61a3 Mon Sep 17 00:00:00 2001 From: Bero Date: Wed, 18 Dec 2024 08:28:35 +0100 Subject: [PATCH 10/23] Update error message --- packages/playground/blueprints/src/lib/steps/activate-plugin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 26b729a62b..a87cb45e39 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -67,7 +67,7 @@ export const activatePlugin: StepHandler = async ( if (is_wp_error($response)) { die($response->get_error_message()); } else if (false === $response) { - die("The plugin $plugin_path does not exist."); + die("The activatePlugin step wasn't able to find the plugin $plugin_path."); } `, env: { From fd4cb609e48a1d1ebebc822f98b811b51fd4437a Mon Sep 17 00:00:00 2001 From: Bero Date: Wed, 18 Dec 2024 08:28:50 +0100 Subject: [PATCH 11/23] Fix linter errror --- packages/playground/blueprints/src/lib/steps/activate-plugin.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index a87cb45e39..9d1b6aaa15 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -1,4 +1,3 @@ -import { phpVar } from '@php-wasm/util'; import { StepHandler } from '.'; import { logger } from '@php-wasm/logger'; /** From 74143813f461a67e89298636839dff3c1f269621 Mon Sep 17 00:00:00 2001 From: Bero Date: Wed, 18 Dec 2024 08:35:44 +0100 Subject: [PATCH 12/23] Move activate plugin error logging before is active check --- .../src/lib/steps/activate-plugin.ts | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 9d1b6aaa15..79a6948468 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -74,6 +74,11 @@ export const activatePlugin: StepHandler = async ( DOCROOT: docroot, }, }); + if (activatePluginResult.text) { + logger.warn( + `Plugin ${pluginPath} activation printed the following bytes: ${activatePluginResult.text}` + ); + } /** * Instead of checking the plugin activation response, @@ -123,18 +128,17 @@ export const activatePlugin: StepHandler = async ( }, }); - if (activatePluginResult.text) { - logger.warn(activatePluginResult.text); + if (isActiveCheckResult.text === 'true') { + return; } - if (isActiveCheckResult.text !== 'true') { - if (isActiveCheckResult.text !== 'false') { - logger.debug(isActiveCheckResult.text); - } - throw new Error( - `Plugin ${pluginPath} could not be activated – WordPress exited with no error. ` + - `Sometimes, when $_SERVER or site options are not configured correctly, ` + - `WordPress exits early with a 301 redirect. ` + - `Inspect the "debug" logs in the console for more details.` - ); + + if (isActiveCheckResult.text !== 'false') { + logger.debug(isActiveCheckResult.text); } + throw new Error( + `Plugin ${pluginPath} could not be activated – WordPress exited with no error. ` + + `Sometimes, when $_SERVER or site options are not configured correctly, ` + + `WordPress exits early with a 301 redirect. ` + + `Inspect the "debug" logs in the console for more details.` + ); }; From 2dd659e7a01255d09f771507ebeb65112ec686f4 Mon Sep 17 00:00:00 2001 From: Bero Date: Wed, 18 Dec 2024 08:35:58 +0100 Subject: [PATCH 13/23] Add test for double plugin activation --- .../src/lib/steps/activate-plugin.spec.ts | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts index a9e350e4d5..be774fc6be 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts @@ -26,7 +26,7 @@ describe('Blueprint step activatePlugin()', () => { }); it('should activate the plugin', async () => { - const docroot = php.documentRoot; + const docroot = handler.documentRoot; php.writeFile( `${docroot}/wp-content/plugins/test-plugin.php`, ` { }); it('should detect a silent failure in activating the plugin', async () => { - const docroot = php.documentRoot; + const docroot = handler.documentRoot; php.writeFile( `${docroot}/wp-content/plugins/test-plugin.php`, ` { }); it('should run the activation hooks as a priviliged user', async () => { - const docroot = php.documentRoot; + const docroot = handler.documentRoot; const createdFilePath = docroot + '/activation-ran-as-a-priviliged-user.txt'; php.writeFile( @@ -86,7 +86,7 @@ describe('Blueprint step activatePlugin()', () => { }); it('should succeed when the plugin redirects during activation', async () => { - const docroot = php.documentRoot; + const docroot = handler.documentRoot; php.writeFile( `${docroot}/wp-content/plugins/test-plugin.php`, ` { }); it('should activate the plugin if it produces output during activation', async () => { - const docroot = php.documentRoot; + const docroot = handler.documentRoot; php.writeFile( `${docroot}/wp-content/plugins/test-plugin.php`, ` { }); }).not.toThrow(); }); + + it('should successfully complete plugin activation when the plugin is already active', async () => { + const docroot = handler.documentRoot; + php.writeFile( + `${docroot}/wp-content/plugins/test-plugin.php`, + ` { + await activatePlugin(php, { + pluginPath: 'test-plugin.php', + }); + }).not.toThrow(); + }); }); From 9b148b28dbb273346ce167ed3a57d2d907e1dc34 Mon Sep 17 00:00:00 2001 From: Bero Date: Wed, 18 Dec 2024 14:03:26 +0100 Subject: [PATCH 14/23] Ensure all supported plugin paths can be activated --- .../src/lib/steps/activate-plugin.spec.ts | 101 ++++++++++++------ .../src/lib/steps/activate-plugin.ts | 37 ++++--- 2 files changed, 94 insertions(+), 44 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts index be774fc6be..ec778c1be4 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts @@ -25,24 +25,64 @@ describe('Blueprint step activatePlugin()', () => { php = await handler.getPrimaryPhp(); }); - it('should activate the plugin', async () => { + it('should activate a plugin file located in the plugins directory', async () => { const docroot = handler.documentRoot; php.writeFile( `${docroot}/wp-content/plugins/test-plugin.php`, ` { + const docroot = handler.documentRoot; + const pluginDir = `${docroot}/wp-content/plugins/test-plugin`; + php.mkdir(pluginDir); + php.writeFile( + `${pluginDir}/test-plugin.php`, + ` { + const docroot = handler.documentRoot; + php.mkdir(`${docroot}/wp-content/plugins/test-plugin`); + php.writeFile( + `${docroot}/wp-content/plugins/test-plugin/test-plugin.php`, + ` { + const docroot = handler.documentRoot; + php.mkdir(`${docroot}/wp-content/plugins/test-plugin`); + php.writeFile( + `${docroot}/wp-content/plugins/test-plugin/index.php`, + ` { @@ -56,11 +96,10 @@ describe('Blueprint step activatePlugin()', () => { `${docroot}/wp-content/mu-plugins/0-exit.php`, ` - await activatePlugin(php, { - pluginPath: 'test-plugin.php', - }) + await expect( + activatePlugin(php, { + pluginPath: 'test-plugin.php', + }) ).rejects.toThrow(/Plugin test-plugin.php could not be activated/); }); @@ -101,14 +140,14 @@ describe('Blueprint step activatePlugin()', () => { } ); ` ); - await expect(async () => { - await activatePlugin(php, { + await expect( + activatePlugin(php, { pluginPath: 'test-plugin.php', - }); - }).not.toThrow(); + }) + ).resolves.toBeUndefined(); }); - it('should activate the plugin if it produces output during activation', async () => { + it('should activate the plugin if it produces a output during activation', async () => { const docroot = handler.documentRoot; php.writeFile( `${docroot}/wp-content/plugins/test-plugin.php`, @@ -119,14 +158,14 @@ describe('Blueprint step activatePlugin()', () => { echo 'Hello World'; ` ); - await expect(async () => { - await activatePlugin(php, { + await expect( + activatePlugin(php, { pluginPath: 'test-plugin.php', - }); - }).not.toThrow(); + }) + ).resolves.toBeUndefined(); }); - it('should successfully complete plugin activation when the plugin is already active', async () => { + it('should not throw an error if the plugin is already active', async () => { const docroot = handler.documentRoot; php.writeFile( `${docroot}/wp-content/plugins/test-plugin.php`, @@ -135,10 +174,10 @@ describe('Blueprint step activatePlugin()', () => { await activatePlugin(php, { pluginPath: 'test-plugin.php', }); - await expect(async () => { - await activatePlugin(php, { + await expect( + activatePlugin(php, { pluginPath: 'test-plugin.php', - }); - }).not.toThrow(); + }) + ).resolves.toBeUndefined(); }); }); diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 79a6948468..32739384ff 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -48,7 +48,7 @@ export const activatePlugin: StepHandler = async ( $plugin_path = getenv('PLUGIN_PATH'); $response = false; - if (!is_dir($plugin_path)) { + if ( ! is_dir( $plugin_path)) { $response = activate_plugin($plugin_path); } @@ -63,10 +63,10 @@ export const activatePlugin: StepHandler = async ( } } - if (is_wp_error($response)) { - die($response->get_error_message()); - } else if (false === $response) { - die("The activatePlugin step wasn't able to find the plugin $plugin_path."); + if ( is_wp_error($response) ) { + die( $response->get_error_message() ); + } else if ( false === $response ) { + die( "The activatePlugin step wasn't able to find the plugin $plugin_path." ); } `, env: { @@ -101,26 +101,35 @@ export const activatePlugin: StepHandler = async ( const isActiveCheckResult = await playground.run({ code: ` = async ( }, }); + console.log(isActiveCheckResult.text); + if (isActiveCheckResult.text === 'true') { return; } From 622099dc37b586df167a1eee70009cd25d39b19c Mon Sep 17 00:00:00 2001 From: Bero Date: Thu, 19 Dec 2024 07:39:47 +0100 Subject: [PATCH 15/23] Clear up is active check --- .../src/lib/steps/activate-plugin.spec.ts | 20 ++++----- .../src/lib/steps/activate-plugin.ts | 41 +++++++++++-------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts index ec778c1be4..275cb47b4d 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts @@ -55,32 +55,32 @@ describe('Blueprint step activatePlugin()', () => { ).resolves.toBeUndefined(); }); - it('should activate a plugin if the absolute plugin directory path is provided', async () => { + it('should activate a plugin if a absolute plugin path is provided', async () => { const docroot = handler.documentRoot; php.mkdir(`${docroot}/wp-content/plugins/test-plugin`); php.writeFile( - `${docroot}/wp-content/plugins/test-plugin/test-plugin.php`, + `${docroot}/wp-content/plugins/test-plugin/index.php`, ` { + it('should activate a plugin if the absolute plugin directory path is provided', async () => { const docroot = handler.documentRoot; php.mkdir(`${docroot}/wp-content/plugins/test-plugin`); php.writeFile( - `${docroot}/wp-content/plugins/test-plugin/index.php`, + `${docroot}/wp-content/plugins/test-plugin/test-plugin.php`, ` { ).rejects.toThrow(/Plugin test-plugin.php could not be activated/); }); - it('should run the activation hooks as a priviliged user', async () => { + it('should run the activation hooks as a privileged user', async () => { const docroot = handler.documentRoot; const createdFilePath = - docroot + '/activation-ran-as-a-priviliged-user.txt'; + docroot + '/activation-ran-as-a-privileged-user.txt'; php.writeFile( `${docroot}/wp-content/plugins/test-plugin.php`, ` { expect(php.fileExists(createdFilePath)).toBe(true); }); - it('should succeed when the plugin redirects during activation', async () => { + it('should activate a plugin if it redirects during activation', async () => { const docroot = handler.documentRoot; php.writeFile( `${docroot}/wp-content/plugins/test-plugin.php`, @@ -147,7 +147,7 @@ describe('Blueprint step activatePlugin()', () => { ).resolves.toBeUndefined(); }); - it('should activate the plugin if it produces a output during activation', async () => { + it('should activate a plugin if it produces a output during activation', async () => { const docroot = handler.documentRoot; php.writeFile( `${docroot}/wp-content/plugins/test-plugin.php`, diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 32739384ff..f6d50ce94d 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -103,28 +103,37 @@ export const activatePlugin: StepHandler = async ( ob_start(); require_once( getenv( 'DOCROOT' ) . "/wp-load.php" ); + /** + * Extracts the relative plugin path from either an absolute or relative plugin path. + * + * Absolute paths starting with plugin directory (e.g., '/wordpress/wp-content/plugins/test-plugin/index.php') + * should be converted to relative paths (e.g., 'test-plugin/index.php') + * + * Directories should finish with a trailing slash to ensure we match the full plugin directory name. + * + * Examples: + * - '/wordpress/wp-content/plugins/test-plugin/index.php' → 'test-plugin/index.php' + * - '/wordpress/wp-content/plugins/test-plugin/' → 'test-plugin/' + * - '/wordpress/wp-content/plugins/test-plugin' → 'test-plugin/' + * - 'test-plugin/index.php' → 'test-plugin/index.php' + * - 'test-plugin/' → 'test-plugin/' + * - 'test-plugin' → 'test-plugin/' + */ $plugin_directory = getenv( 'DOCROOT' ) . "/wp-content/plugins/"; $relative_plugin_path = getenv( 'PLUGIN_PATH' ); - if ( strpos( $relative_plugin_path, $plugin_directory ) === 0 ) { - $relative_plugin_path = substr_replace( - $relative_plugin_path, - '', - 0, - strlen( $plugin_directory ) - ); + if (strpos($relative_plugin_path, $plugin_directory) === 0) { + $relative_plugin_path = substr($relative_plugin_path, strlen($plugin_directory)); + } + if ( + pathinfo($relative_plugin_path, PATHINFO_EXTENSION) !== 'php' && + substr($relative_plugin_path, -1) !== '/' + ) { + $relative_plugin_path = $relative_plugin_path . '/'; } - - $is_relative_plugin_path_a_plugin_directory_name = ( - false === strpos( $relative_plugin_path, '/' ) && - substr( $relative_plugin_path, -strlen( '.php' ) ) !== '.php' - ); $active_plugins = get_option( 'active_plugins' ); foreach ( $active_plugins as $plugin ) { - if ( $relative_plugin_path === $plugin || ( - $is_relative_plugin_path_a_plugin_directory_name && - substr( $plugin, 0, strlen( $relative_plugin_path ) + 1 ) === $relative_plugin_path . '/' - ) ) { + if ( substr( $plugin, 0, strlen( $relative_plugin_path ) ) === $relative_plugin_path ) { ob_end_clean(); die( 'true' ); } From d7e7827a7a79c712f82a044abf2699ae3002b5b1 Mon Sep 17 00:00:00 2001 From: Bero Date: Thu, 19 Dec 2024 07:41:43 +0100 Subject: [PATCH 16/23] Add trailing slash activation test --- .../src/lib/steps/activate-plugin.spec.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts index 275cb47b4d..dc799ff8cb 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts @@ -70,7 +70,7 @@ describe('Blueprint step activatePlugin()', () => { ).resolves.toBeUndefined(); }); - it('should activate a plugin if the absolute plugin directory path is provided', async () => { + it('should activate a plugin if a absolute plugin directory path is provided', async () => { const docroot = handler.documentRoot; php.mkdir(`${docroot}/wp-content/plugins/test-plugin`); php.writeFile( @@ -85,6 +85,21 @@ describe('Blueprint step activatePlugin()', () => { ).resolves.toBeUndefined(); }); + it('should activate a plugin if a absolute plugin directory path with a trailing slash is provided', async () => { + const docroot = handler.documentRoot; + php.mkdir(`${docroot}/wp-content/plugins/test-plugin`); + php.writeFile( + `${docroot}/wp-content/plugins/test-plugin/test-plugin.php`, + ` { const docroot = handler.documentRoot; php.writeFile( From 56cd8757beb073d516cf9fbcf5627fe3b52623e4 Mon Sep 17 00:00:00 2001 From: Bero Date: Thu, 19 Dec 2024 12:58:33 +0100 Subject: [PATCH 17/23] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adam Zieliński --- packages/playground/blueprints/src/lib/steps/activate-plugin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index f6d50ce94d..658fe11561 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -146,9 +146,9 @@ export const activatePlugin: StepHandler = async ( }, }); - console.log(isActiveCheckResult.text); if (isActiveCheckResult.text === 'true') { + // Plugin activation was successful, yay! return; } From 2efc8e2e6491ba1ff371288fe63eb58ab00f061b Mon Sep 17 00:00:00 2001 From: Bero Date: Thu, 19 Dec 2024 13:15:15 +0100 Subject: [PATCH 18/23] Check if the plugin exists in the filesystem --- .../blueprints/src/lib/steps/activate-plugin.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 658fe11561..404db0f0e7 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -119,11 +119,19 @@ export const activatePlugin: StepHandler = async ( * - 'test-plugin/' → 'test-plugin/' * - 'test-plugin' → 'test-plugin/' */ - $plugin_directory = getenv( 'DOCROOT' ) . "/wp-content/plugins/"; + $plugin_directory = WP_PLUGIN_DIR . '/'; $relative_plugin_path = getenv( 'PLUGIN_PATH' ); if (strpos($relative_plugin_path, $plugin_directory) === 0) { $relative_plugin_path = substr($relative_plugin_path, strlen($plugin_directory)); } + + // Let's make sure the plugin path exists on the filesystem. + if ( ! file_exists( $plugin_directory . $relative_plugin_path ) && + ! is_dir( $plugin_directory . $relative_plugin_path ) + ) { + die( 'There is no plugin at ' . $plugin_directory . $relative_plugin_path ); + } + if ( pathinfo($relative_plugin_path, PATHINFO_EXTENSION) !== 'php' && substr($relative_plugin_path, -1) !== '/' @@ -146,7 +154,6 @@ export const activatePlugin: StepHandler = async ( }, }); - if (isActiveCheckResult.text === 'true') { // Plugin activation was successful, yay! return; From dde935a546bd03e1a6bf8b0838e93f6e6e42b365 Mon Sep 17 00:00:00 2001 From: Bero Date: Thu, 19 Dec 2024 13:16:29 +0100 Subject: [PATCH 19/23] Check if the current relative plugin path is a directory before adding a slash --- .../blueprints/src/lib/steps/activate-plugin.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 404db0f0e7..39a951bbaf 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -125,16 +125,10 @@ export const activatePlugin: StepHandler = async ( $relative_plugin_path = substr($relative_plugin_path, strlen($plugin_directory)); } - // Let's make sure the plugin path exists on the filesystem. - if ( ! file_exists( $plugin_directory . $relative_plugin_path ) && - ! is_dir( $plugin_directory . $relative_plugin_path ) - ) { - die( 'There is no plugin at ' . $plugin_directory . $relative_plugin_path ); - } - if ( pathinfo($relative_plugin_path, PATHINFO_EXTENSION) !== 'php' && - substr($relative_plugin_path, -1) !== '/' + substr($relative_plugin_path, -1) !== '/' && + is_dir( $plugin_directory . $relative_plugin_path ) ) { $relative_plugin_path = $relative_plugin_path . '/'; } From c0dd231c3f84a7dceb4140aaab61646c0b7af44a Mon Sep 17 00:00:00 2001 From: Bero Date: Thu, 19 Dec 2024 15:12:39 +0100 Subject: [PATCH 20/23] Use resolves.not.toThrow in tests --- .../src/lib/steps/activate-plugin.spec.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts index dc799ff8cb..74c1da99e0 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.spec.ts @@ -36,7 +36,7 @@ describe('Blueprint step activatePlugin()', () => { activatePlugin(php, { pluginPath: 'test-plugin.php', }) - ).resolves.toBeUndefined(); + ).resolves.not.toThrow(); }); it('should activate a plugin file located in a subdirectory of the plugins directory', async () => { @@ -52,7 +52,7 @@ describe('Blueprint step activatePlugin()', () => { activatePlugin(php, { pluginPath: `test-plugin/test-plugin.php`, }) - ).resolves.toBeUndefined(); + ).resolves.not.toThrow(); }); it('should activate a plugin if a absolute plugin path is provided', async () => { @@ -67,7 +67,7 @@ describe('Blueprint step activatePlugin()', () => { activatePlugin(php, { pluginPath: `${docroot}/wp-content/plugins/test-plugin/index.php`, }) - ).resolves.toBeUndefined(); + ).resolves.not.toThrow(); }); it('should activate a plugin if a absolute plugin directory path is provided', async () => { @@ -82,7 +82,7 @@ describe('Blueprint step activatePlugin()', () => { activatePlugin(php, { pluginPath: `${docroot}/wp-content/plugins/test-plugin`, }) - ).resolves.toBeUndefined(); + ).resolves.not.toThrow(); }); it('should activate a plugin if a absolute plugin directory path with a trailing slash is provided', async () => { @@ -97,7 +97,7 @@ describe('Blueprint step activatePlugin()', () => { activatePlugin(php, { pluginPath: `${docroot}/wp-content/plugins/test-plugin/`, }) - ).resolves.toBeUndefined(); + ).resolves.not.toThrow(); }); it('should detect a silent failure in activating the plugin', async () => { @@ -159,7 +159,7 @@ describe('Blueprint step activatePlugin()', () => { activatePlugin(php, { pluginPath: 'test-plugin.php', }) - ).resolves.toBeUndefined(); + ).resolves.not.toThrow(); }); it('should activate a plugin if it produces a output during activation', async () => { @@ -177,7 +177,7 @@ describe('Blueprint step activatePlugin()', () => { activatePlugin(php, { pluginPath: 'test-plugin.php', }) - ).resolves.toBeUndefined(); + ).resolves.not.toThrow(); }); it('should not throw an error if the plugin is already active', async () => { @@ -193,6 +193,6 @@ describe('Blueprint step activatePlugin()', () => { activatePlugin(php, { pluginPath: 'test-plugin.php', }) - ).resolves.toBeUndefined(); + ).resolves.not.toThrow(); }); }); From 0c09fc0029bd593802b59c1a0b3b6dd6f9be9bda Mon Sep 17 00:00:00 2001 From: Bero Date: Thu, 19 Dec 2024 15:13:50 +0100 Subject: [PATCH 21/23] Explain why a WP_Error during plugin activation doesn't mean the plugin isn'a active --- .../playground/blueprints/src/lib/steps/activate-plugin.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index 39a951bbaf..c675dc2b35 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -89,7 +89,9 @@ export const activatePlugin: StepHandler = async ( * which would prevent any output that happens after activation from being returned. * * Relying on the plugin activation response is not reliable because if the plugin activation - * produces any output, WordPress will assume it's an activation error. + * produces any output, WordPress will assume it's an activation error and return a WP_Error. + * WordPress will still activate the plugin and load the required page, + * but it will also show the error as a notice in wp-admin. * See WordPress source code for more details: * https://github.com/WordPress/wordpress-develop/blob/6.7/src/wp-admin/includes/plugin.php#L733 * From e27c6fb7e4ed5b2118182c8a900998b63f33bf32 Mon Sep 17 00:00:00 2001 From: Bero Date: Thu, 19 Dec 2024 15:18:32 +0100 Subject: [PATCH 22/23] Remove .php check --- packages/playground/blueprints/src/lib/steps/activate-plugin.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index c675dc2b35..c6a700ed89 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -128,7 +128,6 @@ export const activatePlugin: StepHandler = async ( } if ( - pathinfo($relative_plugin_path, PATHINFO_EXTENSION) !== 'php' && substr($relative_plugin_path, -1) !== '/' && is_dir( $plugin_directory . $relative_plugin_path ) ) { From bb24b49757cde44936c9307a84aef3bb3873faf1 Mon Sep 17 00:00:00 2001 From: Bero Date: Thu, 19 Dec 2024 15:19:41 +0100 Subject: [PATCH 23/23] Use rtrim and only is_dir --- .../playground/blueprints/src/lib/steps/activate-plugin.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts index c6a700ed89..706b0a8211 100644 --- a/packages/playground/blueprints/src/lib/steps/activate-plugin.ts +++ b/packages/playground/blueprints/src/lib/steps/activate-plugin.ts @@ -127,11 +127,8 @@ export const activatePlugin: StepHandler = async ( $relative_plugin_path = substr($relative_plugin_path, strlen($plugin_directory)); } - if ( - substr($relative_plugin_path, -1) !== '/' && - is_dir( $plugin_directory . $relative_plugin_path ) - ) { - $relative_plugin_path = $relative_plugin_path . '/'; + if ( is_dir( $plugin_directory . $relative_plugin_path ) ) { + $relative_plugin_path = rtrim( $relative_plugin_path, '/' ) . '/'; } $active_plugins = get_option( 'active_plugins' );