Skip to content

Conversation

@umputun
Copy link
Member

@umputun umputun commented Dec 22, 2025

Summary

  • Change header priority to: X-Real-IP → CF-Connecting-IP → X-Forwarded-For (leftmost) → RemoteAddr
  • Only accept public IPs from headers; skip private/loopback/link-local IPs
  • Add isPublicIP() and parseRemoteAddr() helpers for cleaner code
  • Update godoc and README documentation

Problem

With Cloudflare/CDN setups, X-Forwarded-For looks like <client>, <cdn-edge>. The old rightmost-IP logic returned the CDN edge server IP instead of the actual client IP, causing audit logs to show inconsistent, meaningless IPs.

Breaking change

X-Forwarded-For parsing now uses leftmost (not rightmost) public IP. This is intentional for CDN compatibility but changes behavior for non-CDN setups that relied on rightmost selection.

Related to #40

change header priority to: X-Real-IP → CF-Connecting-IP → X-Forwarded-For (leftmost) → RemoteAddr. only accept public IPs from headers, skip private/loopback/link-local. this fixes issue where Cloudflare edge server IPs were returned instead of actual client IPs.

Related to #40
Copilot AI review requested due to automatic review settings December 22, 2025 21:37
@umputun umputun self-assigned this Dec 22, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the RealIP middleware to properly handle Cloudflare and other CDN setups by changing the X-Forwarded-For parsing strategy from rightmost to leftmost public IP selection. The changes ensure that the actual client IP is extracted rather than intermediate CDN/proxy IPs.

Key changes:

  • Modified header priority: X-Real-IP → CF-Connecting-IP → X-Forwarded-For (leftmost) → RemoteAddr
  • Added public IP validation to skip private/loopback/link-local addresses
  • Refactored code with new helper functions isPublicIP() and parseRemoteAddr()

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
realip/real.go Core implementation changes: new header priority logic, added isPublicIP() and parseRemoteAddr() helper functions, changed to leftmost public IP selection for X-Forwarded-For
realip/real_test.go Comprehensive test suite rewrite covering all header priorities, IPv4/IPv6 scenarios, public/private IP filtering, and CDN use cases
middleware.go Updated RealIP middleware documentation to reflect new header priority and leftmost selection behavior
README.md Enhanced documentation explaining header priority order and CDN compatibility
nocache.go Minor comment capitalization fixes for consistency
logger/logger.go Minor comment capitalization fixes for consistency
benchmarks_test.go Minor comment capitalization fixes for consistency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

{name: "CF-Connecting-IP public", headers: map[string]string{"CF-Connecting-IP": "8.8.8.8"}, wantIP: "8.8.8.8"},
{name: "CF-Connecting-IP private falls through", remoteAddr: "1.2.3.4:1234",
headers: map[string]string{"CF-Connecting-IP": "10.0.0.1"}, wantIP: "1.2.3.4"},
{name: "CF-Connecting-IP IPv6 public", headers: map[string]string{"CF-Connecting-IP": "2001:db8::1"}, wantIP: "2001:db8::1"},
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The IPv6 address 2001:db8::/32 is reserved for documentation (RFC 3849) and is not a valid public/routable IP address. This test should use an actual public IPv6 address instead, such as 2606:4700:4700::1111 (Cloudflare DNS) or 2001:4860:4860::8888 (Google DNS).

Copilot uses AI. Check for mistakes.
// RemoteAddr fallback tests
{name: "RemoteAddr IPv4 with port", remoteAddr: "192.0.2.1:1234", wantIP: "192.0.2.1"},
{name: "RemoteAddr IPv4 without port", remoteAddr: "127.0.0.1", wantIP: "127.0.0.1"},
{name: "RemoteAddr IPv6 with port", remoteAddr: "[2001:db8::1]:1234", wantIP: "2001:db8::1"},
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The IPv6 address 2001:db8::/32 is reserved for documentation (RFC 3849) and is not a valid public/routable IP address. This test should use an actual public IPv6 address instead, such as 2606:4700:4700::1111 (Cloudflare DNS) or 2001:4860:4860::8888 (Google DNS).

