Wp-rocket: Detect if paths in advanced-cache.php are correct

Created on 31 May 2019  ·  31Comments  ·  Source: wp-media/wp-rocket

Based on customer feedback:

WPR saves the absolute path of the WP installation in multiple lines of the file advanced-cache.php, for example:

if ( file_exists( 'public_html/blog/wp-content/plugins/wp-rocket/inc/vendors/classes/class-rocket-mobile-detect.php' ) && ! class_exists( 'Rocket_Mobile_Detect' ) ) {
include_once 'public_html/blog/wp-content/plugins/wp-rocket/inc/vendors/classes/class-rocket-mobile-detect.php';
}

We have a couple of incidents when the following happened:

  1. we migrate a website to a different hosting provider
  2. the website was working fine on the first examination
  3. after a few days, the website homepage was just a blank page for non-logged users
  4. the dashboard was working fine with no error message
  5. nothing in error_log or on screen
    We found out that the problem was the old path in the advanced-cache.php file.

Would it be possible / useful to detect if the path is correct, and if not, then warn the user to deactivate / reactivate WP Rocket?

Not sure if there are many cases like this one, but it could be useful when migrating sites between environments (using automated migration tools for example), to avoid issues with incorrect paths.

Related HS ticket:
https://secure.helpscout.net/conversation/866426504/109859?folderId=273761

Slack Thread: https://wp-media.slack.com/archives/C43T1AYMQ/p1559333955020300

[S] cache high bug

All 31 comments

+1
Affected by this issue when promoting a site from staging to production.

an update from the customer:

You can easily fix the path issue by using the ABSPATH WordPress variable because even he change the hosting service, the paths will be still valid.

+1 here as well. A lot of sites I run are on Pantheon.io which uses a load-balanced cloud configuration. Between URL requests, the absolute path to the server can change. If the path is tokenized to always point to the current server's path, then that will solve the problem.

@Tabrisrp What are your thoughts on using ABSPATH? It is definitely defined before advanced-cache.php and seems logical to use to me.

I'm not sure ABSPATH is reliable in all possible setups for a WP site. @Screenfeed and @hellofromtonya feedbacks on that would be useful

I'm not sure ABSPATH is reliable in all possible setups for a WP site.

Let me see if I understand the problem correctly. When this problem happens:

  • the filesystem structure is different when moving/migrating from one machine to another
  • in the advanced-cache.php file, the absolutely path to a directory or file is in string literal and based on the machine that generated the file

Looking at the code, the constants are running during file generation. The result is a string literal, i.e. not the constant, is stored in the file. So at runtime, the pathing uses a string literal instead of the constant.

For example:

https://github.com/wp-media/wp-rocket/blob/7723e789d158d2bf00ee5da49ef9158061e158eb/inc/functions/files.php#L23-L27

What happens here is this:

  • PHP extracts the value for each constant, such as WP_ROCKET_VENDORS_PATH
  • In the generated advanced-cache.php file that value is stored as a string literal

Yes, I can see that is problematic when the filesystem changes, such as moving a website to a different server or machine.

@Tabrisrp I'd suggest that we insert the constants as code instead. That way when the file runs, it programmatically determines the absolute pathing for each constant based on the machine where it's running. In other words, when generating the file, insert the constants to be a placeholder for the path.

$buffer .= "if ( file_exists( WP_ROCKET_VENDORS_PATH . 'classes/class-rocket-mobile-detect.php' ) && ! class_exists( 'Rocket_Mobile_Detect' ) ) {\n";
$buffer .= "\tinclude_once WP_ROCKET_VENDORS_PATH . 'classes/class-rocket-mobile-detect.php';\n";
$buffer .= "}\n\n";

@Tabrisrp What do you think?

I don't think WP_ROCKET_VENDORS_PATH will be defined, when used by advanced-cache.php.

I don't think WP_ROCKET_VENDORS_PATH will be defined, when used by advanced-cache.php.

Good point @Screenfeed. What if we separated WPR's constant configurations (ie into a separate file) and then load it from within WPR and advanced-cache.php?

