Fix merge conflicts in https://github.com/plausible/wordpress/pull/300#307
Fix merge conflicts in https://github.com/plausible/wordpress/pull/300#307Dan0sz wants to merge 13 commits into
Conversation
… in Proxy.php and URL length check in MU plugin Agent-Logs-Url: https://github.com/crweiner/plausible-wordpress-plugin/sessions/b35d7e5f-5764-4ad1-9b52-a62192c6ac6b Co-authored-by: crweiner <23106097+crweiner@users.noreply.github.com>
…es return a 404 so you cant assume an endpoint exists based on response
Added hardening to the host matching and also to ensure that all rout…
- Guard n/d/u with isset + is_string before string ops to avoid TypeError on non-string payloads (which would surface as 500s and defeat the uniform-404 design). - Use === '' instead of empty() for n and u so legitimate values like 0 aren't rejected. - Strip scheme and www. from the home_url() fallback in get_expected_domain() so it returns a bare domain matching Helpers::get_domain(), preventing silent rejection of valid payloads on sites without a configured domain_name setting.
|
Warning Review limit reached
More reviews will be available in 25 minutes. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds cached proxy config and an early mu-plugin short-circuit that rejects non-conforming proxy paths; implements multi-stage validation (method, JSON type, provenance, size, payload, host/domain normalization) in both mu-plugin and REST Proxy, hides proxy routes from REST discovery, and centralizes uniform JSON no-route errors. ChangesProxy Request Hardening with Early Detection and Validation
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 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❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
@coderabbitai review this PR |
|
✅ Action performedReview finished.
|
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mu-plugin/plausible-proxy-speed-module.php (1)
283-284:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing filter application for max body size creates inconsistency with REST layer.
The REST-side validation in
src/Proxy.phpusesapply_filters('plausible_analytics_proxy_max_body_bytes', self::MAX_REQUEST_BYTES)to allow configuration of the body size limit. This mu-plugin uses the hardcoded constant directly, so if someone uses the filter to adjust the limit, the two validation layers will enforce different thresholds.🔧 Suggested fix
private function request_body_too_large() { - return strlen( $this->get_request_body() ) > self::MAX_REQUEST_BYTES; + $max_request_bytes = (int) apply_filters( 'plausible_analytics_proxy_max_body_bytes', self::MAX_REQUEST_BYTES ); + + return $max_request_bytes > 0 && strlen( $this->get_request_body() ) > $max_request_bytes; }🤖 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 283 - 284, The request_body_too_large() method uses the hardcoded self::MAX_REQUEST_BYTES, causing mismatch with the REST layer; update request_body_too_large() to obtain the limit via the same filter used in src/Proxy.php by calling apply_filters('plausible_analytics_proxy_max_body_bytes', self::MAX_REQUEST_BYTES) and compare strlen($this->get_request_body()) against that filtered value so both layers enforce the same configurable max body size.
🧹 Nitpick comments (1)
mu-plugin/plausible-proxy-speed-module.php (1)
363-364: 💤 Low valueConsider using
str_starts_with()for consistency.The rest of the file uses
str_starts_with()(e.g., lines 93, 215), but this line uses the olderstrpos(...) === 0pattern.♻️ Suggested refactor
- if ( ! $host && strpos( $url, '/' ) === 0 ) { + if ( ! $host && str_starts_with( $url, '/' ) ) {🤖 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 363 - 364, The condition checking whether $url starts with '/' uses strpos(...) === 0 inconsistently; in the same file other checks use str_starts_with(). Update the check inside the function in plausible-proxy-speed-module.php (the block that currently reads if ( ! $host && strpos( $url, '/' ) === 0 ) ) to use str_starts_with($url, '/') instead, keeping the existing !$host guard and return true behavior so the logic remains identical while matching the file's style.
🤖 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.
Outside diff comments:
In `@mu-plugin/plausible-proxy-speed-module.php`:
- Around line 283-284: The request_body_too_large() method uses the hardcoded
self::MAX_REQUEST_BYTES, causing mismatch with the REST layer; update
request_body_too_large() to obtain the limit via the same filter used in
src/Proxy.php by calling
apply_filters('plausible_analytics_proxy_max_body_bytes',
self::MAX_REQUEST_BYTES) and compare strlen($this->get_request_body()) against
that filtered value so both layers enforce the same configurable max body size.
---
Nitpick comments:
In `@mu-plugin/plausible-proxy-speed-module.php`:
- Around line 363-364: The condition checking whether $url starts with '/' uses
strpos(...) === 0 inconsistently; in the same file other checks use
str_starts_with(). Update the check inside the function in
plausible-proxy-speed-module.php (the block that currently reads if ( ! $host &&
strpos( $url, '/' ) === 0 ) ) to use str_starts_with($url, '/') instead, keeping
the existing !$host guard and return true behavior so the logic remains
identical while matching the file's style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 526cd647-a498-48b1-aaac-4d4c84e4db7b
📒 Files selected for processing (1)
mu-plugin/plausible-proxy-speed-module.php
This PR fixes the merge conflicts in #300
Summary by CodeRabbit