Repository: labstack/echo (go)

Issue:
Multiple groups with the same path will overwrite actual routes (Caused by trying to run group middlewares on unregistered routes)
### Issue Description

### Checklist

- [x] Dependencies installed
- [x] No typos
- [x] Searched existing issues and docs

### Expected behaviour
I can access the `GET` route successfully, and it doesn't run the group middlewares for unregistered routes (Same as gin).

### Actual behaviour
It returns 404 and also unexpectedly runs the middlewares of the second group instead of the middlewares of the first one, and also runs them for unregistered routes.

This is caused by the following code: https://github.com/labstack/echo/blob/151ed6b3f150163352985448b5630ab69de40aa5/group.go#L26-L29

Not only is it unexpected to run the middlewares in such cases, especially if there are different groups with the same path (Which one of the group of middlewares are supposed to run for unregistered ones? For example, multiple different routes using the same path that have different middlewares that check permissions), but it *tramples all preexisting routes with the same path*.

AFAIK Gin doesn't have such behavior, and it should probably be removed entirely, barring backwards compatibility concerns, though this seems like strange behavior to depend on to begin with and anyone, who wants it can easily add such `Any` routes by himself.

### Steps to reproduce

1. Run the given code.
2. `curl -v http://localhost:1323/` will give you an unexpected 404 while also unexpectedly running middleware2.
<details><summary>output & log</summary>

```sh
➜ curl -v http://localhost:1323/     
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 1323 (#0)
> GET / HTTP/1.1
> Host: localhost:1323
> User-Agent: curl/7.54.0
> Accept: */*
> 
< HTTP/1.1 404 Not Found
< Content-Type: application/json; charset=UTF-8
< Date: Wed, 21 Oct 2020 16:42:44 GMT
< Content-Length: 24
< 
{"message":"Not Found"}
* Connection #0 to host localhost left intact
```

```
{"time":"2020-10-21T19:42:44.185404+03:00","level":"INFO","prefix":"echo","file":"main.go","line":"37","message":"middleware2"}
{"time":"2020-10-21T19:42:44.185927+03:00","id":"","remote_ip":"::1","host":"localhost:1323","method":"GET","uri":"/","user_agent":"curl/7.54.0","status":404,"error":"code=404, message=Not Found","latency":555023,"latency_human":"555.023µs","bytes_in":0,"bytes_out":24}
```
</details>

3. Run `curl -v -X POST http://localhost:1323/other` which does work

<details><summary>output & log</summary>

```sh
➜ curl -v -X POST http://localhost:1323/other
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 1323 (#0)
> POST /other HTTP/1.1
> Host: localhost:1323
> User-Agent: curl/7.54.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< Date: Wed, 21 Oct 2020 16:42:47 GMT
< Content-Length: 0
< 
* Connection #0 to host localhost left intact
```

```
{"time":"2020-10-21T19:42:47.193948+03:00","level":"INFO","prefix":"echo","file":"main.go","line":"37","message":"middleware2"}
{"time":"2020-10-21T19:42:47.194022+03:00","level":"INFO","prefix":"echo","file":"main.go","line":"43","message":"POST/other"}
{"time":"2020-10-21T19:42:47.194044+03:00","id":"","remote_ip":"::1","host":"localhost:1323","method":"POST","uri":"/other","user_agent":"curl/7.54.0","status":200,"error":"","latency":107297,"latency_human":"107.297µs","bytes_in":0,"bytes_out":0}
```
</details>

### Working code to debug

```go
package main

import (
	"github.com/labstack/echo/v4"
	"github.com/labstack/echo/v4/middleware"
	"github.com/labstack/gommon/log"
)

func main() {
	e := echo.New()
	e.Logger.SetLevel(log.INFO)
	e.Use(middleware.Logger(), middleware.Recover())

	g1 := e.Group("", middleware1())
	{
		g1.GET("", handler)
	}
	g2 := e.Group("", middleware2())
	{
		g2.POST("/other", handler)
	}

	e.Logger.Fatal(e.Start(":1323"))
}

func middleware1() echo.MiddlewareFunc {
	return func(next echo.HandlerFunc) echo.HandlerFunc {
		return func(c echo.Context) error {
			c.Logger().Info("middleware1")
			return next(c)
		}
	}
}

func middleware2() echo.MiddlewareFunc {
	return func(next echo.HandlerFunc) echo.HandlerFunc {
		return func(c echo.Context) error {
			c.Logger().Info("middleware2")
			return next(c)
		}
	}
}

func handler(c echo.Context) error {
	c.Logger().Info(c.Request().Method, c.Request().URL)

	return nil
}
```

