Fixed: speed module would cause fatal errors in wp-admin on multisite networks.#306
Fixed: speed module would cause fatal errors in wp-admin on multisite networks.#306Dan0sz wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughPlugin version bumped to 1.0.2. The ChangesProxy Request Detection Hardening
Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mu-plugin/plausible-proxy-speed-module.php`:
- Around line 71-80: The current guard and docblock overstate rest_url()'s
dependency on $wp_rewrite and can cause false negatives; update the comment to
note that $wp_rewrite is only needed in specific multisite/permalink cases (via
get_rest_url()/rest_url()), and change the early-return to only bail when
$wp_rewrite is null AND one of those conditions applies (e.g., is_multisite() or
a non-empty get_option('permalink_structure')); reference get_rest_url(),
rest_url(), and the global $wp_rewrite in the updated comment and change the if
to: if ( $wp_rewrite === null && ( is_multisite() ||
get_option('permalink_structure') ) ) return false;
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52a8a81a-dc9a-4c97-989f-92c1fad1b3f8
📒 Files selected for processing (1)
mu-plugin/plausible-proxy-speed-module.php
| /** | ||
| * @see rest_url() requires $wp_rewrite to be set. If it's not set yet, just assume this isn't a proxy request. | ||
| * | ||
| * @since v1.0.2 | ||
| */ | ||
| global $wp_rewrite; | ||
|
|
||
| if ( $wp_rewrite === null ) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the relationship between rest_url() and $wp_rewrite in WordPress core
# Search for rest_url implementation and $wp_rewrite usage
rg -n -C5 'function rest_url' --type=php
# Check if there's any documentation about this dependency in WordPress core or comments
rg -n '$wp_rewrite.*rest_url|rest_url.*wp_rewrite' --type=phpRepository: plausible/wordpress
Length of output: 225
🌐 Web query:
Does WordPress rest_url() function require $wp_rewrite global to be initialized?
💡 Result:
No. WordPress’s rest_url (implemented via get_rest_url / rest_url in wp-includes/rest-api.php) does not require the global $wp_rewrite to already be initialized. Evidence: - The rest_url helper simply calls get_rest_url(null, $path, $scheme) [1]. - In get_rest_url, $wp_rewrite is only used inside the conditional branch for multisite when permalink_structure is enabled; it is referenced as a global $wp_rewrite in that branch [2][3]. - In other cases (non-multisite or permalink_structure not enabled), get_rest_url builds the URL without using $wp_rewrite at all [2][3]. - Therefore, if you are in a context where that multisite/permalinks branch is taken and $wp_rewrite is null/uninitialized, then get_rest_url would need it (so you’d want to ensure rewrite init before calling in that specific scenario). But there is no general “rest_url requires $wp_rewrite to be initialized” requirement across all call sites; the dependency is conditional [2][3]. Practical guidance: - If you’re on multisite with permalinks enabled (permalink_structure), make sure WordPress has set up $wp_rewrite before calling rest_url/get_rest_url. - Otherwise (single site, or multisite without permalink_structure), rest_url should not depend on $wp_rewrite [2][3].
Citations:
- 1: https://github.com/WordPress/wordpress-develop/blob/6.0/src/wp-includes/rest-api/class-wp-rest-server.php
- 2: https://developer.wordpress.org/reference/functions/get_rest_url/
- 3: https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/rest-api.php
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '55,95p' mu-plugin/plausible-proxy-speed-module.php | cat -nRepository: plausible/wordpress
Length of output: 1271
Refine the $wp_rewrite guard and docblock
rest_url() (via get_rest_url()) doesn’t generally require the global $wp_rewrite to be initialized—$wp_rewrite is only used in a conditional multisite/permalink_structure path. As written, returning early when $wp_rewrite === null could cause false negatives, and the current @see ... requires $wp_rewrite wording is too strong; narrow the guard or update the comment to match the conditional dependency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mu-plugin/plausible-proxy-speed-module.php` around lines 71 - 80, The current
guard and docblock overstate rest_url()'s dependency on $wp_rewrite and can
cause false negatives; update the comment to note that $wp_rewrite is only
needed in specific multisite/permalink cases (via get_rest_url()/rest_url()),
and change the early-return to only bail when $wp_rewrite is null AND one of
those conditions applies (e.g., is_multisite() or a non-empty
get_option('permalink_structure')); reference get_rest_url(), rest_url(), and
the global $wp_rewrite in the updated comment and change the if to: if (
$wp_rewrite === null && ( is_multisite() || get_option('permalink_structure') )
) return false;
There was a problem hiding this comment.
You're incorrect here. In wp-includes/rest-api.php on line 518 and 520, it says:
global $wp_rewrite;
if ( $wp_rewrite->using_index_permalinks() ) {
Since the MU plugin initializes first, in some cases, $wp_rewrite isn't initialized yet. Throwing an error when using_index_permalinks() is triggered on null.
An alternative would be to intialize $wp_rewrite, like this:
if ($wp_rewrite === null) {
include_once('some-file.php');
}
But, which file would I load there?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find wp-includes/rest-api.php around lines 515-525 to confirm the $wp_rewrite usage
# (using a vendor/wp copy if present, otherwise search for WP_Rewrite class definition)
fd -t f 'rest-api.php' | head -5
fd -t f 'class-wp-rewrite.php' | head -5
# Also check if there's a vendor WordPress copy
find . -path '*/wp-includes/rest-api.php' 2>/dev/null | head -5
find . -path '*/wp-includes/class-wp-rewrite.php' 2>/dev/null | head -5Length of output: 1998
🌐 Web query:
WordPress WP_Rewrite class file location wp-includes/class-wp-rewrite.php get_rest_url using_index_permalinks
Length of output: 19011
@Dan0sz, you're absolutely right — I apologize for the incorrect analysis. The WordPress core source confirms exactly what you described. In get_rest_url() (wp-includes/rest-api.php):
if ( is_multisite() && get_blog_option( $blog_id, 'permalink_structure' ) || get_option( 'permalink_structure' ) ) {
global $wp_rewrite;
if ( $wp_rewrite->using_index_permalinks() ) { // Fatal if $wp_rewrite is nullThere is no null check before calling ->using_index_permalinks(), so your guard is both necessary and correct.
On the "which file" question: WP_Rewrite lives in wp-includes/class-wp-rewrite.php, so the include would be:
if ( $wp_rewrite === null ) {
require_once ABSPATH . WPINC . '/class-wp-rewrite.php';
$wp_rewrite = new WP_Rewrite();
}However, I'd recommend against this approach for a couple of reasons:
WP_Rewrite::init()reads thepermalink_structureoption from the database, and at the MU plugin initialization stage, the DB connection may not yet be fully bootstrapped.- WordPress will instantiate
$wp_rewriteitself shortly after anyway (inwp-settings.php), which could conflict with or overwrite your early instance.
Your current fix — returning false early when $wp_rewrite === null — is the cleanest and safest solution. In that edge case, the speed module simply skips filtering (no optimization), which is a much better outcome than a fatal error. The current implementation is correct as-is.
🧬 Code Graph Analysis Results
mu-plugin/plausible-proxy-speed-module.php
Relevant snippet (initializes during MU plugin load and calls is_proxy_request()): lines 33-38 + 128
public function __construct() {
$this->request_uri = $this->get_request_uri();
$this->is_proxy_request = $this->is_proxy_request();
$this->init();
}
new PlausibleProxySpeed();Relevant snippet (checks $wp_rewrite before calling rest_url()): lines 58-88
private function is_proxy_request() {
$namespace = get_option( 'plausible_analytics_proxy_resources' )['namespace'] ?? '';
if ( ! $namespace ) {
return false;
}
$path = parse_url( $this->request_uri, PHP_URL_PATH );
if ( ! is_string( $path ) || $path === '' ) {
return false;
}
/**
* `@see` rest_url() requires $wp_rewrite to be set. If it's not set yet, just assume this isn't a proxy request.
*
* `@since` v1.0.2
*/
global $wp_rewrite;
if ( $wp_rewrite === null ) {
return false;
}
$expected = function_exists( 'rest_url' )
? untrailingslashit( (string) wp_parse_url( rest_url( trim( $namespace, '/' ) ), PHP_URL_PATH ) )
: '/wp-json/' . trim( $namespace, '/' );
return $path === $expected
|| str_starts_with( $path, $expected . '/' );
}✏️ Learnings added
Learnt from: Dan0sz
Repo: plausible/wordpress PR: 306
File: mu-plugin/plausible-proxy-speed-module.php:71-80
Timestamp: 2026-05-28T17:47:59.566Z
Learning: In `mu-plugin/plausible-proxy-speed-module.php` (Plausible Analytics WordPress plugin, PHP), `is_proxy_request()` guards against a `null` global `$wp_rewrite` before calling `rest_url()` / `get_rest_url()`. This is necessary because WordPress core's `get_rest_url()` (`wp-includes/rest-api.php`) calls `$wp_rewrite->using_index_permalinks()` without a null check when `permalink_structure` is set or on multisite. Since the MU plugin initializes before WordPress fully bootstraps, `$wp_rewrite` can be null at that point, causing a fatal PHP error. The correct fix is the early `return false` guard (not manually instantiating `WP_Rewrite`).
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
1 participant
Summary by CodeRabbit