Experimental Request Handler breaks WP Rocket

[RafaelKr]

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:

• public/content/advanced-cache.php (generated) calls wp-media/wp-rocket@09708aa/inc/classes/Buffer/class-cache.php::maybe_init_process()
• maybe_init_process calls ob_start( [ $this, 'maybe_process_buffer' ] );
• Some time later maybe_process_buffer is called with an empty string https://github.com/wp-media/wp-rocket/blob/09708aa55d362a9535ad4d45cb68280325efaecb/inc/classes/Buffer/class-cache.php#L256

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

Steps to reproduce

• Add ACORN_ENABLE_EXPERIMENTAL_WORDPRESS_REQUEST_HANDLER=true to .env
• composer install wp-media/wp-rocket
• Activate WP Rocket (By default it will add define( 'WP_CACHE', true ); // Added by WP Rocket to wp-config.php, but that should be fine for testing. Can be disabled via https://docs.wp-rocket.me/article/958-how-to-use-wp-rocket-with-bedrock-wordpress-boilerplate)
• Go to public/content/cache/wp-rocket/your-domain.localhost (or the wp-content path which is configured without Radicle)
• Load a page and see that there are no generated files
• Remove ACORN_ENABLE_EXPERIMENTAL_WORDPRESS_REQUEST_HANDLER=true from .env
• Load a page and now it should generate static files

System info

No response

Log output

No response

Please confirm this isn't a support request.

Yes

[RafaelKr]

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

[RafaelKr]

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.

[Log1x]

Care to get a PR up for testing?

[RafaelKr]

Hey @Log1x I just created #423

Can you please also test this thoroughly with your current state of Acorn 5? I can only confirm that my patches work correctly in our production project with v4.3.0

[romainhe]

Hey ! Any news on this ? :)

[RafaelKr]

Hey! I just had a look at the recent changes to https://github.com/roots/acorn/blob/v5.0.4/src/Roots/Acorn/Application/Concerns/Bootable.php

Especially those commits change some behavior which probably make my adjustments in #423 incompatible:

• https://github.com/roots/acorn/commit/e7a15b880e2309d9c8e7c272ceca346a813aaa32
• https://github.com/roots/acorn/commit/03f0d7d88ac3ff566a24214ebaa09aa27bdb3bdb
• https://github.com/roots/acorn/commit/8b1dc374a7aa8ea8fe8f2218531d831e1e0fb11c

BUT, maybe they already fix this issue in another way. I didn't test it, but by looking at the code I could imagine that output buffers are now handled in the correct order, so they work with Caching plugins.

cc @Log1x did you maybe test compatibility with WP Rocket?

[Log1x]

I didn't test it, but curious to know if there's any difference now.

[romainhe]

I had this issue with V5 and WP Rocket. Sorry, I should have been more specific in my first message...

For now, keeping only the modification to the registerDefaultRoute function seems to work correctly. Both modifications together caused a blank page and I haven't looked into it further, to be honest...