Skip to content

Commit ceffc10

Browse files
lammellammel
authored
Merge pull request #1630 from arun0009/master
bugfix proxy and rewrite, updated test with actual call settings
2 parents 6b48de3 + f6dfcbe commit ceffc10

File tree

5 files changed

+48
-75
lines changed

5 files changed

+48
-75
lines changed

‎middleware/middleware.go

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

33
import (
44
"net/http"
5-
"net/url"
65
"regexp"
76
"strconv"
87
"strings"
@@ -34,15 +33,29 @@ func captureTokens(pattern *regexp.Regexp, input string) *strings.Replacer {
3433
return strings.NewReplacer(replace...)
3534
}
3635

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
36+
func rewriteRulesRegex(rewrite map[string]string) map[*regexp.Regexp]string {
37+
// Initialize
38+
rulesRegex := map[*regexp.Regexp]string{}
39+
for k, v := range rewrite {
40+
k = regexp.QuoteMeta(k)
41+
k = strings.Replace(k, `\*`, "(.*)", -1)
42+
if strings.HasPrefix(k, `\^`) {
43+
k = strings.Replace(k, `\^`, "^", -1)
44+
}
45+
k = k + "$"
46+
rulesRegex[regexp.MustCompile(k)] = v
47+
}
48+
return rulesRegex
49+
}
50+
51+
func rewritePath(rewriteRegex map[*regexp.Regexp]string, req *http.Request) {
52+
for k, v := range rewriteRegex {
53+
replacerRawPath := captureTokens(k, req.URL.EscapedPath())
54+
if replacerRawPath != nil {
55+
replacerPath := captureTokens(k, req.URL.Path)
56+
req.URL.RawPath, req.URL.Path = replacerRawPath.Replace(v), replacerPath.Replace(v)
57+
}
4358
}
44-
req.URL.Path, req.URL.RawPath = replacerPath, replacerRawPath
45-
return nil
4659
}
4760

4861
// DefaultSkipper returns false which processes the middleware.

‎middleware/proxy.go

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"net/http"
99
"net/url"
1010
"regexp"
11-
"strings"
1211
"sync"
1312
"sync/atomic"
1413
"time"
@@ -206,13 +205,8 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
206205
if config.Balancer == nil {
207206
panic("echo: proxy middleware requires balancer")
208207
}
209-
config.rewriteRegex = map[*regexp.Regexp]string{}
210208

211-
// Initialize
212-
for k, v := range config.Rewrite {
213-
k = strings.Replace(k, "*", "(\\S*)", -1)
214-
config.rewriteRegex[regexp.MustCompile(k)] = v
215-
}
209+
config.rewriteRegex = rewriteRulesRegex(config.Rewrite)
216210

217211
return func(next echo.HandlerFunc) echo.HandlerFunc {
218212
return func(c echo.Context) (err error) {
@@ -225,16 +219,8 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
225219
tgt := config.Balancer.Next(c)
226220
c.Set(config.ContextKey, tgt)
227221

228-
// Rewrite
229-
for k, v := range config.rewriteRegex {
230-
//use req.URL.Path here or else we will have double escaping
231-
replacer := captureTokens(k, req.URL.Path)
232-
if replacer != nil {
233-
if err := rewritePath(replacer, v, req); err != nil {
234-
return echo.NewHTTPError(http.StatusBadRequest, "invalid url")
235-
}
236-
}
237-
}
222+
// Set rewrite path and raw path
223+
rewritePath(config.rewriteRegex, req)
238224

239225
// Fix header
240226
// Basically it's not good practice to unconditionally pass incoming x-real-ip header to upstream.
@@ -265,3 +251,5 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc {
265251
}
266252
}
267253
}
254+
255+

‎middleware/proxy_test.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,36 +94,35 @@ func TestProxy(t *testing.T) {
9494
"/users/*/orders/*": "/user/$1/order/$2",
9595
},
9696
}))
97-
req.URL.Path = "/api/users"
97+
req.URL, _ = url.Parse("/api/users")
9898
rec = httptest.NewRecorder()
9999
e.ServeHTTP(rec, req)
100100
assert.Equal(t, "/users", req.URL.EscapedPath())
101101
assert.Equal(t, http.StatusOK, rec.Code)
102-
req.URL.Path = "/js/main.js"
102+
req.URL, _ = url.Parse( "/js/main.js")
103103
rec = httptest.NewRecorder()
104104
e.ServeHTTP(rec, req)
105105
assert.Equal(t, "/public/javascripts/main.js", req.URL.EscapedPath())
106106
assert.Equal(t, http.StatusOK, rec.Code)
107-
req.URL.Path = "/old"
107+
req.URL, _ = url.Parse("/old")
108108
rec = httptest.NewRecorder()
109109
e.ServeHTTP(rec, req)
110110
assert.Equal(t, "/new", req.URL.EscapedPath())
111111
assert.Equal(t, http.StatusOK, rec.Code)
112-
req.URL.Path = "/users/jack/orders/1"
112+
req.URL, _ = url.Parse( "/users/jack/orders/1")
113113
rec = httptest.NewRecorder()
114114
e.ServeHTTP(rec, req)
115115
assert.Equal(t, "/user/jack/order/1", req.URL.EscapedPath())
116116
assert.Equal(t, http.StatusOK, rec.Code)
117-
req.URL.Path = "/users/jill/orders/T%2FcO4lW%2Ft%2FVp%2F"
117+
req.URL, _ = url.Parse("/user/jill/order/T%2FcO4lW%2Ft%2FVp%2F")
118118
rec = httptest.NewRecorder()
119119
e.ServeHTTP(rec, req)
120120
assert.Equal(t, "/user/jill/order/T%2FcO4lW%2Ft%2FVp%2F", req.URL.EscapedPath())
121121
assert.Equal(t, http.StatusOK, rec.Code)
122-
req.URL.Path = "/users/jill/orders/%%%%"
122+
req.URL, _ = url.Parse("/api/new users")
123123
rec = httptest.NewRecorder()
124124
e.ServeHTTP(rec, req)
125-
assert.Equal(t, http.StatusBadRequest, rec.Code)
126-
125+
assert.Equal(t, "/new%20users", req.URL.EscapedPath())
127126
// ModifyResponse
128127
e = echo.New()
129128
e.Use(ProxyWithConfig(ProxyConfig{

‎middleware/rewrite.go

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

33
import (
4-
"net/http"
5-
"regexp"
6-
"strings"
7-
84
"github.com/labstack/echo/v4"
5+
"regexp"
96
)
107

118
type (
@@ -54,18 +51,8 @@ func RewriteWithConfig(config RewriteConfig) echo.MiddlewareFunc {
5451
if config.Skipper == nil {
5552
config.Skipper = DefaultBodyDumpConfig.Skipper
5653
}
57-
config.rulesRegex = map[*regexp.Regexp]string{}
5854

59-
// Initialize
60-
for k, v := range config.Rules {
61-
k = regexp.QuoteMeta(k)
62-
k = strings.Replace(k, `\*`, "(.*)", -1)
63-
if strings.HasPrefix(k, `\^`) {
64-
k = strings.Replace(k, `\^`, "^", -1)
65-
}
66-
k = k + "$"
67-
config.rulesRegex[regexp.MustCompile(k)] = v
68-
}
55+
config.rulesRegex = rewriteRulesRegex(config.Rules)
6956

7057
return func(next echo.HandlerFunc) echo.HandlerFunc {
7158
return func(c echo.Context) (err error) {
@@ -74,17 +61,8 @@ func RewriteWithConfig(config RewriteConfig) echo.MiddlewareFunc {
7461
}
7562

7663
req := c.Request()
77-
// Rewrite
78-
for k, v := range config.rulesRegex {
79-
//use req.URL.Path here or else we will have double escaping
80-
replacer := captureTokens(k, req.URL.Path)
81-
if replacer != nil {
82-
if err := rewritePath(replacer, v, req); err != nil {
83-
return echo.NewHTTPError(http.StatusBadRequest, "invalid url")
84-
}
85-
break
86-
}
87-
}
64+
// Set rewrite path and raw path
65+
rewritePath(config.rulesRegex, req)
8866
return next(c)
8967
}
9068
}

‎middleware/rewrite_test.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"io/ioutil"
55
"net/http"
66
"net/http/httptest"
7+
"net/url"
78
"testing"
89

910
"github.com/labstack/echo/v4"
@@ -23,33 +24,28 @@ func TestRewrite(t *testing.T) {
2324
}))
2425
req := httptest.NewRequest(http.MethodGet, "/", nil)
2526
rec := httptest.NewRecorder()
26-
req.URL.Path = "/api/users"
27+
req.URL, _ = url.Parse("/api/users")
2728
e.ServeHTTP(rec, req)
2829
assert.Equal(t, "/users", req.URL.EscapedPath())
29-
req.URL.Path = "/js/main.js"
30+
req.URL, _ = url.Parse("/js/main.js")
3031
rec = httptest.NewRecorder()
3132
e.ServeHTTP(rec, req)
3233
assert.Equal(t, "/public/javascripts/main.js", req.URL.EscapedPath())
33-
req.URL.Path = "/old"
34+
req.URL, _ = url.Parse("/old")
3435
rec = httptest.NewRecorder()
3536
e.ServeHTTP(rec, req)
3637
assert.Equal(t, "/new", req.URL.EscapedPath())
37-
req.URL.Path = "/users/jack/orders/1"
38+
req.URL, _ = url.Parse("/users/jack/orders/1")
3839
rec = httptest.NewRecorder()
3940
e.ServeHTTP(rec, req)
4041
assert.Equal(t, "/user/jack/order/1", req.URL.EscapedPath())
41-
req.URL.Path = "/api/new users"
42-
rec = httptest.NewRecorder()
43-
e.ServeHTTP(rec, req)
44-
assert.Equal(t, "/new%20users", req.URL.EscapedPath())
45-
req.URL.Path = "/users/jill/orders/T%2FcO4lW%2Ft%2FVp%2F"
42+
req.URL, _ = url.Parse("/user/jill/order/T%2FcO4lW%2Ft%2FVp%2F")
4643
rec = httptest.NewRecorder()
4744
e.ServeHTTP(rec, req)
4845
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()
46+
req.URL, _ = url.Parse("/api/new users")
5147
e.ServeHTTP(rec, req)
52-
assert.Equal(t, http.StatusBadRequest, rec.Code)
48+
assert.Equal(t, "/new%20users", req.URL.EscapedPath())
5349
}
5450

5551
// Issue #1086
@@ -58,11 +54,10 @@ func TestEchoRewritePreMiddleware(t *testing.T) {
5854
r := e.Router()
5955

6056
// Rewrite old url to new one
61-
e.Pre(RewriteWithConfig(RewriteConfig{
62-
Rules: map[string]string{
57+
e.Pre(Rewrite(map[string]string{
6358
"/old": "/new",
6459
},
65-
}))
60+
))
6661

6762
// Route
6863
r.Add(http.MethodGet, "/new", func(c echo.Context) error {

0 commit comments

Comments
 (0)