fix: Client IP resolving when proxy reusing TCP connection#3043
fix: Client IP resolving when proxy reusing TCP connection#3043kuznetsss merged 4 commits intoXRPLF:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes incorrect client-IP resolution when a trusted proxy reuses the same backend TCP connection for requests originating from different clients.
Changes:
- Update
ProxyIpResolver::resolveClientIpto returnstd::optional<std::string>(only returns a value when the request is trusted and contains a parseableForwardedclient IP). - Track whether a connection has been identified as coming from a trusted proxy, and re-extract the client IP on subsequent requests.
- Add unit tests covering proxy connection reuse with same and different client IPs (sequential + parallel processing).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/web/ng/impl/ConnectionHandler.cpp |
Reworks per-request client IP resolution and adds “proxy connection” tracking. |
src/web/ng/Connection.hpp |
Adds isProxyConnection_ flag and accessors to connection metadata. |
src/web/impl/HttpBase.hpp |
Mirrors the “proxy connection” tracking logic for the legacy HTTP stack. |
src/web/ProxyIpResolver.hpp |
Changes resolver API to return std::optional<std::string> and updates docs. |
src/web/ProxyIpResolver.cpp |
Implements new std::optional semantics for trusted vs untrusted requests. |
tests/unit/web/ng/impl/ConnectionHandlerTests.cpp |
Adds tests for proxy TCP-connection reuse across requests. |
tests/unit/web/ProxyIpResolverTests.cpp |
Updates tests to match new optional-return resolver semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3043 +/- ##
===========================================
+ Coverage 81.81% 81.88% +0.06%
===========================================
Files 396 396
Lines 15827 17258 +1431
Branches 8403 8565 +162
===========================================
+ Hits 12949 14131 +1182
- Misses 1670 1830 +160
- Partials 1208 1297 +89 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/web/ProxyIpResolver.hpp:77
extractClientIpnow intentionally prefers the lastfor=occurrence (viarfind) to support multi-hop/append semantics, but the doc comment still reads like it extracts “the”forparameter per RFC 7239. Please update the comment to clarify that when multiplefor=values are present, the last one is used (and why), so callers don’t assume the first/original client is returned.
* @brief Extracts the client IP from the `Forwarded` HTTP header.
*
* The `Forwarded` header is expected to be in the format specified by RFC 7239.
* This function looks for the `for` parameter.
*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Proxy may reuse TCP connections to send HTTP requests from different clients. This PR fixes bug when Clio resolves client IP only once per connection.