fixes #2016 - make IP() and IPs() more reliable

app.go

@@ -367,6 +367,13 @@ type Config struct {
 	trustedProxiesMap  map[string]struct{}
 	trustedProxyRanges []*net.IPNet
 
+	// If set to true, c.IP() and c.IPs() will validate IP addresses before returning them.
+	// Also, c.IP() will return only the first valid IP rather than just the raw header
+	// WARNING: this has a performance cost associated with it.
+	//
+	// Default: false
+	EnableIPValidation bool `json:"enable_ip_validation"`
+
 	// If set to true, will print all routes with their method, path and handler.
 	// Default: false
 	EnablePrintRoutes bool `json:"enable_print_routes"`

ctx.go

@@ -649,33 +649,94 @@ func (c *Ctx) Port() string {
 }
 
 // IP returns the remote IP address of the request.
+// If ProxyHeader and IP Validation is configured, it will parse that header and return the first valid IP address.
 // Please use Config.EnableTrustedProxyCheck to prevent header spoofing, in case when your app is behind the proxy.
 func (c *Ctx) IP() string {
 	if c.IsProxyTrusted() && len(c.app.config.ProxyHeader) > 0 {
-		return c.Get(c.app.config.ProxyHeader)
+		return c.extractIPFromHeader(c.app.config.ProxyHeader)
 	}
 
 	return c.fasthttp.RemoteIP().String()
 }
 
-// IPs returns an string slice of IP addresses specified in the X-Forwarded-For request header.
-func (c *Ctx) IPs() (ips []string) {
-	header := c.fasthttp.Request.Header.Peek(HeaderXForwardedFor)
-	if len(header) == 0 {
-		return
+// validateIPIfEnabled will return the input IP when validation is disabled.
+// when validation is enabled, it will return an empty string if the input is not a valid IP.
+func (c *Ctx) validateIPIfEnabled(ip string) string {
+	if c.app.config.EnableIPValidation && net.ParseIP(ip) == nil {
+		return ""
 	}
-	ips = make([]string, bytes.Count(header, []byte(","))+1)
-	var commaPos, i int
+	return ip
+}
+
+// extractIPsFromHeader will return a slice of IPs it found given a header name in the order they appear.
+// When IP validation is enabled, any invalid IPs will be omitted.
+func (c *Ctx) extractIPsFromHeader(header string) (ipsFound []string) {
+	headerValue := c.Get(header)
+
+	// try to gather IPs in the input with minimal allocations to improve performance
+	ips := make([]string, bytes.Count([]byte(headerValue), []byte(","))+1)
+	var commaPos, i, validCount int
 	for {
-		commaPos = bytes.IndexByte(header, ',')
+		commaPos = bytes.IndexByte([]byte(headerValue), ',')
 		if commaPos != -1 {
-			ips[i] = utils.Trim(c.app.getString(header[:commaPos]), ' ')
-			header, i = header[commaPos+1:], i+1
+			ips[i] = c.validateIPIfEnabled(utils.Trim(headerValue[:commaPos], ' '))
+			if ips[i] != "" {
+				validCount++
+			}
+			headerValue, i = headerValue[commaPos+1:], i+1
 		} else {
-			ips[i] = utils.Trim(c.app.getString(header), ' ')
-			return
+			ips[i] = c.validateIPIfEnabled(utils.Trim(headerValue, ' '))
+			if ips[i] != "" {
+				validCount++
+			}
+			break
 		}
 	}
+
+	// filter out any invalid IP(s) that we found
+	if len(ips) == validCount {
+		ipsFound = ips
+	} else {
+		ipsFound = make([]string, validCount)
+		var validIndex int
+		for n := range ips {
+			if ips[n] != "" {
+				ipsFound[validIndex] = ips[n]
+				validIndex++
+			}
+		}
+	}
+	return
+}
+
+// extractIPFromHeader will attempt to pull the real client IP from the given header when IP validation is enabled.
+// currently, it will return the first valid IP address in header.
+// when IP validation is disabled, it will simply return the value of the header without any inspection.
+func (c *Ctx) extractIPFromHeader(header string) string {
+	if c.app.config.EnableIPValidation {
+		// extract all IPs from the header's value
+		ips := c.extractIPsFromHeader(header)
+
+		// since X-Forwarded-For has no RFC, it's really up to the proxy to decide whether to append
+		// or prepend IPs to this list. For example, the AWS ALB will prepend but the F5 BIG-IP will append ;(
+		// for now lets just go with the first value in the list...
+		if len(ips) > 0 {
+			return ips[0]
+		}
+
+		// return the IP from the stack if we could not find any valid Ips
+		return c.fasthttp.RemoteIP().String()
+	}
+
+	// default behaviour if IP validation is not enabled is just to return whatever value is
+	// in the proxy header. Even if it is empty or invalid
+	return c.Get(c.app.config.ProxyHeader)
+}
+
+// IPs returns a string slice of IP addresses specified in the X-Forwarded-For request header.
+// When IP validation is enabled, only valid IPs are returned.
+func (c *Ctx) IPs() (ips []string) {
+	return c.extractIPsFromHeader(HeaderXForwardedFor)
 }
 
 // Is returns the matching content type,

ctx_test.go

@@ -1099,19 +1099,86 @@ func Test_Ctx_PortInHandler(t *testing.T) {
 // go test -run Test_Ctx_IP
 func Test_Ctx_IP(t *testing.T) {
 	t.Parallel()
+
 	app := New()
 	c := app.AcquireCtx(&fasthttp.RequestCtx{})
 	defer app.ReleaseCtx(c)
+
+	// default behaviour will return the remote IP from the stack
+	utils.AssertEqual(t, "0.0.0.0", c.IP())
+
+	// X-Forwarded-For is set, but it is ignored because proxyHeader is not set
+	c.Request().Header.Set(HeaderXForwardedFor, "0.0.0.1")
 	utils.AssertEqual(t, "0.0.0.0", c.IP())
 }
 
 // go test -run Test_Ctx_IP_ProxyHeader
 func Test_Ctx_IP_ProxyHeader(t *testing.T) {
 	t.Parallel()
-	app := New(Config{ProxyHeader: "Real-Ip"})
-	c := app.AcquireCtx(&fasthttp.RequestCtx{})
-	defer app.ReleaseCtx(c)
-	utils.AssertEqual(t, "", c.IP())
+
+	// make sure that the same behaviour exists for different proxy header names
+	proxyHeaderNames := []string{"Real-Ip", HeaderXForwardedFor}
+
+	for _, proxyHeaderName := range proxyHeaderNames {
+		app := New(Config{ProxyHeader: proxyHeaderName})
+		c := app.AcquireCtx(&fasthttp.RequestCtx{})
+
+		c.Request().Header.Set(proxyHeaderName, "0.0.0.1")
+		utils.AssertEqual(t, "0.0.0.1", c.IP())
+
+		// without IP validation we return the full string
+		c.Request().Header.Set(proxyHeaderName, "0.0.0.1, 0.0.0.2")
+		utils.AssertEqual(t, "0.0.0.1, 0.0.0.2", c.IP())
+
+		// without IP validation we return invalid IPs
+		c.Request().Header.Set(proxyHeaderName, "invalid, 0.0.0.2, 0.0.0.3")
+		utils.AssertEqual(t, "invalid, 0.0.0.2, 0.0.0.3", c.IP())
+
+		// when proxy header is enabled but the value is empty, without IP validation we return an empty string
+		c.Request().Header.Set(proxyHeaderName, "")
+		utils.AssertEqual(t, "", c.IP())
+
+		// without IP validation we return an invalid IP
+		c.Request().Header.Set(proxyHeaderName, "not-valid-ip")
+		utils.AssertEqual(t, "not-valid-ip", c.IP())
+
+		app.ReleaseCtx(c)
+	}
+}
+
+// go test -run Test_Ctx_IP_ProxyHeader
+func Test_Ctx_IP_ProxyHeader_With_IP_Validation(t *testing.T) {
+	t.Parallel()
+
+	// make sure that the same behaviour exists for different proxy header names
+	proxyHeaderNames := []string{"Real-Ip", HeaderXForwardedFor}
+
+	for _, proxyHeaderName := range proxyHeaderNames {
+		app := New(Config{EnableIPValidation: true, ProxyHeader: proxyHeaderName})
+		c := app.AcquireCtx(&fasthttp.RequestCtx{})
+
+		// when proxy header & validation is enabled and the value is a valid IP, we return it
+		c.Request().Header.Set(proxyHeaderName, "0.0.0.1")
+		utils.AssertEqual(t, "0.0.0.1", c.IP())
+
+		// when proxy header & validation is enabled and the value is a list of IPs, we return the first valid IP
+		c.Request().Header.Set(proxyHeaderName, "0.0.0.1, 0.0.0.2")
+		utils.AssertEqual(t, "0.0.0.1", c.IP())
+
+		c.Request().Header.Set(proxyHeaderName, "invalid, 0.0.0.2, 0.0.0.3")
+		utils.AssertEqual(t, "0.0.0.2", c.IP())
+
+		// when proxy header & validation is enabled but the value is empty, we will ignore the header
+		c.Request().Header.Set(proxyHeaderName, "")
+		utils.AssertEqual(t, "0.0.0.0", c.IP())
+
+		// when proxy header & validation is enabled but the value is not an IP, we will ignore the header
+		// and return the IP of the caller
+		c.Request().Header.Set(proxyHeaderName, "not-valid-ip")
+		utils.AssertEqual(t, "0.0.0.0", c.IP())
+
+		app.ReleaseCtx(c)
+	}
 }
 
 // go test -run Test_Ctx_IP_UntrustedProxy
@@ -1140,29 +1207,136 @@ func Test_Ctx_IPs(t *testing.T) {
 	app := New()
 	c := app.AcquireCtx(&fasthttp.RequestCtx{})
 	defer app.ReleaseCtx(c)
+
+	// normal happy path test case
+	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.1, 127.0.0.2, 127.0.0.3")
+	utils.AssertEqual(t, []string{"127.0.0.1", "127.0.0.2", "127.0.0.3"}, c.IPs())
+
+	// inconsistent space formatting
+	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.1,127.0.0.2  ,127.0.0.3")
+	utils.AssertEqual(t, []string{"127.0.0.1", "127.0.0.2", "127.0.0.3"}, c.IPs())
+
+	// invalid IPs are allowed to be returned
+	c.Request().Header.Set(HeaderXForwardedFor, "invalid, 127.0.0.1, 127.0.0.2")
+	utils.AssertEqual(t, []string{"invalid", "127.0.0.1", "127.0.0.2"}, c.IPs())
+	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.1, invalid, 127.0.0.2")
+	utils.AssertEqual(t, []string{"127.0.0.1", "invalid", "127.0.0.2"}, c.IPs())
+
+	// ensure that the ordering of IPs in the header is maintained
+	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.3, 127.0.0.1, 127.0.0.2")
+	utils.AssertEqual(t, []string{"127.0.0.3", "127.0.0.1", "127.0.0.2"}, c.IPs())
+
+	// empty header
+	c.Request().Header.Set(HeaderXForwardedFor, "")
+	utils.AssertEqual(t, 0, len(c.IPs()))
+
+	// missing header
+	c.Request()
+	utils.AssertEqual(t, 0, len(c.IPs()))
+}
+
+func Test_Ctx_IPs_With_IP_Validation(t *testing.T) {
+	t.Parallel()
+	app := New(Config{EnableIPValidation: true})
+	c := app.AcquireCtx(&fasthttp.RequestCtx{})
+	defer app.ReleaseCtx(c)
+
+	// normal happy path test case
 	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.1, 127.0.0.2, 127.0.0.3")
 	utils.AssertEqual(t, []string{"127.0.0.1", "127.0.0.2", "127.0.0.3"}, c.IPs())
 
+	// inconsistent space formatting
 	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.1,127.0.0.2  ,127.0.0.3")
 	utils.AssertEqual(t, []string{"127.0.0.1", "127.0.0.2", "127.0.0.3"}, c.IPs())
 
+	// invalid IPs are in the header
+	c.Request().Header.Set(HeaderXForwardedFor, "invalid, 127.0.0.1, 127.0.0.2")
+	utils.AssertEqual(t, []string{"127.0.0.1", "127.0.0.2"}, c.IPs())
+	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.1, invalid, 127.0.0.2")
+	utils.AssertEqual(t, []string{"127.0.0.1", "127.0.0.2"}, c.IPs())
+
+	// ensure that the ordering of IPs in the header is maintained
+	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.3, 127.0.0.1, 127.0.0.2")
+	utils.AssertEqual(t, []string{"127.0.0.3", "127.0.0.1", "127.0.0.2"}, c.IPs())
+
+	// empty header
 	c.Request().Header.Set(HeaderXForwardedFor, "")
 	utils.AssertEqual(t, 0, len(c.IPs()))
+
+	// missing header
+	c.Request()
+	utils.AssertEqual(t, 0, len(c.IPs()))
 }
 
 // go test -v -run=^$ -bench=Benchmark_Ctx_IPs -benchmem -count=4
 func Benchmark_Ctx_IPs(b *testing.B) {
 	app := New()
 	c := app.AcquireCtx(&fasthttp.RequestCtx{})
 	defer app.ReleaseCtx(c)
-	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.1, 127.0.0.1, 127.0.0.1")
+	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.1, invalid, 127.0.0.1")
+	var res []string
+	b.ReportAllocs()
+	b.ResetTimer()
+	for n := 0; n < b.N; n++ {
+		res = c.IPs()
+	}
+	utils.AssertEqual(b, []string{"127.0.0.1", "invalid", "127.0.0.1"}, res)
+}
+
+func Benchmark_Ctx_IPs_With_IP_Validation(b *testing.B) {
+	app := New(Config{EnableIPValidation: true})
+	c := app.AcquireCtx(&fasthttp.RequestCtx{})
+	defer app.ReleaseCtx(c)
+	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.1, invalid, 127.0.0.1")
 	var res []string
 	b.ReportAllocs()
 	b.ResetTimer()
 	for n := 0; n < b.N; n++ {
 		res = c.IPs()
 	}
-	utils.AssertEqual(b, []string{"127.0.0.1", "127.0.0.1", "127.0.0.1"}, res)
+	utils.AssertEqual(b, []string{"127.0.0.1", "127.0.0.1"}, res)
+}
+
+func Benchmark_Ctx_IP_With_ProxyHeader(b *testing.B) {
+	app := New(Config{ProxyHeader: HeaderXForwardedFor})
+	c := app.AcquireCtx(&fasthttp.RequestCtx{})
+	defer app.ReleaseCtx(c)
+	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.1")
+	var res string
+	b.ReportAllocs()
+	b.ResetTimer()
+	for n := 0; n < b.N; n++ {
+		res = c.IP()
+	}
+	utils.AssertEqual(b, "127.0.0.1", res)
+}
+
+func Benchmark_Ctx_IP_With_ProxyHeader_and_IP_Validation(b *testing.B) {
+	app := New(Config{ProxyHeader: HeaderXForwardedFor, EnableIPValidation: true})
+	c := app.AcquireCtx(&fasthttp.RequestCtx{})
+	defer app.ReleaseCtx(c)
+	c.Request().Header.Set(HeaderXForwardedFor, "127.0.0.1")
+	var res string
+	b.ReportAllocs()
+	b.ResetTimer()
+	for n := 0; n < b.N; n++ {
+		res = c.IP()
+	}
+	utils.AssertEqual(b, "127.0.0.1", res)
+}
+
+func Benchmark_Ctx_IP(b *testing.B) {
+	app := New()
+	c := app.AcquireCtx(&fasthttp.RequestCtx{})
+	defer app.ReleaseCtx(c)
+	c.Request()
+	var res string
+	b.ReportAllocs()
+	b.ResetTimer()
+	for n := 0; n < b.N; n++ {
+		res = c.IP()
+	}
+	utils.AssertEqual(b, "0.0.0.0", res)
 }
 
 // go test -run Test_Ctx_Is