Suggested change
{name: "RemoteAddr IPv6 with port", remoteAddr: "[2001:db8::1]:1234", wantIP: "2001:db8::1"},
{name: "RemoteAddr IPv6 with port", remoteAddr: "[2606:4700:4700::1111]:1234", wantIP: "2606:4700:4700::1111"},

Copilot uses AI. Check for mistakes.
{name: "Cloudflare with CF-Connecting-IP", headers: map[string]string{"CF-Connecting-IP": "203.0.113.195", "X-Forwarded-For": "203.0.113.195, 172.70.231.89"}, wantIP: "203.0.113.195"},

// RemoteAddr fallback tests
{name: "RemoteAddr IPv4 with port", remoteAddr: "192.0.2.1:1234", wantIP: "192.0.2.1"},
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The IP address 192.0.2.1 is from the TEST-NET-1 range (192.0.2.0/24) which is reserved for documentation purposes per RFC 5737. This test case should use an actual public IP address instead, such as 1.1.1.1 (Cloudflare DNS) or 8.8.8.8 (Google DNS).

Suggested change
{name: "RemoteAddr IPv4 with port", remoteAddr: "192.0.2.1:1234", wantIP: "192.0.2.1"},
{name: "RemoteAddr IPv4 with port", remoteAddr: "1.1.1.1:1234", wantIP: "1.1.1.1"},

Copilot uses AI. Check for mistakes.
headers: map[string]string{"X-Real-IP": "192.168.1.1"}, wantIP: "1.2.3.4"},
{name: "X-Real-IP loopback falls through", remoteAddr: "1.2.3.4:1234",
headers: map[string]string{"X-Real-IP": "127.0.0.1"}, wantIP: "1.2.3.4"},
{name: "X-Real-IP IPv6 public", headers: map[string]string{"X-Real-IP": "2001:db8::1"}, wantIP: "2001:db8::1"},
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The IPv6 address 2001:db8::/32 is reserved for documentation (RFC 3849) and is not a valid public/routable IP address. This test should use an actual public IPv6 address instead, such as 2606:4700:4700::1111 (Cloudflare DNS) or 2001:4860:4860::8888 (Google DNS). The current test will pass but doesn't accurately test the real-world behavior of the isPublicIP function with actual public IPv6 addresses.

Copilot uses AI. Check for mistakes.
Comment on lines +48 to +49
{name: "XFF IPv6 public", headers: map[string]string{"X-Forwarded-For": "2001:db8::1"}, wantIP: "2001:db8::1"},
{name: "XFF IPv6 mixed", headers: map[string]string{"X-Forwarded-For": "::1, fc00::1, 2001:db8::1"}, wantIP: "2001:db8::1"},
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The IPv6 addresses 2001:db8::/32 are reserved for documentation (RFC 3849) and are not valid public/routable IP addresses. These tests should use actual public IPv6 addresses instead, such as 2606:4700:4700::1111 (Cloudflare DNS) or 2001:4860:4860::8888 (Google DNS).

Suggested change
{name: "XFF IPv6 public", headers: map[string]string{"X-Forwarded-For": "2001:db8::1"}, wantIP: "2001:db8::1"},
{name: "XFF IPv6 mixed", headers: map[string]string{"X-Forwarded-For": "::1, fc00::1, 2001:db8::1"}, wantIP: "2001:db8::1"},
{name: "XFF IPv6 public", headers: map[string]string{"X-Forwarded-For": "2606:4700:4700::1111"}, wantIP: "2606:4700:4700::1111"},
{name: "XFF IPv6 mixed", headers: map[string]string{"X-Forwarded-For": "::1, fc00::1, 2606:4700:4700::1111"}, wantIP: "2606:4700:4700::1111"},