### Version/commit
v4.1.17

Pull request:
Remove group.Use registering Any routes that break other routes
This can be a breaking change if this behavior is somehow important to anyone.

Fixes #1657

Patch:
diff --git a/context.go b/context.go
index 0507f13..99ef03b 100644
--- a/context.go
+++ b/context.go
@@ -276,11 +276,7 @@ func (c *context) RealIP() string {
 	}
 	// Fall back to legacy behavior
 	if ip := c.request.Header.Get(HeaderXForwardedFor); ip != "" {
-		i := strings.IndexAny(ip, ", ")
-		if i > 0 {
-			return ip[:i]
-		}
-		return ip
+		return strings.Split(ip, ", ")[0]
 	}
 	if ip := c.request.Header.Get(HeaderXRealIP); ip != "" {
 		return ip
diff --git a/echo.go b/echo.go
index 29b88b7..128f84f 100644
--- a/echo.go
+++ b/echo.go
@@ -362,12 +362,10 @@ func (e *Echo) DefaultHTTPErrorHandler(err error, c Context) {
 	// Issue #1426
 	code := he.Code
 	message := he.Message
-	if m, ok := he.Message.(string); ok {
-		if e.Debug {
-			message = Map{"message": m, "error": err.Error()}
-		} else {
-			message = Map{"message": m}
-		}
+	if e.Debug {
+		message = err.Error()
+	} else if m, ok := message.(string); ok {
+		message = Map{"message": m}
 	}
 
 	// Send response
diff --git a/group.go b/group.go
index 426bef9..d239fb5 100644
--- a/group.go
+++ b/group.go
@@ -23,10 +23,6 @@ func (g *Group) Use(middleware ...MiddlewareFunc) {
 	if len(g.middleware) == 0 {
 		return
 	}
-	// Allow all requests to reach the group as they might get dropped if router
-	// doesn't find a match, making none of the group middleware process.
-	g.Any("", NotFoundHandler)
-	g.Any("/*", NotFoundHandler)
 }
 
 // CONNECT implements `Echo#CONNECT()` for sub-routes within the Group.
diff --git a/middleware/middleware.go b/middleware/middleware.go
index 60834b5..12260dd 100644
--- a/middleware/middleware.go
+++ b/middleware/middleware.go
@@ -2,6 +2,7 @@ package middleware
 
 import (
 	"net/http"
+	"net/url"
 	"regexp"
 	"strconv"
 	"strings"
@@ -33,29 +34,15 @@ func captureTokens(pattern *regexp.Regexp, input string) *strings.Replacer {
 	return strings.NewReplacer(replace...)
 }
 
-func rewriteRulesRegex(rewrite map[string]string) map[*regexp.Regexp]string {
-	// Initialize
-	rulesRegex := map[*regexp.Regexp]string{}
-	for k, v := range rewrite {
-		k = regexp.QuoteMeta(k)
-		k = strings.Replace(k, `\*`, "(.*)", -1)
-		if strings.HasPrefix(k, `\^`) {
-			k = strings.Replace(k, `\^`, "^", -1)
-		}
-		k = k + "$"
-		rulesRegex[regexp.MustCompile(k)] = v
-	}
-	return rulesRegex
-}
-
-func rewritePath(rewriteRegex map[*regexp.Regexp]string, req *http.Request) {
-	for k, v := range rewriteRegex {
-		replacerRawPath := captureTokens(k, req.URL.EscapedPath())
-		if replacerRawPath != nil {
-			replacerPath := captureTokens(k, req.URL.Path)
-			req.URL.RawPath, req.URL.Path = replacerRawPath.Replace(v), replacerPath.Replace(v)
-		}
+//rewritePath sets request url path and raw path
+func rewritePath(replacer *strings.Replacer, target string, req *http.Request) error {
+	replacerRawPath := replacer.Replace(target)
+	replacerPath, err := url.PathUnescape(replacerRawPath)
+	if err != nil {
+		return err
 	}
+	req.URL.Path, req.URL.RawPath = replacerPath, replacerRawPath
+	return nil
 }
 
 // DefaultSkipper returns false which processes the middleware.
diff --git a/middleware/proxy.go b/middleware/proxy.go
index 1b972eb..cd50b76 100644
--- a/middleware/proxy.go
+++ b/middleware/proxy.go
@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"net/url"
 	"regexp"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -205,8 +206,13 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 	if config.Balancer == nil {
 		panic("echo: proxy middleware requires balancer")
 	}
+	config.rewriteRegex = map[*regexp.Regexp]string{}
 
-	config.rewriteRegex = rewriteRulesRegex(config.Rewrite)
+	// Initialize
+	for k, v := range config.Rewrite {
+		k = strings.Replace(k, "*", "(\\S*)", -1)
+		config.rewriteRegex[regexp.MustCompile(k)] = v
+	}
 
 	return func(next echo.HandlerFunc) echo.HandlerFunc {
 		return func(c echo.Context) (err error) {
@@ -219,8 +225,16 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 			tgt := config.Balancer.Next(c)
 			c.Set(config.ContextKey, tgt)
 
-			// Set rewrite path and raw path
-			rewritePath(config.rewriteRegex, req)
+			// Rewrite
+			for k, v := range config.rewriteRegex {
+				//use req.URL.Path here or else we will have double escaping
+				replacer := captureTokens(k, req.URL.Path)
+				if replacer != nil {
+					if err := rewritePath(replacer, v, req); err != nil {
+						return echo.NewHTTPError(http.StatusBadRequest, "invalid url")
+					}
+				}
+			}
 
 			// Fix header
 			// Basically it's not good practice to unconditionally pass incoming x-real-ip header to upstream.
@@ -251,5 +265,3 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
 		}
 	}
 }
-
-
diff --git a/middleware/rewrite.go b/middleware/rewrite.go
index 0965e31..855c863 100644
--- a/middleware/rewrite.go
+++ b/middleware/rewrite.go
@@ -1,8 +1,11 @@
 package middleware
 
 import (
-	"github.com/labstack/echo/v4"
+	"net/http"
 	"regexp"
+	"strings"
+
+	"github.com/labstack/echo/v4"
 )
 
 type (
@@ -51,8 +54,18 @@ func RewriteWithConfig(config RewriteConfig) echo.MiddlewareFunc {
 	if config.Skipper == nil {
 		config.Skipper = DefaultBodyDumpConfig.Skipper
 	}
+	config.rulesRegex = map[*regexp.Regexp]string{}
 
-	config.rulesRegex = rewriteRulesRegex(config.Rules)
+	// Initialize
+	for k, v := range config.Rules {
+		k = regexp.QuoteMeta(k)
+		k = strings.Replace(k, `\*`, "(.*)", -1)
+		if strings.HasPrefix(k, `\^`) {
+			k = strings.Replace(k, `\^`, "^", -1)
+		}
+		k = k + "$"
+		config.rulesRegex[regexp.MustCompile(k)] = v
+	}
 
 	return func(next echo.HandlerFunc) echo.HandlerFunc {
 		return func(c echo.Context) (err error) {
@@ -61,8 +74,17 @@ func RewriteWithConfig(config RewriteConfig) echo.MiddlewareFunc {
 			}
 
 			req := c.Request()
-			// Set rewrite path and raw path
-			rewritePath(config.rulesRegex, req)
+			// Rewrite
+			for k, v := range config.rulesRegex {
+				//use req.URL.Path here or else we will have double escaping
+				replacer := captureTokens(k, req.URL.Path)
+				if replacer != nil {
+					if err := rewritePath(replacer, v, req); err != nil {
+						return echo.NewHTTPError(http.StatusBadRequest, "invalid url")
+					}
+					break
+				}
+			}
 			return next(c)
 		}
 	}


Test patch:
diff --git a/context_test.go b/context_test.go
index 0044bf8..73e5dcb 100644
--- a/context_test.go
+++ b/context_test.go
@@ -72,15 +72,6 @@ func BenchmarkAllocXML(b *testing.B) {
 	}
 }
 
-func BenchmarkRealIPForHeaderXForwardFor(b *testing.B) {
-	c := context{request: &http.Request{
-		Header: http.Header{HeaderXForwardedFor: []string{"127.0.0.1, 127.0.1.1, "}},
-	}}
-	for i := 0; i < b.N; i++ {
-		c.RealIP()
-	}
-}
-
 func (t *Template) Render(w io.Writer, name string, data interface{}, c Context) error {
 	return t.templates.ExecuteTemplate(w, name, data)
 }
@@ -856,14 +847,6 @@ func TestContext_RealIP(t *testing.T) {
 			},
 			"127.0.0.1",
 		},
-		{
-			&context{
-				request: &http.Request{
-					Header: http.Header{HeaderXForwardedFor: []string{"127.0.0.1"}},
-				},
-			},
-			"127.0.0.1",
-		},
 		{
 			&context{
 				request: &http.Request{
diff --git a/echo_test.go b/echo_test.go
index 0368dbd..e1706ef 100644
--- a/echo_test.go
+++ b/echo_test.go
@@ -568,49 +568,6 @@ func TestHTTPError(t *testing.T) {
 	})
 }
 
-func TestDefaultHTTPErrorHandler(t *testing.T) {
-	e := New()
-	e.Debug = true
-	e.Any("/plain", func(c Context) error {
-		return errors.New("An error occurred")
-	})
-	e.Any("/badrequest", func(c Context) error {
-		return NewHTTPError(http.StatusBadRequest, "Invalid request")
-	})
-	e.Any("/servererror", func(c Context) error {
-		return NewHTTPError(http.StatusInternalServerError, map[string]interface{}{
-			"code":    33,
-			"message": "Something bad happened",
-			"error":   "stackinfo",
-		})
-	})
-	// With Debug=true plain response contains error message
-	c, b := request(http.MethodGet, "/plain", e)
-	assert.Equal(t, http.StatusInternalServerError, c)
-	assert.Equal(t, "{\n  \"error\": \"An error occurred\",\n  \"message\": \"Internal Server Error\"\n}\n", b)
-	// and special handling for HTTPError
-	c, b = request(http.MethodGet, "/badrequest", e)
-	assert.Equal(t, http.StatusBadRequest, c)
-	assert.Equal(t, "{\n  \"error\": \"code=400, message=Invalid request\",\n  \"message\": \"Invalid request\"\n}\n", b)
-	// complex errors are serialized to pretty JSON
-	c, b = request(http.MethodGet, "/servererror", e)
-	assert.Equal(t, http.StatusInternalServerError, c)
-	assert.Equal(t, "{\n  \"code\": 33,\n  \"error\": \"stackinfo\",\n  \"message\": \"Something bad happened\"\n}\n", b)
-
-	e.Debug = false
-	// With Debug=false the error response is shortened
-	c, b = request(http.MethodGet, "/plain", e)
-	assert.Equal(t, http.StatusInternalServerError, c)
-	assert.Equal(t, "{\"message\":\"Internal Server Error\"}\n", b)
-	c, b = request(http.MethodGet, "/badrequest", e)
-	assert.Equal(t, http.StatusBadRequest, c)
-	assert.Equal(t, "{\"message\":\"Invalid request\"}\n", b)
-	// No difference for error response with non plain string errors
-	c, b = request(http.MethodGet, "/servererror", e)
-	assert.Equal(t, http.StatusInternalServerError, c)
-	assert.Equal(t, "{\"code\":33,\"error\":\"stackinfo\",\"message\":\"Something bad happened\"}\n", b)
-}
-
 func TestEchoClose(t *testing.T) {
 	e := New()
 	errCh := make(chan error)
diff --git a/group_test.go b/group_test.go
index c51fd91..d4a6846 100644
--- a/group_test.go
+++ b/group_test.go
@@ -119,3 +119,37 @@ func TestGroupRouteMiddlewareWithMatchAny(t *testing.T) {
 	assert.Equal(t, "/*", m)
 
 }
+
+func TestMultipleGroupSamePathMiddleware(t *testing.T) {
+	// Ensure multiple groups with the same path do not clobber previous routes or mixup middlewares
+	e := New()
+	m1 := func(next HandlerFunc) HandlerFunc {
+		return func(c Context) error {
+			c.Set("middleware", "m1")
+			return next(c)
+		}
+	}
+	m2 := func(next HandlerFunc) HandlerFunc {
+		return func(c Context) error {
+			c.Set("middleware", "m2")
+			return next(c)
+		}
+	}
+	h := func(c Context) error {
+		return c.String(http.StatusOK, c.Get("middleware").(string))
+	}
+
+	g1 := e.Group("/group", m1)
+	{
+		g1.GET("", h)
+	}
+	g2 := e.Group("/group", m2)
+	{
+		g2.GET("/other", h)
+	}
+
+	_, m := request(http.MethodGet, "/group", e)
+	assert.Equal(t, "m1", m)
+	_, m = request(http.MethodGet, "/group/other", e)
+	assert.Equal(t, "m2", m)
+}
diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go
index 534e45f..4bb7464 100644
--- a/middleware/proxy_test.go
+++ b/middleware/proxy_test.go
@@ -94,35 +94,36 @@ func TestProxy(t *testing.T) {
 			"/users/*/orders/*": "/user/$1/order/$2",
 		},
 	}))
-	req.URL, _ = url.Parse("/api/users")
+	req.URL.Path = "/api/users"
 	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
 	assert.Equal(t, "/users", req.URL.EscapedPath())
 	assert.Equal(t, http.StatusOK, rec.Code)
-	req.URL, _ = url.Parse( "/js/main.js")
+	req.URL.Path = "/js/main.js"
 	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
 	assert.Equal(t, "/public/javascripts/main.js", req.URL.EscapedPath())
 	assert.Equal(t, http.StatusOK, rec.Code)
-	req.URL, _ = url.Parse("/old")
+	req.URL.Path = "/old"
 	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
 	assert.Equal(t, "/new", req.URL.EscapedPath())
 	assert.Equal(t, http.StatusOK, rec.Code)
-	req.URL, _ = url.Parse( "/users/jack/orders/1")
+	req.URL.Path = "/users/jack/orders/1"
 	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
 	assert.Equal(t, "/user/jack/order/1", req.URL.EscapedPath())
 	assert.Equal(t, http.StatusOK, rec.Code)
-	req.URL, _ = url.Parse("/user/jill/order/T%2FcO4lW%2Ft%2FVp%2F")
+	req.URL.Path = "/users/jill/orders/T%2FcO4lW%2Ft%2FVp%2F"
 	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
 	assert.Equal(t, "/user/jill/order/T%2FcO4lW%2Ft%2FVp%2F", req.URL.EscapedPath())
 	assert.Equal(t, http.StatusOK, rec.Code)
-	req.URL, _ = url.Parse("/api/new users")
+	req.URL.Path =  "/users/jill/orders/%%%%"
 	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
-	assert.Equal(t, "/new%20users", req.URL.EscapedPath())
+	assert.Equal(t, http.StatusBadRequest, rec.Code)
+
 	// ModifyResponse
 	e = echo.New()
 	e.Use(ProxyWithConfig(ProxyConfig{
diff --git a/middleware/rewrite_test.go b/middleware/rewrite_test.go
index abf11b2..a9b3437 100644
--- a/middleware/rewrite_test.go
+++ b/middleware/rewrite_test.go
@@ -4,7 +4,6 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
-	"net/url"
 	"testing"
 
 	"github.com/labstack/echo/v4"
@@ -24,28 +23,33 @@ func TestRewrite(t *testing.T) {
 	}))
 	req := httptest.NewRequest(http.MethodGet, "/", nil)
 	rec := httptest.NewRecorder()
-	req.URL, _ = url.Parse("/api/users")
+	req.URL.Path = "/api/users"
 	e.ServeHTTP(rec, req)
 	assert.Equal(t, "/users", req.URL.EscapedPath())
-	req.URL, _ = url.Parse("/js/main.js")
+	req.URL.Path = "/js/main.js"
 	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
 	assert.Equal(t, "/public/javascripts/main.js", req.URL.EscapedPath())
-	req.URL, _ = url.Parse("/old")
+	req.URL.Path = "/old"
 	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
 	assert.Equal(t, "/new", req.URL.EscapedPath())
-	req.URL, _ = url.Parse("/users/jack/orders/1")
+	req.URL.Path = "/users/jack/orders/1"
 	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
 	assert.Equal(t, "/user/jack/order/1", req.URL.EscapedPath())
-	req.URL, _ = url.Parse("/user/jill/order/T%2FcO4lW%2Ft%2FVp%2F")
+	req.URL.Path = "/api/new users"
+	rec = httptest.NewRecorder()
+	e.ServeHTTP(rec, req)
+	assert.Equal(t, "/new%20users", req.URL.EscapedPath())
+	req.URL.Path = "/users/jill/orders/T%2FcO4lW%2Ft%2FVp%2F"
 	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
 	assert.Equal(t, "/user/jill/order/T%2FcO4lW%2Ft%2FVp%2F", req.URL.EscapedPath())
-	req.URL, _ = url.Parse("/api/new users")
+	req.URL.Path = "/users/jill/orders/%%%%"
+	rec = httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
-	assert.Equal(t, "/new%20users", req.URL.EscapedPath())
+	assert.Equal(t, http.StatusBadRequest, rec.Code)
 }
 
 // Issue #1086
@@ -54,10 +58,11 @@ func TestEchoRewritePreMiddleware(t *testing.T) {
 	r := e.Router()
 
 	// Rewrite old url to new one
-	e.Pre(Rewrite(map[string]string{
+	e.Pre(RewriteWithConfig(RewriteConfig{
+		Rules: map[string]string{
 			"/old": "/new",
 		},
-	))
+	}))
 
 	// Route
 	r.Add(http.MethodGet, "/new", func(c echo.Context) error {