Skip to content

fix: resolve RealIP for bare RemoteAddr without port#3054

Merged
aldas merged 1 commit into
labstack:masterfrom
sonnemusk:fix/realip-bare-remote-addr
Jul 23, 2026
Merged

fix: resolve RealIP for bare RemoteAddr without port#3054
aldas merged 1 commit into
labstack:masterfrom
sonnemusk:fix/realip-bare-remote-addr

Conversation

@sonnemusk

Copy link
Copy Markdown
Contributor

Summary

Context.RealIP() (and the LegacyIPExtractor fallback) used net.SplitHostPort and ignored the error, so a bare IP RemoteAddr (no :port) returned an empty string.

Change

  • Fall back through extractIP, the same helper used by ExtractIPDirect.
  • Add unit tests for bare IPv4 and IPv6 RemoteAddr values.

Test plan

  • go test . -run TestContext_RealIP

RealIP and LegacyIPExtractor discarded the address when net.SplitHostPort
failed (e.g. bare IPv4/IPv6 without :port). Reuse extractIP so behavior
matches ExtractIPDirect.
@aldas

aldas commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

In which environment request.RemoteAddr has only IP? When I am debugging with the actual http server - it has port?

@sonnemusk

Copy link
Copy Markdown
Contributor Author

Good question — under a normal net/http TCP server, RemoteAddr is always host:port.

Where a bare IP shows up in practice:

  1. Manually built / rewritten requests — unit tests, internal sub-requests, and some gateway adapters set RemoteAddr to just an IP (or an IPv6 address without brackets/port).
  2. Consistency with ExtractIPDirect — that helper already falls back correctly when SplitHostPort fails (net.ParseIP). The default RealIP path (no IPExtractor) previously returned "" in the same situation, so callers that rely on the default got a different result than ExtractIPDirect.

So this is less “production TCP omits the port” and more “make the documented fallback robust and consistent with ExtractIPDirect”. Happy to narrow the PR description to that if you prefer.

@sonnemusk
sonnemusk force-pushed the fix/realip-bare-remote-addr branch from e98a6ab to c0d7fc3 Compare July 22, 2026 07:56
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.34%. Comparing base (dcb05f0) to head (c0d7fc3).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3054      +/-   ##
==========================================
- Coverage   93.34%   93.34%   -0.01%     
==========================================
  Files          43       43              
  Lines        4735     4733       -2     
==========================================
- Hits         4420     4418       -2     
  Misses        192      192              
  Partials      123      123              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aldas
aldas merged commit a45f07d into labstack:master Jul 23, 2026
10 checks passed
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.

2 participants