fix(ci): satisfy clippy filter_next in rate-limit XFF parsing

[cursor]

Summary

Fixes the cargo build & test CI failure on PR #89 caused by clippy::filter_next (-D warnings).

Changes

• Replace .filter(|part| !part.is_empty()).next_back() with .rfind(|part| !part.is_empty()) when selecting the rightmost non-empty X-Forwarded-For hop in client_ip().

Context

Broken by: #89
Failure: https://github.com/OpenRTMP/librtmp2-server/actions/runs/29549064766/job/87787531321

  

---

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

---

Note

Medium Risk
Changes how client IPs are derived for HTTP rate limits behind proxies, which can shift throttling behavior in production; the change is intentional hardening against header spoofing.

Overview
HTTP rate limiting behind trusted proxies no longer keys buckets off the leftmost X-Forwarded-For entry (client-spoofable). For trusted peers, client_ip() now takes the rightmost non-empty hop—the address the immediate proxy appended—and does not use X-Real-IP.

Unparsable rightmost hops log a warning and fall back to the TCP peer IP. Config docs for http_trusted_proxies match this behavior. XFF parsing uses .rfind() instead of .filter().next_back() for clippy::filter_next CI. Tests cover rightmost selection, ignoring spoofed leftmost XFF, and ignoring X-Real-IP. Cargo.lock bumps tokio / tokio-macros patch versions.

^{Reviewed by Cursor Bugbot for commit c0c4d97. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_3fe1a495-96ed-408f-8496-3e85378834f8)

[AlexanderWagnerDev]

@codex review

[qodo-code-review]

PR Summary by Qodo

Fix clippy filter_next and harden trusted-proxy client IP parsing

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Fix Clippy filter_next CI failure by using rfind() for XFF parsing.
• Derive rate-limit client IP safely behind trusted proxies using X-Real-IP or rightmost XFF hop.
• Add/adjust unit tests to prevent client-controlled XFF rate-limit bypass.

Diagram

graph TD
  A["Incoming HTTP request"] --> B["TCP peer (ConnectInfo)"] --> C["client_ip()"] --> D["RateLimiter.check()"] --> E{"Allowed?"} --> F["Next handler / 429"]
  A -->|"X-Real-IP / X-Forwarded-For"| C
  G["HTTP_TRUSTED_PROXIES"] --> C

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Walk XFF from right-to-left and skip trusted proxies

• ➕ Correctly resolves the “first untrusted” client IP when multiple trusted proxies are chained.
• ➕ More robust for deployments with multiple internal hops (LB → ingress → app).
• ➖ More logic and more edge cases (mixed IPv4/IPv6, malformed hops, private ranges policy).
• ➖ Requires a clear definition of what constitutes a trusted hop beyond simple IP allowlisting.

2. Prefer standardized `Forwarded` header (RFC 7239) / use a crate

• ➕ Leverages a standard format and/or vetted parsing logic.
• ➕ Can reduce bespoke parsing and Clippy-driven micro-changes.
• ➖ Not all proxies emit Forwarded by default; operational changes may be required.
• ➖ Adds dependency/complexity if introducing a new crate just for this parsing.

Recommendation: Current approach is a good fit if HTTP_TRUSTED_PROXIES represents the immediate reverse proxy (nginx-style) and you want to rate limit by the address the proxy actually saw connect. If the project expects multi-proxy chains where the real client may be multiple hops away, consider the “skip trusted proxies from the right” variant as a follow-up hardening step.

Files changed (3) +68 / -9

Bug fix (1) +64 / -7

rate_limit.rsHarden trusted-proxy IP parsing and satisfy Clippy +64/-7

Harden trusted-proxy IP parsing and satisfy Clippy

• Updates 'client_ip()' to prefer 'X-Real-IP' when present and otherwise use the rightmost non-empty 'X-Forwarded-For' entry via 'rfind()' (fixing 'clippy::filter_next'). Adds tests covering rightmost-hop selection, ignoring client-supplied leftmost XFF entries, and 'X-Real-IP' precedence.

src/rate_limit.rs

Documentation (2) +4 / -2

.env.exampleClarify trusted proxy header behavior for rate limiting +1/-1

Clarify trusted proxy header behavior for rate limiting

• Updates the 'HTTP_TRUSTED_PROXIES' comment to indicate trusted proxies may set 'X-Real-IP' and/or 'X-Forwarded-For' for rate-limit client identification.

.env.example

config.rsDocument trusted-proxy client IP derivation semantics +3/-1

Document trusted-proxy client IP derivation semantics

• Expands 'http_trusted_proxies' docs to describe deriving the client IP from 'X-Real-IP' or the rightmost 'X-Forwarded-For' hop appended by the immediate trusted proxy.

src/config.rs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 5 rules

✅ Cross-repo context

  Not relevant to this PR: OpenRTMP/librtmp2

  Not relevant to this PR: OpenRTMP/librtmp2-server-panel

  Not relevant to this PR: AlexanderWagnerDev/nginx-obs-automatic-low-bitrate-switching

