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 9 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

I have a issue with the ip extractor that looks verry much like this issue. However it behaves differently on ARM vs on X86_64 I solved this by using a custom function for X86_64 binaries:

func getIpFromContext(c echo.Context) net.IP {
	contextIp := c.RealIP()
	if strings.Contains(contextIp, "[") {
		endIndex := strings.Index(contextIp, "]:")
		return net.ParseIP(contextIp[1:endIndex])
	}
	if strings.Contains(contextIp, ":") {
		endIndex := strings.Index(contextIp, ":")
		return net.ParseIP(contextIp[:endIndex])
	}
	return net.ParseIP(contextIp)
}

This is used in production like this:

	var requesterIpAddress string
	frameworkExtractedIP := c.RealIP()
	customIpExtractor := getIpFromContext(c).String()

	// this is required due to differences with X86_64 and ARM implementations of the echo frameworks ip extractor.
	if ip := net.ParseIP(frameworkExtractedIP); ip != nil {
		requesterIpAddress = ip.String()
	}

	// this is required due to differences with X86_64 and ARM implementations of the echo frameworks ip extractor.
	if ip := net.ParseIP(customIpExtractor); ip != nil {
		requesterIpAddress = ip.String()
	}

To be honest this was a dirty work around but it works and it was only an issue as I tried to support ARM

Merlinux-source avatar Mar 24 '25 17:03 Merlinux-source

Echo does not differentiate any CPU architecture from each other. Echo is built on Go standard library and if there are differences - it has to originate from there.


about

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.

http.Request.RemoteAddr documentation says it is set to IP:port

	// RemoteAddr allows HTTP servers and other software to record
	// the network address that sent the request, usually for
	// logging. This field is not filled in by ReadRequest and
	// has no defined format. The HTTP server in this package
	// sets RemoteAddr to an "IP:port" address before invoking a
	// handler.
	// This field is ignored by the HTTP client.
	RemoteAddr string

In which situation/environment/OS RemoteAddr is not in that format? We can not just start to make workarounds for problems what I am having trouble to reproduce.

I am testing with this trivial example:

package main

import (
	"fmt"
	"github.com/labstack/echo/v4"
	"log/slog"
	"net/http"
)

func main() {
	e := echo.New()

	//_, trusted, _ := net.ParseCIDR("127.0.0.1/32")
	//e.IPExtractor = echo.ExtractIPFromRealIPHeader(
	//	echo.TrustIPRange(trusted),
	//)

	e.GET("/", func(c echo.Context) error {
		return c.String(http.StatusOK, fmt.Sprintf("RemoteAddr: %s\n", c.Request().RemoteAddr))
	})

	if err := e.Start(":8080"); err != nil {
		slog.Error("start server error: %v", err)
	}
}

aldas avatar Mar 24 '25 19:03 aldas

I'm sorry for jumping to conclusions to quickly, the issue I'm experiencing may not be related to this issue at all... I'm trying to reproduce my issue and managed to do it once so far, but after trying to make the setup easier to reproduce, the issue didn't occur again. I used a docker from scratch container, with SSL certificates and users copied from the go build container based on alpine. I used Caddy as a reverse proxy and enabled ipv6 support for docker. Then under some conditions c.RealIp() returns host with port. I'm still trying to debug this further, but debugging inside of a docker container is not that easy for me. if this seems related or relevant to you, you can see my progress here

Merlinux-source avatar Mar 25 '25 08:03 Merlinux-source

Fixed in commit a92f420

The extractIP function now:

  • Handles RemoteAddr without port by validating with net.ParseIP
  • Returns the IP directly as a fallback when SplitHostPort fails
  • Maintains full backwards compatibility for existing behavior

This fixes cases where RemoteAddr like "192.168.1.1" (without port) would previously return an empty string but now correctly returns "192.168.1.1".

Your suggested implementation was spot-on! Thanks for the detailed analysis and solution. 🎉

vishr avatar Sep 16 '25 00:09 vishr