Isn't this a snake bitting its own tail story? To include a file from the plugin, we include a file from the same plugin ^^'
Maybe we can store WP_ROCKET_VENDORS_PATH’s value in the DB (or its md5 value): then on the admin page load, compare it to the current value of the constant. If it’s different, refresh the advanced-cache.php file.

If I am not wrong, the variable that changes based on host is $rocket_path right?

Can we do something after this line?

https://github.com/wp-media/wp-rocket/blob/1ffb4f447d6f6ed8f413cec1b1c88f10beb0dcc4/inc/process-autoloader.php#L27-L29

Basically when the file doesn't exist, we know something is wrong. Maybe we can test to see if $rocket_path is still valid?

At the start of the file, before doing anything, we could have a check for $rocket_path, and if invalid, we go into
define( 'WP_ROCKET_ADVANCED_CACHE_PROBLEM', true );
which will automatically trigger a regeneration of the advanced-cache.php file whenever admin_init is fired

I like the idea, except for its order. Unless there is a reason or benefit, I recommend the check to happen only on a failure (of file_exists( $file ) for instance).

This way, there is zero performance loss (even if it's tiny or negligible). And no change for existing customers where the issue doesn't exist.

Doing the check early would prevent any chances of blank screen in case the path is incorrect

Wouldn't we know if the path is incorrect or not by the time this is executed?

https://github.com/wp-media/wp-rocket/blob/1ffb4f447d6f6ed8f413cec1b1c88f10beb0dcc4/inc/process-autoloader.php#L27-L29

I'm not sure to follow, this part of the code is not the issue here.
Adding one additional file_exists call won't change anything, we already have 9 others. Adding it early will benefit everyone in the long run

I didn't know what spl_autoload_register was. So I see why I was confusing. Thanks for the answer @Tabrisrp

Going back to Know The Code for now.

A customer requested here: https://secure.helpscout.net/conversation/1010195086/132073?folderId=273761

Using WordPress paths instead of absolute paths would resolve the problem with switching between environments.

e.g.
$rocket_path = WP_CONTENT_DIR . '/plugins/wp-rocket';

the plugins folder can be different with the WP_PLUGIN_DIR constant. It's also possible the WP Rocket directory is not named wp-rocket. We can't work based on assumptions there.

What about just this, using the __DIR__ instead of the static path? It's what I am always reverting advanced-cache.php to whenever the plugin updates it to a static path.

``` defined( 'ABSPATH' ) || exit;

define( 'WP_ROCKET_ADVANCED_CACHE', true );

if ( version_compare( phpversion(), '5.6' ) >= 0 ) {

spl_autoload_register(
    function( $class ) {
        $rocket_path    =  __DIR__ .  '/plugins/wp-rocket/';
        $rocket_classes = [
            'WP_Rocket\\Buffer\\Abstract_Buffer' => $rocket_path . 'inc/classes/Buffer/class-abstract-buffer.php',
            'WP_Rocket\\Buffer\\Cache'           => $rocket_path . 'inc/classes/Buffer/class-cache.php',
            'WP_Rocket\\Buffer\\Tests'           => $rocket_path . 'inc/classes/Buffer/class-tests.php',
            'WP_Rocket\\Buffer\\Config'          => $rocket_path . 'inc/classes/Buffer/class-config.php',
            'WP_Rocket\\Logger\\HTML_Formatter'  => $rocket_path . 'inc/classes/logger/class-html-formatter.php',
            'WP_Rocket\\Logger\\Logger'          => $rocket_path . 'inc/classes/logger/class-logger.php',
            'WP_Rocket\\Logger\\Stream_Handler'  => $rocket_path . 'inc/classes/logger/class-stream-handler.php',
            'WP_Rocket\\Traits\\Memoize'         => $rocket_path . 'inc/classes/traits/trait-memoize.php',
        ];

        if ( isset( $rocket_classes[ $class ] ) ) {
            $file = $rocket_classes[ $class ];
        } elseif ( strpos( $class, 'Monolog\\' ) === 0 ) {
            $file = $rocket_path . 'vendor/monolog/monolog/src/' . str_replace( '\\', '/', $class ) . '.php';
        } elseif ( strpos( $class, 'Psr\\Log\\' ) === 0 ) {
            $file = $rocket_path . 'vendor/psr/log/' . str_replace( '\\', '/', $class ) . '.php';
        } else {
            return;
        }

        if ( file_exists( $file ) ) {
            require $file;
        }
    }
);

if ( ! class_exists( '\WP_Rocket\Buffer\Cache' ) ) {
    if ( ! defined( 'DONOTROCKETOPTIMIZE' ) ) {
        define( 'DONOTROCKETOPTIMIZE', true ); // WPCS: prefix ok.
    }
    return;
}

$rocket_config_class = new \WP_Rocket\Buffer\Config(
    [
        'config_dir_path' => __DIR__ .  '/wp-rocket-config/',
    ]
);

( new \WP_Rocket\Buffer\Cache(
    new \WP_Rocket\Buffer\Tests(
        $rocket_config_class
    ),
    $rocket_config_class,
    [
        'cache_dir_path' => __DIR__ .  '/cache/wp-rocket/',
    ]
) )->maybe_init_process();

} else {
define( 'WP_ROCKET_ADVANCED_CACHE_PROBLEM', true );
}
```

+1 bajillion, i have run into this numerous times and didn't know what to attribute it, this explains soooooooooooooooooooooooooooooooooooooooooo much. Please fix this ASAP. I've wasted so many hours trying to diagnose and fix this issue over the last year.

I'm still thinking the approach proposed in my https://github.com/wp-media/wp-rocket/issues/1747#issuecomment-566699594 is the most simple and will work all the time.

@hellofromtonya @crystinutzaa What do you think?

I'm still thinking the approach proposed in my #1747 (comment) is the most simple and will work all the time.

Let's unpack this. The problem is the pathing may have changed since the last time the advanced-cache.php file was generated.

I think it's a combination of your comment @Tabrisrp where we check the paths to ensure they are validate and if no, then regenerate. If ! file_exists(), regenerate.

For this to work:

  • we need it as a guard clause at the top of the advanced-cache.php file` as you noted @Tabrisrp.
  • and we need to check all root key paths

What do you think?

So, at the start of the file, with file_exists() check for:

  • Rocket plugin path $rocket_path
  • Rocket config path $rocket_config_path
  • Rocket cache path $rocket_cache_path

If any of them returns false, go into the conditional containing define( 'WP_ROCKET_ADVANCED_CACHE_PROBLEM', true ); to make sure the file is regenerated as needed.

Do you agree?

@Tabrisrp 👍 I think that's a good plan. We'll validate it when we build, test, and QA.

Scope a solution
at the start of the advanced-cache.php file, with file_exists() check for:

  • Rocket plugin path $rocket_path
  • Rocket config path $rocket_config_path
  • Rocket cache path $rocket_cache_path

If any of them returns false, go into the conditional containing define( 'WP_ROCKET_ADVANCED_CACHE_PROBLEM', true ); to make sure the file is regenerated as needed.

File to update: inc/functions/files.php
Function: get_rocket_advanced_cache_file()

Effort
Effort [S]

+1 here as well. A lot of sites I run are on Pantheon.io which uses a load-balanced cloud configuration. Between URL requests, the absolute path to the server can change. If the path is tokenized to always point to the current server's path, then that will solve the problem.

+1 I'm dying on Pantheon. Would LOVE to have WP Rocket working there.

+1 This would be a great feature to have. We're using deployHQ to deploy our WordPress Websites from dev to stage and production and have the same problem with the changing paths. In our case we're additionally using the atomic deployment strategy offered by deployHQ which symlinks the web root directory to the latest release directory after each deploy. For now we're going with a "static" advanced cache file where we place the environment specific web root paths in a variable which is altered based on an environment variable from .env file. (Using the bedrock boilerplate from the roots.io guys.) Maybe this is some setup to consider when testing the new feature.

@Tabrisrp Was this a success? If so, I'm curious when this goes public. Hoping to try it on Pantheon. Thanks!

Was this page helpful?
0 / 5 - 0 ratings