Copilot uses AI. Check for mistakes.
Comment on lines +145 to +146
{name: "public IPv6", ip: "2001:db8::1", public: true},
{name: "public IPv6 full", ip: "2001:0db8:85a3:0000:0000:8a2e:0370:7334", public: true},
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The IPv6 addresses 2001:db8::/32 are reserved for documentation (RFC 3849) and are not valid public/routable IP addresses. These test cases should use actual public IPv6 addresses instead, such as 2606:4700:4700::1111 (Cloudflare DNS) or 2001:4860:4860::8888 (Google DNS). The current tests will pass but don't accurately validate the isPublicIP function's behavior with real public IPv6 addresses.

Suggested change
{name: "public IPv6", ip: "2001:db8::1", public: true},
{name: "public IPv6 full", ip: "2001:0db8:85a3:0000:0000:8a2e:0370:7334", public: true},
{name: "public IPv6", ip: "2606:4700:4700::1111", public: true}, // Cloudflare DNS
{name: "public IPv6 full", ip: "2001:4860:4860:0000:0000:0000:0000:8888", public: true}, // Google DNS (expanded)

Copilot uses AI. Check for mistakes.
Comment on lines +62 to +63
{name: "Cloudflare chain leftmost is client", headers: map[string]string{"X-Forwarded-For": "203.0.113.195, 172.70.231.89"}, wantIP: "203.0.113.195"},
{name: "Cloudflare with CF-Connecting-IP", headers: map[string]string{"CF-Connecting-IP": "203.0.113.195", "X-Forwarded-For": "203.0.113.195, 172.70.231.89"}, wantIP: "203.0.113.195"},
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The IP address 203.0.113.195 is from the TEST-NET-3 range (203.0.113.0/24) which is reserved for documentation purposes per RFC 5737. These test cases should use actual public IP addresses instead, such as 1.1.1.1 (Cloudflare DNS) or 8.8.8.8 (Google DNS), to accurately test real-world scenarios.

Suggested change
{name: "Cloudflare chain leftmost is client", headers: map[string]string{"X-Forwarded-For": "203.0.113.195, 172.70.231.89"}, wantIP: "203.0.113.195"},
{name: "Cloudflare with CF-Connecting-IP", headers: map[string]string{"CF-Connecting-IP": "203.0.113.195", "X-Forwarded-For": "203.0.113.195, 172.70.231.89"}, wantIP: "203.0.113.195"},
{name: "Cloudflare chain leftmost is client", headers: map[string]string{"X-Forwarded-For": "1.1.1.1, 172.70.231.89"}, wantIP: "1.1.1.1"},
{name: "Cloudflare with CF-Connecting-IP", headers: map[string]string{"CF-Connecting-IP": "1.1.1.1", "X-Forwarded-For": "1.1.1.1, 172.70.231.89"}, wantIP: "1.1.1.1"},

Copilot uses AI. Check for mistakes.
// public IPv4
{name: "public 8.8.8.8", ip: "8.8.8.8", public: true},
{name: "public 1.1.1.1", ip: "1.1.1.1", public: true},
{name: "public 203.0.113.1", ip: "203.0.113.1", public: true},
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The IP address 203.0.113.1 is from the TEST-NET-3 range (203.0.113.0/24) which is reserved for documentation purposes per RFC 5737. This test case should use an actual public IP address instead, such as 1.1.1.1 (Cloudflare DNS) or 8.8.8.8 (Google DNS).

Suggested change
{name: "public 203.0.113.1", ip: "203.0.113.1", public: true},
{name: "public 9.9.9.9", ip: "9.9.9.9", public: true},

Copilot uses AI. Check for mistakes.
@umputun umputun merged commit b8cec47 into master Dec 22, 2025
8 checks passed
@umputun umputun deleted the realip-cdn-support branch December 22, 2025 21:46
@coveralls
Copy link

Pull Request Test Coverage Report for Build 20444653006

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 87.976%

Totals Coverage Status
Change from base Build 20278657700: 0.0%
Covered Lines: 739
Relevant Lines: 840

💛 - Coveralls

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.

3 participants