roots / acorn

Laravel components for WordPress plugins and themes
https://roots.io/acorn/
MIT License
824 stars 94 forks source link

Experimental Request Handler breaks WP Rocket #406

Open RafaelKr opened 1 month ago

RafaelKr commented 1 month ago

Version

4.3.0

What did you expect to happen?

In a Radicle project we have the experimental Request handler enabled via ACORN_ENABLE_EXPERIMENTAL_WORDPRESS_REQUEST_HANDLER=true.

We now installed WP Rocket (Previously we had WP Optimize where it also didn't work).

It should create static html files inside public/content/cache/wp-rocket/example.com

What actually happens?

It only creates those static html files when the experimental request handler is disabled.

It's related to output buffering. WP Rocket receives an empty output buffer at https://github.com/wp-media/wp-rocket/blob/09708aa55d362a9535ad4d45cb68280325efaecb/inc/classes/Buffer/class-cache.php#L259

The order of calls is:

When we disable the experimental request handler the buffer contains the correct content and the static file is generated.

Steps to reproduce

System info

No response

Log output

No response

Please confirm this isn't a support request.

Yes

RafaelKr commented 1 month ago

I created a patch which at least works with WP Rocket. I'll post it with some info later this week.

RafaelKr commented 1 month ago

The problem I identified was that the code is running in the following order:

  1. WP Rocket is initialized very early via advanced-cache.php. It calls ob_start( [ $this, 'maybe_process_buffer' ] );.
  2. Acorn Bootloader is initialized and does ob_start().
  3. Acorn removes the default shutdown function with priority 1 which is added by WordPress. It adds an own shutdown function with priority 100 where the Response is handled by the Laravel Router.
  4. WordPress did all its work and the Acorn shutdown action is called.
  5. Inside the Acorn Router it does an ob_get_clean() for every ob_get_level().
  6. WP Rockets maybe_process_buffer method is called and receives an empty buffer because it was cleaned by Acorn.

I now modified the the previous steps as follows:

The code diff below is based on Acorn 4.3.0. The comments starting with // [X] are added afterwards. X is indicating the related item from list found below the diff. To apply the patch to Acorn the comments need to be removed.

diff --git a/src/Roots/Acorn/Bootloader.php b/src/Roots/Acorn/Bootloader.php
index de0a3ca..ffdcbe4 100644
--- a/src/Roots/Acorn/Bootloader.php
+++ b/src/Roots/Acorn/Bootloader.php
@@ -3,6 +3,7 @@
 namespace Roots\Acorn;

 use Illuminate\Contracts\Foundation\Application as ApplicationContract;
+use Illuminate\Http\Request;
 use Illuminate\Http\Response;
 use Illuminate\Support\Env;
 use Illuminate\Support\Facades\Facade;
@@ -203,7 +204,7 @@ class Bootloader
     protected function registerDefaultRoute(): void
     {
         $this->app->make('router')
-            ->any('{any?}', fn () => tap(response(''), function (Response $response) {
+          // [5] Pass in request
+            ->any('{any?}', fn (Request $request) => tap(response(''), function (Response $response) use ($request) {
                 foreach (headers_list() as $header) {
                     [$header, $value] = explode(': ', $header, 2);

@@ -218,12 +219,12 @@ class Bootloader
                     $response->header('X-Powered-By', $this->app->version());
                 }

-                $content = '';
+               // [5] Get content attached to the request
+                $content = $request->request->get('wp_ob_content');

                 $levels = ob_get_level();

                 for ($i = 0; $i < $levels; $i++) {
-                    $content .= ob_get_clean();
+                   // [5] Don't clean the output_buffer
+                    $content .= ob_get_contents();
                 }

                 $response->setContent($content);
@@ -286,9 +288,20 @@ class Bootloader

         $route->middleware($isApi ? $config['api'] : $config['web']);

+        // [2] Save ob starting level
+        $startLevel = ob_get_level();
         ob_start();

         remove_action('shutdown', 'wp_ob_end_flush_all', 1);
+        // [3] Add shutdown handler to end output buffering with the same priority as default WordPress does it
+        add_action('shutdown', function () use ($request, $startLevel) {
+            // [4] Clean buffer only until start level
+            $levels = ob_get_level() - $startLevel;
+            $content = '';
+
+            for ( $i = 0; $i < $levels; $i++ ) {
+                $content .= ob_get_clean();
+            }
+
+            // [4] Save content onto request to have it available in [5]
+            $request->request->set('wp_ob_content', $content);
+        }, 1);
         add_action('shutdown', fn () => $this->handleRequest($kernel, $request), 100);
     }
  1. [No change]
  2. Before Acorn does ob_start() we save the current level.
  3. We add an additional shutdown action with the same priority as the WordPress original action (priority 1).
  4. Inside our new shutdown action we run ob_get_clean() until the level we saved before ob_start(). The content is attached to the $request. This makes sure the WP Rocket output buffer which started before Acorn is not touched. Afterwards the original Acorn shutdown action with priority 100 is called, passing the request to the Laravel Router.
  5. We now get the content attached to the request. We now loop ob_get_level() again and attach it to the content. Instead of using ob_get_clean() we use ob_get_contents().
  6. Because the buffer is not cleaned WP Rocket receives the contents and can cache them.

This change works for WP Rocket and should also work for all other plugins which have an "outer" output buffer (a buffer started before Acorn). At least when the outer buffer is handled after a shutdown action with priority 100.

I'm not 100% sure if this change could lead to duplicated output. But I wouldn't expect that as original WordPress already flushes the buffer at shutdown with priority.