Repository: labstack/echo
Pull request #1478 (2020-01-15 02:47:38)

Safer/trustable extraction of real ip from request
IP address extraction using HTTP header requires additional care. Otherwise attackers might have a chance to deceive wrong ip address. 

I fixed this problem.

See also: https://github.com/labstack/echo/issues/855

I wrote detailed doc in https://github.com/labstack/echox/pull/134, tl; dr:
- Don't use HTTP header unless it's reported by trustable client
- For XFF header, use rightmost untrustable value, not the leftmost value.
- When proxing, don't relay incoming X-Real-IP unless it's reported by trustable client

Especially about XFF header parsing, see implementations from other players:
- Rails:
    - doc: https://api.rubyonrails.org/classes/ActionDispatch/RemoteIp.html
    - src: https://github.com/rails/rails/blob/v6.0.2.1/actionpack/lib/action_dispatch/middleware/remote_ip.rb
- Express.js: 
    - doc: https://expressjs.com/en/guide/behind-proxies.html
    - src: https://github.com/jshttp/proxy-addr/blob/v2.0.5/index.js
- Werkzeug (used by Flask)
    - src: https://github.com/pallets/werkzeug/blob/1.0.0/src/werkzeug/middleware/proxy_fix.py
    - doc: https://werkzeug.palletsprojects.com/en/1.0.x/middleware/proxy_fix/
- Nginx
    - doc: https://nginx.org/en/docs/http/ngx_http_realip_module.html
    - src: https://github.com/nginx/nginx/blob/release-1.17.8/src/http/ngx_http_core_module.c#L2619
- Envoy:
    - src: https://github.com/envoyproxy/envoy/blob/v1.13.0/source/common/http/utility.cc#L353

