Skip to content

Conversation

jamescrosswell
Copy link
Collaborator

Resolves #1309:

Summary

The X-Forwarded-For header is supposed to preserve a chain of IP addresses that the message was forwarded via (see docs).

So for example it might contain:

X-Forwarded-For: 203.0.113.195, 2001:db8:85a3:8d3:1319:8a2e:370:7348

Which is a specific instance of:

X-Forwarded-For: <client>, <proxy>, …, <proxyN>

Previously, the SentryTunnelMiddleware was overwriting this value so that it only included the IP address from DefaultHttpContext.Connection.RemoteIpAddress... which was fine if the request didn't include any X-Forwarded-For but, if the request was proxied (e.g. by CloudFlare) then effectively by doing that we delete the upstream IP addresses so we no longer know where the message came from (or the User's real IP address).

Solution

This PR changes the behaviour of the SentryTunnelMiddleware so that the RemoteIpAddress is appended to the X-Forwarded-For so that the client IP address is retained in the left most address from the list in the X-Forwarded-For header.

@jamescrosswell jamescrosswell marked this pull request as ready for review July 23, 2025 21:07

return string.IsNullOrEmpty(existingForwardedFor)
? clientIp
: $"{existingForwardedFor}, {clientIp}";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Header Malformation with Empty Values

The CreateXForwardedForHeader method malforms X-Forwarded-For headers when the incoming header contains only empty string values. This occurs because StringValues.ToString() returns a comma-only string (e.g., ',') for empty values, which string.IsNullOrEmpty() incorrectly evaluates as non-empty. This results in malformed headers like ', 192.168.1.1' or ', , 192.168.1.1' instead of only the client IP.

Locations (1)

Fix in CursorFix in Web

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In context:

if (existingForwardedFor.Count == 0)
{
return clientIp;
}
return string.IsNullOrEmpty(existingForwardedFor)
? clientIp
: $"{existingForwardedFor}, {clientIp}";

So existingForwardedFor will never be empty when this codepath is executed.

@jamescrosswell jamescrosswell merged commit 060d446 into main Jul 25, 2025
46 of 47 checks passed
@jamescrosswell jamescrosswell deleted the x-forward-for branch July 25, 2025 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sentry.Tunnel doesn't include X-Forwarded-To
3 participants