Skip to content

Commit 1a6ec73

Browse files
author
Arun Gopalpuri
committed
using url.EscapedPath instead of custom GetPath, rewritePath func added to middleware - used by proxy and rewrite
1 parent cb84205 commit 1a6ec73

File tree

6 files changed

+63
-28
lines changed

6 files changed

+63
-28
lines changed

‎echo.go

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -612,16 +612,15 @@ func (e *Echo) ServeHTTP(w http.ResponseWriter, r *http.Request) {
612612
// Acquire context
613613
c := e.pool.Get().(*context)
614614
c.Reset(r, w)
615-
616615
h := NotFoundHandler
617616

618617
if e.premiddleware == nil {
619-
e.findRouter(r.Host).Find(r.Method, GetPath(r), c)
618+
e.findRouter(r.Host).Find(r.Method, r.URL.EscapedPath(), c)
620619
h = c.Handler()
621620
h = applyMiddleware(h, e.middleware...)
622621
} else {
623622
h = func(c Context) error {
624-
e.findRouter(r.Host).Find(r.Method, GetPath(r), c)
623+
e.findRouter(r.Host).Find(r.Method, r.URL.EscapedPath(), c)
625624
h := c.Handler()
626625
h = applyMiddleware(h, e.middleware...)
627626
return h(c)
@@ -832,15 +831,6 @@ func WrapMiddleware(m func(http.Handler) http.Handler) MiddlewareFunc {
832831
}
833832
}
834833

835-
// GetPath returns RawPath, if it's empty returns Path from URL
836-
func GetPath(r *http.Request) string {
837-
path := r.URL.RawPath
838-
if path == "" {
839-
path = r.URL.Path
840-
}
841-
return path
842-
}
843-
844834
func (e *Echo) findRouter(host string) *Router {
845835
if len(e.routers) > 0 {
846836
if r, ok := e.routers[host]; ok {

‎middleware/middleware.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package middleware
22

33
import (
4+
"net/http"
5+
"net/url"
46
"regexp"
57
"strconv"
68
"strings"
@@ -32,6 +34,17 @@ func captureTokens(pattern *regexp.Regexp, input string) *strings.Replacer {
3234
return strings.NewReplacer(replace...)
3335
}
3436

37+
//rewritePath sets request url path and raw path
38+
func rewritePath(replacer *strings.Replacer, target string, req *http.Request) error {
39+
replacerRawPath := replacer.Replace(target)
40+
replacerPath, err := url.PathUnescape(replacerRawPath)
41+
if err != nil {
42+
return err
43+
}
44+
req.URL.Path, req.URL.RawPath = replacerPath, replacerRawPath
45+
return nil
46+
}
47+
3548
// DefaultSkipper returns false which processes the middleware.
3649
func DefaultSkipper(echo.Context) bool {
3750
return false

‎middleware/proxy.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,12 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
227227

228228
// Rewrite
229229
for k, v := range config.rewriteRegex {
230-
replacer := captureTokens(k, echo.GetPath(req))
230+
//use req.URL.Path here or else we will have double escaping
231+
replacer := captureTokens(k, req.URL.Path)
231232
if replacer != nil {
232-
req.URL.Path = replacer.Replace(v)
233+
if err := rewritePath(replacer, v, req); err != nil {
234+
return echo.NewHTTPError(http.StatusBadRequest, "invalid url")
235+
}
233236
}
234237
}
235238

‎middleware/proxy_test.go

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/stretchr/testify/assert"
1515
)
1616

17+
//Assert expected with url.EscapedPath method to obtain the path.
1718
func TestProxy(t *testing.T) {
1819
// Setup
1920
t1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -94,22 +95,34 @@ func TestProxy(t *testing.T) {
9495
},
9596
}))
9697
req.URL.Path = "/api/users"
98+
rec = httptest.NewRecorder()
9799
e.ServeHTTP(rec, req)
98-
assert.Equal(t, "/users", req.URL.Path)
100+
assert.Equal(t, "/users", req.URL.EscapedPath())
101+
assert.Equal(t, http.StatusOK, rec.Code)
99102
req.URL.Path = "/js/main.js"
103+
rec = httptest.NewRecorder()
100104
e.ServeHTTP(rec, req)
101-
assert.Equal(t, "/public/javascripts/main.js", req.URL.Path)
105+
assert.Equal(t, "/public/javascripts/main.js", req.URL.EscapedPath())
106+
assert.Equal(t, http.StatusOK, rec.Code)
102107
req.URL.Path = "/old"
108+
rec = httptest.NewRecorder()
103109
e.ServeHTTP(rec, req)
104-
assert.Equal(t, "/new", req.URL.Path)
110+
assert.Equal(t, "/new", req.URL.EscapedPath())
111+
assert.Equal(t, http.StatusOK, rec.Code)
105112
req.URL.Path = "/users/jack/orders/1"
113+
rec = httptest.NewRecorder()
106114
e.ServeHTTP(rec, req)
107-
assert.Equal(t, "/user/jack/order/1", req.URL.Path)
115+
assert.Equal(t, "/user/jack/order/1", req.URL.EscapedPath())
108116
assert.Equal(t, http.StatusOK, rec.Code)
109117
req.URL.Path = "/users/jill/orders/T%2FcO4lW%2Ft%2FVp%2F"
118+
rec = httptest.NewRecorder()
110119
e.ServeHTTP(rec, req)
111-
assert.Equal(t, "/user/jill/order/T%2FcO4lW%2Ft%2FVp%2F", req.URL.Path)
120+
assert.Equal(t, "/user/jill/order/T%2FcO4lW%2Ft%2FVp%2F", req.URL.EscapedPath())
112121
assert.Equal(t, http.StatusOK, rec.Code)
122+
req.URL.Path = "/users/jill/orders/%%%%"
123+
rec = httptest.NewRecorder()
124+
e.ServeHTTP(rec, req)
125+
assert.Equal(t, http.StatusBadRequest, rec.Code)
113126

114127
// ModifyResponse
115128
e = echo.New()

‎middleware/rewrite.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package middleware
22

33
import (
4+
"net/http"
45
"regexp"
56
"strings"
67

@@ -70,12 +71,14 @@ func RewriteWithConfig(config RewriteConfig) echo.MiddlewareFunc {
7071
}
7172

7273
req := c.Request()
73-
7474
// Rewrite
7575
for k, v := range config.rulesRegex {
76+
//use req.URL.Path here or else we will have double escaping
7677
replacer := captureTokens(k, req.URL.Path)
7778
if replacer != nil {
78-
req.URL.Path = replacer.Replace(v)
79+
if err := rewritePath(replacer, v, req); err != nil {
80+
return echo.NewHTTPError(http.StatusBadRequest, "invalid url")
81+
}
7982
break
8083
}
8184
}

‎middleware/rewrite_test.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/stretchr/testify/assert"
1111
)
1212

13+
//Assert expected with url.EscapedPath method to obtain the path.
1314
func TestRewrite(t *testing.T) {
1415
e := echo.New()
1516
e.Use(RewriteWithConfig(RewriteConfig{
@@ -24,19 +25,31 @@ func TestRewrite(t *testing.T) {
2425
rec := httptest.NewRecorder()
2526
req.URL.Path = "/api/users"
2627
e.ServeHTTP(rec, req)
27-
assert.Equal(t, "/users", req.URL.Path)
28+
assert.Equal(t, "/users", req.URL.EscapedPath())
2829
req.URL.Path = "/js/main.js"
30+
rec = httptest.NewRecorder()
2931
e.ServeHTTP(rec, req)
30-
assert.Equal(t, "/public/javascripts/main.js", req.URL.Path)
32+
assert.Equal(t, "/public/javascripts/main.js", req.URL.EscapedPath())
3133
req.URL.Path = "/old"
34+
rec = httptest.NewRecorder()
3235
e.ServeHTTP(rec, req)
33-
assert.Equal(t, "/new", req.URL.Path)
36+
assert.Equal(t, "/new", req.URL.EscapedPath())
3437
req.URL.Path = "/users/jack/orders/1"
38+
rec = httptest.NewRecorder()
3539
e.ServeHTTP(rec, req)
36-
assert.Equal(t, "/user/jack/order/1", req.URL.Path)
40+
assert.Equal(t, "/user/jack/order/1", req.URL.EscapedPath())
3741
req.URL.Path = "/api/new users"
42+
rec = httptest.NewRecorder()
3843
e.ServeHTTP(rec, req)
39-
assert.Equal(t, "/new users", req.URL.Path)
44+
assert.Equal(t, "/new%20users", req.URL.EscapedPath())
45+
req.URL.Path = "/users/jill/orders/T%2FcO4lW%2Ft%2FVp%2F"
46+
rec = httptest.NewRecorder()
47+
e.ServeHTTP(rec, req)
48+
assert.Equal(t, "/user/jill/order/T%2FcO4lW%2Ft%2FVp%2F", req.URL.EscapedPath())
49+
req.URL.Path = "/users/jill/orders/%%%%"
50+
rec = httptest.NewRecorder()
51+
e.ServeHTTP(rec, req)
52+
assert.Equal(t, http.StatusBadRequest, rec.Code)
4053
}
4154

4255
// Issue #1086
@@ -59,7 +72,7 @@ func TestEchoRewritePreMiddleware(t *testing.T) {
5972
req := httptest.NewRequest(http.MethodGet, "/old", nil)
6073
rec := httptest.NewRecorder()
6174
e.ServeHTTP(rec, req)
62-
assert.Equal(t, "/new", req.URL.Path)
75+
assert.Equal(t, "/new", req.URL.EscapedPath())
6376
assert.Equal(t, 200, rec.Code)
6477
}
6578

@@ -86,7 +99,7 @@ func TestRewriteWithConfigPreMiddleware_Issue1143(t *testing.T) {
8699
req := httptest.NewRequest(http.MethodGet, "/api/v1/mgmt/proj/test/agt", nil)
87100
rec := httptest.NewRecorder()
88101
e.ServeHTTP(rec, req)
89-
assert.Equal(t, "/api/v1/hosts/test", req.URL.Path)
102+
assert.Equal(t, "/api/v1/hosts/test", req.URL.EscapedPath())
90103
assert.Equal(t, 200, rec.Code)
91104

92105
defer rec.Result().Body.Close()

0 commit comments

Comments
 (0)