Another note about XFF header:
- it could be joined by `,` without whitespace (e.g.: [http-party/node-http-proxy](https://github.com/http-party/node-http-proxy/blob/9bbe486c5efcc356fb4d189ef38eee275bbde345/lib/http-proxy/passes/web-incoming.js#L81))
- multiple header might be set as mentioned in [RFC 7230](https://tools.ietf.org/html/rfc7230#section-3.2.2) (see also [Go's reverseproxy](https://github.com/golang/go/blob/cfe2ab42e764d2eea3a3339aac1eaff97520baa0/src/net/http/httputil/reverseproxy.go#L251-L253))

codecov[bot]: # [Codecov](https://codecov.io/gh/labstack/echo/pull/1478?src=pr&el=h1) Report
> Merging [#1478](https://codecov.io/gh/labstack/echo/pull/1478?src=pr&el=desc) into [master](https://codecov.io/gh/labstack/echo/commit/399da56370c28c1788b9c61e0e55ab82be59fac1?src=pr&el=desc) will **decrease** coverage by `1.85%`.
> The diff coverage is `25%`.

[![Impacted file tree graph](https://codecov.io/gh/labstack/echo/pull/1478/graphs/tree.svg?width=650&token=vqnT76l79i&height=150&src=pr)](https://codecov.io/gh/labstack/echo/pull/1478?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1478      +/-   ##
==========================================
- Coverage   84.32%   82.47%   -1.86%     
==========================================
  Files          27       28       +1     
  Lines        2073     2128      +55     
==========================================
+ Hits         1748     1755       +7     
- Misses        212      260      +48     
  Partials      113      113
```


| [Impacted Files](https://codecov.io/gh/labstack/echo/pull/1478?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [context.go](https://codecov.io/gh/labstack/echo/pull/1478/diff?src=pr&el=tree#diff-Y29udGV4dC5nbw==) | `90.94% <100%> (-0.19%)` | :arrow_down: |
| [echo.go](https://codecov.io/gh/labstack/echo/pull/1478/diff?src=pr&el=tree#diff-ZWNoby5nbw==) | `85.38% <100%> (+0.04%)` | :arrow_up: |
| [ip.go](https://codecov.io/gh/labstack/echo/pull/1478/diff?src=pr&el=tree#diff-aXAuZ28=) | `18.64% <18.64%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/labstack/echo/pull/1478?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/labstack/echo/pull/1478?src=pr&el=footer). Last update [399da56...2c44e34](https://codecov.io/gh/labstack/echo/pull/1478?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/context.go b/context.go
index 0046e5d..86b5064 100644
--- a/context.go
+++ b/context.go
@@ -43,6 +43,7 @@ type (
 
 		// RealIP returns the client's network address based on `X-Forwarded-For`
 		// or `X-Real-IP` request header.
+		// The behavior can be configured using `Echo#IPExtractor`.
 		RealIP() string
 
 		// Path returns the registered path for the handler.
@@ -270,6 +271,10 @@ func (c *context) Scheme() string {
 }
 
 func (c *context) RealIP() string {
+	if c.echo != nil && c.echo.IPExtractor != nil {
+		return c.echo.IPExtractor(c.request)
+	}
+	// Fall back to legacy behavior
 	if ip := c.request.Header.Get(HeaderXForwardedFor); ip != "" {
 		return strings.Split(ip, ", ")[0]
 	}
diff --git a/echo.go b/echo.go
index a759e34..1f8abe7 100644
--- a/echo.go
+++ b/echo.go
@@ -90,6 +90,7 @@ type (
 		Validator        Validator
 		Renderer         Renderer
 		Logger           Logger
+		IPExtractor      IPExtractor
 	}
 
 	// Route contains a handler and information for matching against requests.
diff --git a/ip.go b/ip.go
new file mode 100644
index 0000000..39cb421
--- /dev/null
+++ b/ip.go
@@ -0,0 +1,137 @@
+package echo
+
+import (
+	"net"
+	"net/http"
+	"strings"
+)
+
+type ipChecker struct {
+	trustLoopback    bool
+	trustLinkLocal   bool
+	trustPrivateNet  bool
+	trustExtraRanges []*net.IPNet
+}
+
+// TrustOption is config for which IP address to trust
+type TrustOption func(*ipChecker)
+
+// TrustLoopback configures if you trust loopback address (default: true).
+func TrustLoopback(v bool) TrustOption {
+	return func(c *ipChecker) {
+		c.trustLoopback = v
+	}
+}
+
+// TrustLinkLocal configures if you trust link-local address (default: true).
+func TrustLinkLocal(v bool) TrustOption {
+	return func(c *ipChecker) {
+		c.trustLinkLocal = v
+	}
+}
+
+// TrustPrivateNet configures if you trust private network address (default: true).
+func TrustPrivateNet(v bool) TrustOption {
+	return func(c *ipChecker) {
+		c.trustPrivateNet = v
+	}
+}
+
+// TrustIPRange add trustable IP ranges using CIDR notation.
+func TrustIPRange(ipRange *net.IPNet) TrustOption {
+	return func(c *ipChecker) {
+		c.trustExtraRanges = append(c.trustExtraRanges, ipRange)
+	}
+}
+
+func newIPChecker(configs []TrustOption) *ipChecker {
+	checker := &ipChecker{trustLoopback: true, trustLinkLocal: true, trustPrivateNet: true}
+	for _, configure := range configs {
+		configure(checker)
+	}
+	return checker
+}
+
+func isPrivateIPRange(ip net.IP) bool {
+	if ip4 := ip.To4(); ip4 != nil {
+		return ip4[0] == 10 ||
+			ip4[0] == 172 && ip4[1]&0xf0 == 16 ||
+			ip4[0] == 192 && ip4[1] == 168
+	}
+	return len(ip) == net.IPv6len && ip[0]&0xfe == 0xfc
+}
+
+func (c *ipChecker) trust(ip net.IP) bool {
+	if c.trustLoopback && ip.IsLoopback() {
+		return true
+	}
+	if c.trustLinkLocal && ip.IsLinkLocalUnicast() {
+		return true
+	}
+	if c.trustPrivateNet && isPrivateIPRange(ip) {
+		return true
+	}
+	for _, trustedRange := range c.trustExtraRanges {
+		if trustedRange.Contains(ip) {
+			return true
+		}
+	}
+	return false
+}
+
+// IPExtractor is a function to extract IP addr from http.Request.
+// Set appropriate one to Echo#IPExtractor.
+// See https://echo.labstack.com/guide/ip-address for more details.
+type IPExtractor func(*http.Request) string
+
+// ExtractIPDirect extracts IP address using actual IP address.
+// Use this if your server faces to internet directory (i.e.: uses no proxy).
+func ExtractIPDirect() IPExtractor {
+	return func(req *http.Request) string {
+		ra, _, _ := net.SplitHostPort(req.RemoteAddr)
+		return ra
+	}
+}
+
+// ExtractIPFromRealIPHeader extracts IP address using x-real-ip header.
+// Use this if you put proxy which uses this header.
+func ExtractIPFromRealIPHeader(options ...TrustOption) IPExtractor {
+	checker := newIPChecker(options)
+	return func(req *http.Request) string {
+		directIP := ExtractIPDirect()(req)
+		realIP := req.Header.Get(HeaderXRealIP)
+		if realIP != "" {
+			if ip := net.ParseIP(directIP); ip != nil && checker.trust(ip) {
+				return realIP
+			}
+		}
+		return directIP
+	}
+}
+
+// ExtractIPFromXFFHeader extracts IP address using x-forwarded-for header.
+// Use this if you put proxy which uses this header.
+// This returns nearest untrustable IP. If all IPs are trustable, returns furthest one (i.e.: XFF[0]).
+func ExtractIPFromXFFHeader(options ...TrustOption) IPExtractor {
+	checker := newIPChecker(options)
+	return func(req *http.Request) string {
+		directIP := ExtractIPDirect()(req)
+		xffs := req.Header[HeaderXForwardedFor]
+		if len(xffs) == 0 {
+			return directIP
+		}
+		ips := append(strings.Split(strings.Join(xffs, ","), ","), directIP)
+		for i := len(ips) - 1; i >= 0; i-- {
+			ip := net.ParseIP(strings.TrimSpace(ips[i]))
+			if ip == nil {
+				// Unable to parse IP; cannot trust entire records
+				return directIP
+			}
+			if !checker.trust(ip) {
+				return ip.String()
+			}
+		}
+		// All of the IPs are trusted; return first element because it is furthest from server (best effort strategy).
+		return strings.TrimSpace(ips[0])
+	}
+}
diff --git a/middleware/proxy.go b/middleware/proxy.go
index ef5602b..1da370d 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -231,7 +231,9 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 			}
 
 			// Fix header
-			if req.Header.Get(echo.HeaderXRealIP) == "" {
+			// Basically it's not good practice to unconditionally pass incoming x-real-ip header to upstream.
+			// However, for backward compatibility, legacy behavior is preserved unless you configure Echo#IPExtractor.
+			if req.Header.Get(echo.HeaderXRealIP) == "" || c.Echo().IPExtractor != nil {
 				req.Header.Set(echo.HeaderXRealIP, c.RealIP())
 			}
 			if req.Header.Get(echo.HeaderXForwardedProto) == "" {
diff --git a/router.go b/router.go
index 9db9ea9..15a3398 100644
--- a/router.go
+++ b/router.go
@@ -347,7 +347,14 @@ func (r *Router) Find(method, path string, c Context) {
 		if l == pl {
 			// Continue search
 			search = search[l:]
-		} else {
+			// Finish routing if no remaining search and we are on an leaf node
+			if search == "" && (nn == nil || cn.parent == nil || cn.ppath != "") {
+				break
+			}
+		}
+
+		// Attempt to go back up the tree on no matching prefix or no remaining search
+		if l != pl || search == "" {
 			if nn == nil { // Issue #1348
 				return // Not found
 			}
@@ -360,10 +367,6 @@ func (r *Router) Find(method, path string, c Context) {
 			}
 		}
 
-		if search == "" {
-			break
-		}
-
 		// Static node
 		if child = cn.findChild(search[0], skind); child != nil {
 			// Save next
@@ -376,8 +379,8 @@ func (r *Router) Find(method, path string, c Context) {
 			continue
 		}
 
-		// Param node
 	Param:
+		// Param node
 		if child = cn.findChildByKind(pkind); child != nil {
 			// Issue #378
 			if len(pvalues) == n {
@@ -401,47 +404,58 @@ func (r *Router) Find(method, path string, c Context) {
 			continue
 		}
 
-		// Any node
 	Any:
-		if cn = cn.findChildByKind(akind); cn == nil {
-			if nn != nil {
-				// No next node to go down in routing (issue #954)
-				// Find nearest "any" route going up the routing tree
-				search = ns
-				np := nn.parent
-				// Consider param route one level up only
-				// if no slash is remaining in search string
-				if cn = nn.findChildByKind(pkind); cn != nil && strings.IndexByte(ns, '/') == -1 {
+		// Any node
+		if cn = cn.findChildByKind(akind); cn != nil {
+			// If any node is found, use remaining path for pvalues
+			pvalues[len(cn.pnames)-1] = search
+			break
+		}
+
+		// No node found, continue at stored next node
+		// or find nearest "any" route
+		if nn != nil {
+			// No next node to go down in routing (issue #954)
+			// Find nearest "any" route going up the routing tree
+			search = ns
+			np := nn.parent
+			// Consider param route one level up only
+			if cn = nn.findChildByKind(pkind); cn != nil {
+				pos := strings.IndexByte(ns, '/')
+				if pos == -1 {
+					// If no slash is remaining in search string set param value
 					pvalues[len(cn.pnames)-1] = search
 					break
-				} else if cn != nil && strings.IndexByte(ns, '/') != 1 {
-					// If slash is present, it means that this is a parameterized route.
-					cn = cn.parent
+				} else if pos > 0 {
+					// Otherwise continue route processing with restored next node
+					cn = nn
+					nn = nil
+					ns = ""
 					goto Param
 				}
-				for {
-					np = nn.parent
-					if cn = nn.findChildByKind(akind); cn != nil {
-						break
-					}
-					if np == nil {
-						break // no further parent nodes in tree, abort
-					}
-					var str strings.Builder
-					str.WriteString(nn.prefix)
-					str.WriteString(search)
-					search = str.String()
-					nn = np
-				}
-				if cn != nil { // use the found "any" route and update path
-					pvalues[len(cn.pnames)-1] = search
+			}
+			// No param route found, try to resolve nearest any route
+			for {
+				np = nn.parent
+				if cn = nn.findChildByKind(akind); cn != nil {
 					break
 				}
+				if np == nil {
+					break // no further parent nodes in tree, abort
+				}
+				var str strings.Builder
+				str.WriteString(nn.prefix)
+				str.WriteString(search)
+				search = str.String()
+				nn = np
+			}
+			if cn != nil { // use the found "any" route and update path
+				pvalues[len(cn.pnames)-1] = search
+				break
 			}
-			return // Not found
 		}
-		pvalues[len(cn.pnames)-1] = search
-		break
+		return // Not found
+
 	}
 
 	ctx.handler = cn.findHandler(method)


Test patch:
diff --git a/ip_test.go b/ip_test.go
new file mode 100644
index 0000000..5acc117
--- /dev/null
+++ b/ip_test.go
@@ -0,0 +1,235 @@
+package echo
+
+import (
+	"net"
+	"net/http"
+	"strings"
+	"testing"
+
+	testify "github.com/stretchr/testify/assert"
+)
+
+const (
+	// For RemoteAddr
+	ipForRemoteAddrLoopback  = "127.0.0.1" // From 127.0.0.0/8
+	sampleRemoteAddrLoopback = ipForRemoteAddrLoopback + ":8080"
+	ipForRemoteAddrExternal  = "203.0.113.1"
+	sampleRemoteAddrExternal = ipForRemoteAddrExternal + ":8080"
+	// For x-real-ip
+	ipForRealIP = "203.0.113.10"
+	// For XFF
+	ipForXFF1LinkLocal = "169.254.0.101" // From 169.254.0.0/16
+	ipForXFF2Private   = "192.168.0.102" // From 192.168.0.0/16
+	ipForXFF3External  = "2001:db8::103"
+	ipForXFF4Private   = "fc00::104" // From fc00::/7
+	ipForXFF5External  = "198.51.100.105"
+	ipForXFF6External  = "192.0.2.106"
+	ipForXFFBroken     = "this.is.broken.lol"
+	// keys for test cases
+	ipTestReqKeyNoHeader             = "no header"
+	ipTestReqKeyRealIPExternal       = "x-real-ip; remote addr external"
+	ipTestReqKeyRealIPInternal       = "x-real-ip; remote addr internal"
+	ipTestReqKeyRealIPAndXFFExternal = "x-real-ip and xff; remote addr external"
+	ipTestReqKeyRealIPAndXFFInternal = "x-real-ip and xff; remote addr internal"
+	ipTestReqKeyXFFExternal          = "xff; remote addr external"
+	ipTestReqKeyXFFInternal          = "xff; remote addr internal"
+	ipTestReqKeyBrokenXFF            = "broken xff"
+)
+
+var (
+	sampleXFF = strings.Join([]string{
+		ipForXFF6External, ipForXFF5External, ipForXFF4Private, ipForXFF3External, ipForXFF2Private, ipForXFF1LinkLocal,
+	}, ", ")
+
+	requests = map[string]*http.Request{
+		ipTestReqKeyNoHeader: &http.Request{
+			RemoteAddr: sampleRemoteAddrExternal,
+		},
+		ipTestReqKeyRealIPExternal: &http.Request{
+			Header: http.Header{
+				"X-Real-Ip": []string{ipForRealIP},
+			},
+			RemoteAddr: sampleRemoteAddrExternal,
+		},
+		ipTestReqKeyRealIPInternal: &http.Request{
+			Header: http.Header{
+				"X-Real-Ip": []string{ipForRealIP},
+			},
+			RemoteAddr: sampleRemoteAddrLoopback,
+		},
+		ipTestReqKeyRealIPAndXFFExternal: &http.Request{
+			Header: http.Header{
+				"X-Real-Ip":         []string{ipForRealIP},
+				HeaderXForwardedFor: []string{sampleXFF},
+			},
+			RemoteAddr: sampleRemoteAddrExternal,
+		},
+		ipTestReqKeyRealIPAndXFFInternal: &http.Request{
+			Header: http.Header{
+				"X-Real-Ip":         []string{ipForRealIP},
+				HeaderXForwardedFor: []string{sampleXFF},
+			},
+			RemoteAddr: sampleRemoteAddrLoopback,
+		},
+		ipTestReqKeyXFFExternal: &http.Request{
+			Header: http.Header{
+				HeaderXForwardedFor: []string{sampleXFF},
+			},
+			RemoteAddr: sampleRemoteAddrExternal,
+		},
+		ipTestReqKeyXFFInternal: &http.Request{
+			Header: http.Header{
+				HeaderXForwardedFor: []string{sampleXFF},
+			},
+			RemoteAddr: sampleRemoteAddrLoopback,
+		},
+		ipTestReqKeyBrokenXFF: &http.Request{
+			Header: http.Header{
+				HeaderXForwardedFor: []string{ipForXFFBroken + ", " + ipForXFF1LinkLocal},
+			},
+			RemoteAddr: sampleRemoteAddrLoopback,
+		},
+	}
+)
+
+func TestExtractIP(t *testing.T) {
+	_, ipv4AllRange, _ := net.ParseCIDR("0.0.0.0/0")
+	_, ipv6AllRange, _ := net.ParseCIDR("::/0")
+	_, ipForXFF3ExternalRange, _ := net.ParseCIDR(ipForXFF3External + "/48")
+	_, ipForRemoteAddrExternalRange, _ := net.ParseCIDR(ipForRemoteAddrExternal + "/24")
+
+	tests := map[string]*struct {
+		extractor   IPExtractor
+		expectedIPs map[string]string
+	}{
+		"ExtractIPDirect": {
+			ExtractIPDirect(),
+			map[string]string{
+				ipTestReqKeyNoHeader:             ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPExternal:       ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPInternal:       ipForRemoteAddrLoopback,
+				ipTestReqKeyRealIPAndXFFExternal: ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPAndXFFInternal: ipForRemoteAddrLoopback,
+				ipTestReqKeyXFFExternal:          ipForRemoteAddrExternal,
+				ipTestReqKeyXFFInternal:          ipForRemoteAddrLoopback,
+				ipTestReqKeyBrokenXFF:            ipForRemoteAddrLoopback,
+			},
+		},
+		"ExtractIPFromRealIPHeader(default)": {
+			ExtractIPFromRealIPHeader(),
+			map[string]string{
+				ipTestReqKeyNoHeader:             ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPExternal:       ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPInternal:       ipForRealIP,
+				ipTestReqKeyRealIPAndXFFExternal: ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPAndXFFInternal: ipForRealIP,
+				ipTestReqKeyXFFExternal:          ipForRemoteAddrExternal,
+				ipTestReqKeyXFFInternal:          ipForRemoteAddrLoopback,
+				ipTestReqKeyBrokenXFF:            ipForRemoteAddrLoopback,
+			},
+		},
+		"ExtractIPFromRealIPHeader(trust only direct-facing proxy)": {
+			ExtractIPFromRealIPHeader(TrustLoopback(false), TrustLinkLocal(false), TrustPrivateNet(false), TrustIPRange(ipForRemoteAddrExternalRange)),
+			map[string]string{
+				ipTestReqKeyNoHeader:             ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPExternal:       ipForRealIP,
+				ipTestReqKeyRealIPInternal:       ipForRemoteAddrLoopback,
+				ipTestReqKeyRealIPAndXFFExternal: ipForRealIP,
+				ipTestReqKeyRealIPAndXFFInternal: ipForRemoteAddrLoopback,
+				ipTestReqKeyXFFExternal:          ipForRemoteAddrExternal,
+				ipTestReqKeyXFFInternal:          ipForRemoteAddrLoopback,
+				ipTestReqKeyBrokenXFF:            ipForRemoteAddrLoopback,
+			},
+		},
+		"ExtractIPFromRealIPHeader(trust direct-facing proxy)": {
+			ExtractIPFromRealIPHeader(TrustIPRange(ipForRemoteAddrExternalRange)),
+			map[string]string{
+				ipTestReqKeyNoHeader:             ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPExternal:       ipForRealIP,
+				ipTestReqKeyRealIPInternal:       ipForRealIP,
+				ipTestReqKeyRealIPAndXFFExternal: ipForRealIP,
+				ipTestReqKeyRealIPAndXFFInternal: ipForRealIP,
+				ipTestReqKeyXFFExternal:          ipForRemoteAddrExternal,
+				ipTestReqKeyXFFInternal:          ipForRemoteAddrLoopback,
+				ipTestReqKeyBrokenXFF:            ipForRemoteAddrLoopback,
+			},
+		},
+		"ExtractIPFromXFFHeader(default)": {
+			ExtractIPFromXFFHeader(),
+			map[string]string{
+				ipTestReqKeyNoHeader:             ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPExternal:       ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPInternal:       ipForRemoteAddrLoopback,
+				ipTestReqKeyRealIPAndXFFExternal: ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPAndXFFInternal: ipForXFF3External,
+				ipTestReqKeyXFFExternal:          ipForRemoteAddrExternal,
+				ipTestReqKeyXFFInternal:          ipForXFF3External,
+				ipTestReqKeyBrokenXFF:            ipForRemoteAddrLoopback,
+			},
+		},
+		"ExtractIPFromXFFHeader(trust only direct-facing proxy)": {
+			ExtractIPFromXFFHeader(TrustLoopback(false), TrustLinkLocal(false), TrustPrivateNet(false), TrustIPRange(ipForRemoteAddrExternalRange)),
+			map[string]string{
+				ipTestReqKeyNoHeader:             ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPExternal:       ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPInternal:       ipForRemoteAddrLoopback,
+				ipTestReqKeyRealIPAndXFFExternal: ipForXFF1LinkLocal,
+				ipTestReqKeyRealIPAndXFFInternal: ipForRemoteAddrLoopback,
+				ipTestReqKeyXFFExternal:          ipForXFF1LinkLocal,
+				ipTestReqKeyXFFInternal:          ipForRemoteAddrLoopback,
+				ipTestReqKeyBrokenXFF:            ipForRemoteAddrLoopback,
+			},
+		},
+		"ExtractIPFromXFFHeader(trust direct-facing proxy)": {
+			ExtractIPFromXFFHeader(TrustIPRange(ipForRemoteAddrExternalRange)),
+			map[string]string{
+				ipTestReqKeyNoHeader:             ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPExternal:       ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPInternal:       ipForRemoteAddrLoopback,
+				ipTestReqKeyRealIPAndXFFExternal: ipForXFF3External,
+				ipTestReqKeyRealIPAndXFFInternal: ipForXFF3External,
+				ipTestReqKeyXFFExternal:          ipForXFF3External,
+				ipTestReqKeyXFFInternal:          ipForXFF3External,
+				ipTestReqKeyBrokenXFF:            ipForRemoteAddrLoopback,
+			},
+		},
+		"ExtractIPFromXFFHeader(trust everything)": {
+			// This is similar to legacy behavior, but ignores x-real-ip header.
+			ExtractIPFromXFFHeader(TrustIPRange(ipv4AllRange), TrustIPRange(ipv6AllRange)),
+			map[string]string{
+				ipTestReqKeyNoHeader:             ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPExternal:       ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPInternal:       ipForRemoteAddrLoopback,
+				ipTestReqKeyRealIPAndXFFExternal: ipForXFF6External,
+				ipTestReqKeyRealIPAndXFFInternal: ipForXFF6External,
+				ipTestReqKeyXFFExternal:          ipForXFF6External,
+				ipTestReqKeyXFFInternal:          ipForXFF6External,
+				ipTestReqKeyBrokenXFF:            ipForRemoteAddrLoopback,
+			},
+		},
+		"ExtractIPFromXFFHeader(trust ipForXFF3External)": {
+			// This trusts private network also after "additional" trust ranges unlike `TrustNProxies(1)` doesn't
+			ExtractIPFromXFFHeader(TrustIPRange(ipForXFF3ExternalRange)),
+			map[string]string{
+				ipTestReqKeyNoHeader:             ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPExternal:       ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPInternal:       ipForRemoteAddrLoopback,
+				ipTestReqKeyRealIPAndXFFExternal: ipForRemoteAddrExternal,
+				ipTestReqKeyRealIPAndXFFInternal: ipForXFF5External,
+				ipTestReqKeyXFFExternal:          ipForRemoteAddrExternal,
+				ipTestReqKeyXFFInternal:          ipForXFF5External,
+				ipTestReqKeyBrokenXFF:            ipForRemoteAddrLoopback,
+			},
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			assert := testify.New(t)
+			for key, req := range requests {
+				actual := test.extractor(req)
+				expected := test.expectedIPs[key]
+				assert.Equal(expected, actual, "Request: %s", key)
+			}
+		})
+	}
+}
diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index 1a375db..40d150c 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -2,6 +2,7 @@ package middleware
 
 import (
 	"fmt"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -119,3 +120,46 @@ func TestProxy(t *testing.T) {
 	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
 }
+
+func TestProxyRealIPHeader(t *testing.T) {
+	// Setup
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+	defer upstream.Close()
+	url, _ := url.Parse(upstream.URL)
+	rrb := NewRoundRobinBalancer([]*ProxyTarget{{Name: "upstream", URL: url}})
+	e := echo.New()
+	e.Use(Proxy(rrb))
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	rec := httptest.NewRecorder()
+
+	remoteAddrIP, _, _ := net.SplitHostPort(req.RemoteAddr)
+	realIPHeaderIP := "203.0.113.1"
+	extractedRealIP := "203.0.113.10"
+	tests := []*struct {
+		hasRealIPheader bool
+		hasIPExtractor  bool
+		extectedXRealIP string
+	}{
+		{false, false, remoteAddrIP},
+		{false, true, extractedRealIP},
+		{true, false, realIPHeaderIP},
+		{true, true, extractedRealIP},
+	}
+
+	for _, tt := range tests {
+		if tt.hasRealIPheader {
+			req.Header.Set(echo.HeaderXRealIP, realIPHeaderIP)
+		} else {
+			req.Header.Del(echo.HeaderXRealIP)
+		}
+		if tt.hasIPExtractor {
+			e.IPExtractor = func(*http.Request) string {
+				return extractedRealIP
+			}
+		} else {
+			e.IPExtractor = nil
+		}
+		e.ServeHTTP(rec, req)
+		assert.Equal(t, tt.extectedXRealIP, req.Header.Get(echo.HeaderXRealIP), "hasRealIPheader: %t / hasIPExtractor: %t", tt.hasRealIPheader, tt.hasIPExtractor)
+	}
+}
diff --git a/router_test.go b/router_test.go
index f47225d..8c27b9f 100644
--- a/router_test.go
+++ b/router_test.go
@@ -567,6 +567,32 @@ func TestRouterParamWithSlash(t *testing.T) {
 	})
 }
 
+// Issue #1509
+func TestRouterParamStaticConflict(t *testing.T) {
+	e := New()
+	r := e.router
+	handler := func(c Context) error {
+		c.Set("path", c.Path())
+		return nil
+	}
+
+	g := e.Group("/g")
+	g.GET("/skills", handler)
+	g.GET("/status", handler)
+	g.GET("/:name", handler)
+
+	c := e.NewContext(nil, nil).(*context)
+	r.Find(http.MethodGet, "/g/s", c)
+	c.handler(c)
+	assert.Equal(t, "s", c.Param("name"))
+	assert.Equal(t, "/g/:name", c.Get("path"))
+
+	c = e.NewContext(nil, nil).(*context)
+	r.Find(http.MethodGet, "/g/status", c)
+	c.handler(c)
+	assert.Equal(t, "/g/status", c.Get("path"))
+}
+
 func TestRouterMatchAny(t *testing.T) {
 	e := New()
 	r := e.router
@@ -1031,12 +1057,15 @@ func TestRouterParamBacktraceNotFound(t *testing.T) {
 	r.Find(http.MethodGet, "/a", c)
 	assert.Equal(t, "a", c.Param("param1"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/a/foo", c)
 	assert.Equal(t, "a", c.Param("param1"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/a/bar", c)
 	assert.Equal(t, "a", c.Param("param1"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/a/bar/b", c)
 	assert.Equal(t, "a", c.Param("param1"))
 	assert.Equal(t, "b", c.Param("param2"))
@@ -1157,31 +1186,45 @@ func TestRouterParam1466(t *testing.T) {
 	r.Find(http.MethodGet, "/users/ajitem", c)
 	assert.Equal(t, "ajitem", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/sharewithme", c)
 	assert.Equal(t, "sharewithme", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/signup", c)
 	assert.Equal(t, "", c.Param("username"))
 	// Additional assertions for #1479
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/sharewithme/likes/projects/ids", c)
 	assert.Equal(t, "sharewithme", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/ajitem/likes/projects/ids", c)
 	assert.Equal(t, "ajitem", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/sharewithme/profile", c)
 	assert.Equal(t, "sharewithme", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/ajitem/profile", c)
 	assert.Equal(t, "ajitem", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/sharewithme/uploads/self", c)
 	assert.Equal(t, "sharewithme", c.Param("username"))
 	assert.Equal(t, "self", c.Param("type"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/ajitem/uploads/self", c)
 	assert.Equal(t, "ajitem", c.Param("username"))
 	assert.Equal(t, "self", c.Param("type"))
+
+	// Issue #1493 - check for routing loop
+	c = e.NewContext(nil, nil).(*context)
+	r.Find(http.MethodGet, "/users/tree/free", c)
+	assert.Equal(t, "", c.Param("id"))
+	assert.Equal(t, 0, c.response.Status)
 }
 
 func benchmarkRouterRoutes(b *testing.B, routes []*Route) {