1. Silent proxy-IP fallback ✓ Resolved 🐞 Bug ☼ Reliability

Description

In client_ip(), if the TCP peer is trusted but X-Real-IP is absent/invalid and the rightmost
non-empty X-Forwarded-For token isn’t a plain IpAddr, parsing fails and the function silently falls
back to the proxy’s peer IP. This can collapse many end-clients into one rate-limit bucket with no
operator signal that header parsing is failing.

Code

src/rate_limit.rs[R165-175]

+            // Use the rightmost address: the one appended by the immediate trusted
+            // proxy ($proxy_add_x_forwarded_for), not client-controlled leftmost entries.
+            if let Some(client) = xff
+                .split(',')
+                .map(str::trim)
+                .rfind(|part| !part.is_empty())
+                .and_then(|part| part.parse::<IpAddr>().ok())
+            {
+                return client;
+            }
+        }

Relevance

⭐⭐ Medium

Rate-limit hardening accepted (PR38/56), but no precedent for logging invalid XFF/X-Real-IP; could
be deemed noisy.

PR-#38
PR-#56

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The implementation only accepts X-Real-IP if it parses as IpAddr, otherwise it proceeds to
X-Forwarded-For and only attempts to parse the single rightmost non-empty token; on any failure it
exits the trusted-proxy block and returns peer, with no logs.

src/rate_limit.rs[140-179]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`client_ip()` silently falls back to the TCP peer IP (trusted proxy IP) when proxy-derived headers are present but unparsable. This can cause unexpected rate-limit behavior (many users sharing one bucket) and is hard to diagnose.

## Issue Context
The code first tries `X-Real-IP`, then tries the rightmost non-empty `X-Forwarded-For` token. If parsing fails, it returns `peer` with no logging/diagnostic.

## Fix Focus Areas
- src/rate_limit.rs[140-179]

## Suggested fix
- When `trusted_proxies.contains(&peer)` and `X-Real-IP` is present but fails to parse as `IpAddr`, emit a warn/debug log indicating the header value was ignored.
- When `X-Forwarded-For` is present but the selected rightmost token fails to parse, emit a warn/debug log indicating the header was ignored and `peer` is being used.
- Keep the fail-closed behavior (using `peer`) unless you also introduce an explicit, well-defined trusted-proxy-chain parsing policy.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d193a047bc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Do not trust a pass-through X-Real-IP header

When an existing trusted-proxy setup safely appends or overwrites X-Forwarded-For but passes X-Real-IP through unchanged, a client can now supply an arbitrary valid X-Real-IP. Because this new branch takes precedence over the proxy-sanitized XFF value, the client can select a fresh rate-limit bucket for every request and evade per-IP limits. The previous documented contract only required trusted proxies to set XFF, so either avoid consuming this header or require a separately configured guarantee that the proxy overwrites it.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Skip trusted hops when resolving an XFF chain

When traffic crosses more than one trusted proxy, selecting the rightmost XFF entry identifies the proxy immediately upstream of the TCP peer, not the originating client. For example, client -> proxy A -> proxy B -> server produces client, proxy A at the server, so every client routed through proxy A shares one bucket and can collectively exhaust the API or stats limit. Since the configuration can contain all trusted proxy addresses, resolve from right to left while skipping trusted hops and use the first untrusted address instead.

Useful? React with 👍 / 👎.

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_7b10e5db-c272-4cd9-8b18-b1615bbac8d2)

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_66b0b2ed-82ed-4afe-9ae5-d9a6562ce9fa)

[AlexanderWagnerDev]

Addressed the review feedback:

• Codex P1 (X-Real-IP pass-through spoofing): removed X-Real-IP handling entirely. Only the rightmost X-Forwarded-For hop is trusted now, since that's the value the immediate trusted proxy actually appends ($proxy_add_x_forwarded_for); X-Real-IP isn't guaranteed to be proxy-set and could let a client pick its own rate-limit bucket. Added a regression test (trusted_proxy_ignores_x_real_ip) and updated the docs/.env.example accordingly.
• Qodo (silent fallback): added a log_warn! when a trusted proxy sends an XFF hop that fails to parse, instead of silently falling back to the proxy's own peer IP with no signal.
• Codex P2 (multi-hop chains, e.g. client -> proxy A -> proxy B -> server): not addressed here — resolving this correctly requires an explicit trusted-proxy-chain policy (walking right-to-left and skipping every trusted hop), which is a bigger design change than this bugfix PR's scope. Worth a follow-up issue if multi-proxy deployments are expected; single-hop (client -> one trusted reverse proxy -> server), the documented deployment in .env.example, is unaffected.

CI/fmt/clippy all green on the latest commit.

---

Generated by Claude Code

[qodo-code-review]

Code review by qodo was updated up to the latest commit c0c4d97