echo icon indicating copy to clipboard operation
echo copied to clipboard

`extractIP` may return empty IP if `RemoteAddr` has no port ( `SplitHostPort` fallback suggestion)

Open convto opened this issue 8 months ago • 3 comments

The extractIP function currently uses net.SplitHostPort to parse http.Request.RemoteAddr and extract the IP address.

This works well when RemoteAddr is in the form "host:port", but SplitHostPort returns an error (and an empty host) if the port is missing — which is an intentional design choice in Go. In such cases, the extractIP function ends up returning an empty string.

Relevant code: https://github.com/labstack/echo/blob/master/ip.go#L221-L224 Introduced in: https://github.com/labstack/echo/commit/124825ee629f32aade886f1aeb76e0c6f70c7faa

This behavior can lead to issues in environments where RemoteAddr does not include a port such as "192.0.2.10". In such cases, extractIP returns an empty string, which causes functions like RealIP() or ExtractIPFromXForwardedFor() to behave unexpectedly (e.g. returning an empty IP or skipping IP trust checks).

Suggested improvement:

Instead of returning an empty string when SplitHostPort fails, we propose falling back to the original RemoteAddr value — possibly with a simple validation using net.ParseIP.

func extractIP(req *http.Request) string {
    host, _, err := net.SplitHostPort(req.RemoteAddr)
    if err != nil {
        if net.ParseIP(req.RemoteAddr) != nil {
            return req.RemoteAddr
        }
        return ""
    }
    return host
}

This approach improves robustness when RemoteAddr lacks a port. Alternatively, using a regular expression to extract the IP part may also work, but parsing it with net.ParseIP is likely sufficient.

Let me know if this makes sense — happy to submit a PR if it would be helpful.

convto avatar Mar 21 '25 03:03 convto