Repository navigation
Add the MAx Cache extension - #1409
tkalimullin-cloudlinux wants to merge 1 commit into
Conversation
3c8b41b to
211ccfa
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
211ccfa to
919241d
Compare
| } | ||
|
|
||
| return ''; | ||
| } |
There was a problem hiding this comment.
Both builds refuse nginx requests
Medium Severity
When both MAx Cache builds are installed, get_mode() locks to apache, so is_nginx() is false. A request whose SERVER_SOFTWARE is nginx then fails get_web_server_unsupported_reason() even though nginx_module_installed() is true. The extension reports it cannot serve and never falls back to the nginx build, which is the cPanel/CloudLinux case the notify path already special-cases.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 919241d. Configure here.
919241d to
d075e55
Compare
d075e55 to
739f97b
Compare
739f97b to
afd0c6c
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Reviewed by Cursor Bugbot for commit afd0c6c. Configure here.
| if ( \is_admin() || \wp_doing_cron() || ( \defined( 'WP_CLI' ) && WP_CLI ) ) { | ||
| // config_save() drives both passes with a Config instance no earlier refresh has seen; re-entrant by design. | ||
| Extension_MaxCache_Core::refresh( $w3tc_config ); | ||
| } |
There was a problem hiding this comment.
Cron refresh stores torn-file verdict
Low Severity
The rules filter calls refresh() on cron and WP-CLI, and refresh() writes whatever deep_unsupported_reason() sees into the hour-long transient. A concurrent .htaccess write can look like an unfinished MAx Cache block, so that torn read is stored as blocked. Front-end requests then treat the site as unsupported for up to an hour.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit afd0c6c. Configure here.
…he module Gate + rules writer + admin notices + NGINX daemon notify (Extension_MaxCache_*). PgCache_Environment's cookie/UA/cache-path helpers made public for reuse. Util_WpFile: file_put_contents() returning 0 read as failure - fixed.
afd0c6c to
ffc490b
Compare
There was a problem hiding this comment.
Thanks for the extension. Not ready to merge on ffc490b: three findings, highest first.
I ran the standalone suite on that commit (php tests/test-maxcache-core.php, 32 passed, and php tests/test-pgcache-rules-required-filter.php, 10 passed). Travis, phpcs, semgrep, and conftest are green.
1. High — Disk: Enhanced rules stay after the module block is written
w3tc_pgcache_rules_required in Extension_MaxCache_Plugin.php (around line 56) withholds W3TC rewrite rules only when is_enabled() and is_delivering() are both true. Root_Environment runs PgCache_Environment before the MAx Cache listener (priority 100). On the pass that first writes the block, delivery is still false, so the Disk: Enhanced block is written, and then note_delivering(true) in Extension_MaxCache_Environment.php (around line 211) records that the module is serving. Nothing in that request removes the Disk: Enhanced block.
The next removal is a later wp-admin rules pass, and only when "Verify rewrite rules" is on and the request is a W3TC admin screen or plugins.php. With that checkbox off, both blocks stay until the next settings save.
Activation is the sharp case. Extensions_Util::activate_extension() calls Config::save(), which does not run the environment fix, and the handler redirects before admin_notices. The extensions page that follows is the request that writes both blocks.
A local check seeded the W3TC page-cache core markers, called fix_on_event('config_change'), and found both the core markers and the MAx Cache markers still in the file while the filter already returned false. Test [19] checks that filter return value and does not check that the earlier block was removed.
Fix: after note_delivering(true), remove the page-cache rewrite rules in that same request.
Test: seed the core markers, run handlers in Root_Environment order, and assert that only the MAx Cache block remains.
2. Medium — nginx request refused when both builds are installed
Extension_MaxCache_Core::get_mode() (around line 149) locks to apache when both builds are installed, so is_nginx() is false. get_web_server_unsupported_reason() (around line 243) then refuses a request whose SERVER_SOFTWARE is nginx, even though the nginx build is installed. Test [3] pins the Apache-only build.
Fix: when the nginx build is installed and the live server header is nginx, treat that request as nginx.
Test: set both install flags and SERVER_SOFTWARE to nginx, and expect a supported verdict.
3. Low — cron refresh can store an unfinished-file refusal for an hour
On cron and WP-CLI, w3tc_pgcache_rules_required calls refresh() (Extension_MaxCache_Plugin.php around line 51). refresh() stores whatever deep_unsupported_reason() returns (Extension_MaxCache_Core.php around line 667), including a rules file that has the begin marker and no end marker (foreign_block_reason() around line 437). That blocked row is kept for an hour. Test [15] reads get_unsupported_reason() against an already-good transient and does not call refresh() after the partial write.
Fix: do not replace a clear stored verdict with an unfinished-block read.
Test: store a clear verdict, write a begin-without-end file, call refresh() the way the cron filter does, and assert the stored row stays unblocked.
Happy to re-review once these are updated.
2 participants
Description
Adds an extension that serves the Disk: Enhanced page cache through the CloudLinux MAx Cache module (
mod_maxcacheon Apache,ngx_http_maxcache_moduleon NGINX) where it is installed, answering a cached request before PHP starts. How the cache is built does not change - the same files, in the same place, with the same names; only the last step, handing a cached file to the visitor, moves to a server module written for that one job.Where the module is present and the configuration is one it can reproduce, it appears on the Extensions page and, once turned on, a
<IfModule maxcache_module>block is added to the file W3TC already writes, describing the cache path, exclusions and encoding suffixes, and the correspondingRewriteRule/RewriteCondlines are withheld through the existingw3tc_pgcache_rules_requiredfilter: exactly one component ever answers a request. Turning it off, removing the module from the host, or changing the configuration into one it can no longer reproduce reverses that on its own, without further action.A configuration it cannot reproduce exactly is refused rather than approximated, with the reason shown on the Extensions page - among them: Page Cache not set to Disk: Enhanced, multisite, an older module version than the integration needs, a Browser Cache setting the module cannot carry
on what it serves (an expiry header, a dropped ETag, Cache-Control), a cache directory outside what the web server can address, and a page-cache layout (query strings, dynamic cookies, file name) the directive block cannot express as written.
Two helpers already used internally by W3TC's own rule generation (
PgCache_Environment::reject_cookies(),reject_user_agents()) are made public so the extension names the same visitors W3TC's own rules would, rather than keeping a second copy that could drift. On NGINX, where the module reads its configuration from shared memory rather than from.htaccess, a short-lived local socket message tells the configuration daemon the file changed; a daemon that is not there is simply an unsuccessful notification.How to test
The extension looks for two things: a file saying the module is installed, and a second one saying it is new enough to expand W3TC's cache-file tokens - support for that landed in 1.2.6, which is also the version currently shipping. On Apache those are
.version/.version-1.2.6; on NGINX,.nginx-version/.nginx-version-1.2.6; both under/opt/cloudlinux/maxcache/. Installing the module on CloudLinux OS (yum install ea-apache24-mod_maxcache/ea-nginx-maxcache) writes both. Anywhere else, faking them works the same way:mkdir -p /opt/cloudlinux/maxcache touch /opt/cloudlinux/maxcache/.version /opt/cloudlinux/maxcache/.version-1.2.6the extension offers itself identically either way; only the final delivery differs.
.htaccessgains a block naming the cache path and exclusions; W3TC's ownRewriteRulelines for page cache are withheld.New dependencies
None - no new Composer package, no HTTP request, no cron job.
Note
High Risk
Changes who serves page cache and mutates
.htaccess/rule ownership; mis-handoff or a bad directive could break caching or the whole site on Apache line-limit errors, though the gate refuses unsafe configs.Overview
Adds a MAx Cache extension that, on supported CloudLinux hosts, writes a marked
<IfModule maxcache_module>block into the same rules file W3TC already manages and withholds W3TC’s Disk: Enhanced rewrite rules viaw3tc_pgcache_rules_requiredwhen delivery is confirmed—so only one layer serves cached HTML.The integration includes eligibility gating (host/module version, multisite, Browser Cache quirks, directive size/pattern safety, foreign blocks in
.htaccess), NGINX daemon reload over a unix socket when rules change, admin Extensions listing and dismissible notes, and refactorsPgCache_Environment::reject_cookies()/reject_user_agents()(plus publicapache_cache_uri_path()) so module directives match W3TC’s own exclusions.Also fixes
Util_WpFile::write_to_file()treating an empty successful write as failure, and adds standalone tests for block lifecycle, preview/saved config, and safety invariants.Reviewed by Cursor Bugbot for commit ffc490b. Bugbot is set up for automated code reviews on this repo. Configure here.