fix(client-ip): Fall back to peer IP instead of returning 500 #2003
No reviewers
Labels
No labels
Abandoned
Blocked
Bug
Changelog
Added
Changelog
Missing
Changelog
None
Cherry-picking
Database
Dependencies
Dependencies/Renovate
Difficulty
Easy
Difficulty
Hard
Difficulty
Medium
Documentation
Enhancement
Good first issue
Help wanted
Inherited
Matrix/Administration
Matrix/Appservices
Matrix/Auth
Matrix/Client
Matrix/Core
Matrix/E2EE
Matrix/Federation
Matrix/Hydra
Matrix/MSC
Matrix/Media
Matrix/T&S
Merge
Merge/Manual
Merge/Squash
Meta
Meta/CI
Meta/Packaging
Priority
Blocking
Priority
High
Priority
Low
Security
Status
Confirmed
Status
Duplicate
Status
Invalid
Status
Needs Investigation
Support
bob the builder
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
continuwuation/continuwuity!2003
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "eleboucher/continuwuity:fix/client-ip-fallback"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The
axum-client-ipextractor returns a 500 whenever the header setrequest_ip_sourceis absent, duplicated, or unparseable. This breaks proxies that don't emit it on every request , which only setsX-Envoy-External-Addressfor requests it treats as external, turning much of sync and federation into 500s.The client IP is only used for last-seen, logging, and rate limiting, so a 500 is the wrong failure mode. This adds a
ClientIpwrapper that falls back to the connection peer address instead, logging the reason once. Call sites only change their import.Fixes: #1986
Pull request checklist:
mainbranch, and the branch is named something other thanmain.myself, if applicable. This includes ensuring code compiles.
@ -0,0 +46,4 @@return;}warn!(%rejection, "Could not resolve client IP from request_ip_source; using peer address");nit: I'd argue this should be a
debug_warn!()instead of a one-time warning@ -0,0 +55,4 @@use super::*;const PEER: SocketAddr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(198, 51, 100, 7)), 8448);nit: If we're adding unit tests for this, could we also test against IPv6 for completeness' sake?
4829d122f4d3877c71e7d3877c71e7506288415frequest_ip_source#1985