Skip to content

Commit 71325a6

Browse files
committed
erge branch 'master' into routing_misses_performance_improvements
2 parents 3a6100b + 194129d commit 71325a6

19 files changed

+471
-42
lines changed

.github/stale.yml

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,19 @@
11
# Number of days of inactivity before an issue becomes stale
22
daysUntilStale: 60
33
# Number of days of inactivity before a stale issue is closed
4-
daysUntilClose: 7
4+
daysUntilClose: 30
55
# Issues with these labels will never be considered stale
66
exemptLabels:
77
- pinned
88
- security
9+
- bug
10+
- enhancement
911
# Label to use when marking an issue as stale
10-
staleLabel: wontfix
12+
staleLabel: stale
1113
# Comment to post when marking an issue as stale. Set to `false` to disable
1214
markComment: >
1315
This issue has been automatically marked as stale because it has not had
14-
recent activity. It will be closed if no further activity occurs. Thank you
15-
for your contributions.
16+
recent activity. It will be closed within a month if no further activity occurs.
17+
Thank you for your contributions.
1618
# Comment to post when closing a stale issue. Set to `false` to disable
17-
closeComment: false
19+
closeComment: false

.github/workflows/echo.yml

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ on:
99
- 'go.*'
1010
- '_fixture/**'
1111
- '.github/**'
12+
- 'codecov.yml'
1213
pull_request:
1314
branches:
1415
- master
@@ -17,6 +18,7 @@ on:
1718
- 'go.*'
1819
- '_fixture/**'
1920
- '.github/**'
21+
- 'codecov.yml'
2022

2123
jobs:
2224
test:
@@ -62,3 +64,55 @@ jobs:
6264
with:
6365
token:
6466
fail_ci_if_error: false
67+
benchmark:
68+
needs: test
69+
strategy:
70+
matrix:
71+
os: [ubuntu-latest]
72+
go: [1.15]
73+
name: Benchmark comparison ${{ matrix.os }} @ Go ${{ matrix.go }}
74+
runs-on: ${{ matrix.os }}
75+
steps:
76+
- name: Set up Go ${{ matrix.go }}
77+
uses: actions/setup-go@v1
78+
with:
79+
go-version: ${{ matrix.go }}
80+
81+
- name: Set GOPATH and PATH
82+
run: |
83+
echo "GOPATH=$(dirname $GITHUB_WORKSPACE)" >> $GITHUB_ENV
84+
echo "$(dirname $GITHUB_WORKSPACE)/bin" >> $GITHUB_PATH
85+
shell: bash
86+
87+
- name: Set build variables
88+
run: |
89+
echo "GOPROXY=https://proxy.golang.org" >> $GITHUB_ENV
90+
echo "GO111MODULE=on" >> $GITHUB_ENV
91+
92+
- name: Checkout Code (Previous)
93+
uses: actions/checkout@v2
94+
with:
95+
ref: ${{ github.base_ref }}
96+
path: previous
97+
98+
- name: Checkout Code (New)
99+
uses: actions/checkout@v2
100+
with:
101+
path: new
102+
103+
- name: Install Dependencies
104+
run: go get -v golang.org/x/perf/cmd/benchstat
105+
106+
- name: Run Benchmark (Previous)
107+
run: |
108+
cd previous
109+
go test -run="-" -bench=".*" -count=8 ./... > benchmark.txt
110+
111+
- name: Run Benchmark (New)
112+
run: |
113+
cd new
114+
go test -run="-" -bench=".*" -count=8 ./... > benchmark.txt
115+
116+
- name: Run Benchstat
117+
run: |
118+
benchstat previous/benchmark.txt new/benchmark.txt

_fixture/_fixture/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
This directory is used for the static middleware test

codecov.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
coverage:
2+
status:
3+
project:
4+
default:
5+
threshold: 1%
6+
patch:
7+
default:
8+
threshold: 1%
9+
10+
comment:
11+
require_changes: true

echo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ func (e *Echo) Reverse(name string, params ...interface{}) string {
572572
for _, r := range e.router.routes {
573573
if r.Name == name {
574574
for i, l := 0, len(r.Path); i < l; i++ {
575-
if r.Path[i] == ':' && n < ln {
575+
if (r.Path[i] == ':' || r.Path[i] == '*') && n < ln {
576576
for ; i < l && r.Path[i] != '/'; i++ {
577577
}
578578
uri.WriteString(fmt.Sprintf("%v", params[n]))

echo_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,10 +277,12 @@ func TestEchoURL(t *testing.T) {
277277
e := New()
278278
static := func(Context) error { return nil }
279279
getUser := func(Context) error { return nil }
280+
getAny := func(Context) error { return nil }
280281
getFile := func(Context) error { return nil }
281282

282283
e.GET("/static/file", static)
283284
e.GET("/users/:id", getUser)
285+
e.GET("/documents/*", getAny)
284286
g := e.Group("/group")
285287
g.GET("/users/:uid/files/:fid", getFile)
286288

@@ -289,6 +291,9 @@ func TestEchoURL(t *testing.T) {
289291
assert.Equal("/static/file", e.URL(static))
290292
assert.Equal("/users/:id", e.URL(getUser))
291293
assert.Equal("/users/1", e.URL(getUser, "1"))
294+
assert.Equal("/users/1", e.URL(getUser, "1"))
295+
assert.Equal("/documents/foo.txt", e.URL(getAny, "foo.txt"))
296+
assert.Equal("/documents/*", e.URL(getAny))
292297
assert.Equal("/group/users/1/files/:fid", e.URL(getFile, "1"))
293298
assert.Equal("/group/users/1/files/1", e.URL(getFile, "1", "1"))
294299
}
@@ -652,3 +657,28 @@ func TestEchoShutdown(t *testing.T) {
652657
err := <-errCh
653658
assert.Equal(t, err.Error(), "http: Server closed")
654659
}
660+
661+
func TestEchoReverse(t *testing.T) {
662+
assert := assert.New(t)
663+
664+
e := New()
665+
dummyHandler := func(Context) error { return nil }
666+
667+
e.GET("/static", dummyHandler).Name = "/static"
668+
e.GET("/static/*", dummyHandler).Name = "/static/*"
669+
e.GET("/params/:foo", dummyHandler).Name = "/params/:foo"
670+
e.GET("/params/:foo/bar/:qux", dummyHandler).Name = "/params/:foo/bar/:qux"
671+
e.GET("/params/:foo/bar/:qux/*", dummyHandler).Name = "/params/:foo/bar/:qux/*"
672+
673+
assert.Equal("/static", e.Reverse("/static"))
674+
assert.Equal("/static", e.Reverse("/static", "missing param"))
675+
assert.Equal("/static/*", e.Reverse("/static/*"))
676+
assert.Equal("/static/foo.txt", e.Reverse("/static/*", "foo.txt"))
677+
678+
assert.Equal("/params/:foo", e.Reverse("/params/:foo"))
679+
assert.Equal("/params/one", e.Reverse("/params/:foo", "one"))
680+
assert.Equal("/params/:foo/bar/:qux", e.Reverse("/params/:foo/bar/:qux"))
681+
assert.Equal("/params/one/bar/:qux", e.Reverse("/params/:foo/bar/:qux", "one"))
682+
assert.Equal("/params/one/bar/two", e.Reverse("/params/:foo/bar/:qux", "one", "two"))
683+
assert.Equal("/params/one/bar/two/three", e.Reverse("/params/:foo/bar/:qux/*", "one", "two", "three"))
684+
}

middleware/compress.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func GzipWithConfig(config GzipConfig) echo.MiddlewareFunc {
5959
config.Level = DefaultGzipConfig.Level
6060
}
6161

62-
pool := gzipPool(config)
62+
pool := gzipCompressPool(config)
6363

6464
return func(next echo.HandlerFunc) echo.HandlerFunc {
6565
return func(c echo.Context) error {
@@ -133,7 +133,7 @@ func (w *gzipResponseWriter) Push(target string, opts *http.PushOptions) error {
133133
return http.ErrNotSupported
134134
}
135135

136-
func gzipPool(config GzipConfig) sync.Pool {
136+
func gzipCompressPool(config GzipConfig) sync.Pool {
137137
return sync.Pool{
138138
New: func() interface{} {
139139
w, err := gzip.NewWriterLevel(ioutil.Discard, config.Level)

middleware/cors.go

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ type (
1919
// Optional. Default value []string{"*"}.
2020
AllowOrigins []string `yaml:"allow_origins"`
2121

22+
// AllowOriginFunc is a custom function to validate the origin. It takes the
23+
// origin as an argument and returns true if allowed or false otherwise. If
24+
// an error is returned, it is returned by the handler. If this option is
25+
// set, AllowOrigins is ignored.
26+
// Optional.
27+
AllowOriginFunc func(origin string) (bool, error) `yaml:"allow_origin_func"`
28+
2229
// AllowMethods defines a list methods allowed when accessing the resource.
2330
// This is used in response to a preflight request.
2431
// Optional. Default value DefaultCORSConfig.AllowMethods.
@@ -113,40 +120,50 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
113120
return c.NoContent(http.StatusNoContent)
114121
}
115122

116-
// Check allowed origins
117-
for _, o := range config.AllowOrigins {
118-
if o == "*" && config.AllowCredentials {
119-
allowOrigin = origin
120-
break
121-
}
122-
if o == "*" || o == origin {
123-
allowOrigin = o
124-
break
123+
if config.AllowOriginFunc != nil {
124+
allowed, err := config.AllowOriginFunc(origin)
125+
if err != nil {
126+
return err
125127
}
126-
if matchSubdomain(origin, o) {
128+
if allowed {
127129
allowOrigin = origin
128-
break
129130
}
130-
}
131-
132-
// Check allowed origin patterns
133-
for _, re := range allowOriginPatterns {
134-
if allowOrigin == "" {
135-
didx := strings.Index(origin, "://")
136-
if didx == -1 {
137-
continue
131+
} else {
132+
// Check allowed origins
133+
for _, o := range config.AllowOrigins {
134+
if o == "*" && config.AllowCredentials {
135+
allowOrigin = origin
136+
break
138137
}
139-
domAuth := origin[didx+3:]
140-
// to avoid regex cost by invalid long domain
141-
if len(domAuth) > 253 {
138+
if o == "*" || o == origin {
139+
allowOrigin = o
142140
break
143141
}
144-
145-
if match, _ := regexp.MatchString(re, origin); match {
142+
if matchSubdomain(origin, o) {
146143
allowOrigin = origin
147144
break
148145
}
149146
}
147+
148+
// Check allowed origin patterns
149+
for _, re := range allowOriginPatterns {
150+
if allowOrigin == "" {
151+
didx := strings.Index(origin, "://")
152+
if didx == -1 {
153+
continue
154+
}
155+
domAuth := origin[didx+3:]
156+
// to avoid regex cost by invalid long domain
157+
if len(domAuth) > 253 {
158+
break
159+
}
160+
161+
if match, _ := regexp.MatchString(re, origin); match {
162+
allowOrigin = origin
163+
break
164+
}
165+
}
166+
}
150167
}
151168

152169
// Origin not allowed

middleware/cors_test.go

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

33
import (
4+
"errors"
45
"net/http"
56
"net/http/httptest"
67
"testing"
@@ -360,3 +361,49 @@ func TestCorsHeaders(t *testing.T) {
360361
}
361362
}
362363
}
364+
365+
func Test_allowOriginFunc(t *testing.T) {
366+
returnTrue := func(origin string) (bool, error) {
367+
return true, nil
368+
}
369+
returnFalse := func(origin string) (bool, error) {
370+
return false, nil
371+
}
372+
returnError := func(origin string) (bool, error) {
373+
return true, errors.New("this is a test error")
374+
}
375+
376+
allowOriginFuncs := []func(origin string) (bool, error){
377+
returnTrue,
378+
returnFalse,
379+
returnError,
380+
}
381+
382+
const origin = "http://example.com"
383+
384+
e := echo.New()
385+
for _, allowOriginFunc := range allowOriginFuncs {
386+
req := httptest.NewRequest(http.MethodOptions, "/", nil)
387+
rec := httptest.NewRecorder()
388+
c := e.NewContext(req, rec)
389+
req.Header.Set(echo.HeaderOrigin, origin)
390+
cors := CORSWithConfig(CORSConfig{
391+
AllowOriginFunc: allowOriginFunc,
392+
})
393+
h := cors(echo.NotFoundHandler)
394+
err := h(c)
395+
396+
expected, expectedErr := allowOriginFunc(origin)
397+
if expectedErr != nil {
398+
assert.Equal(t, expectedErr, err)
399+
assert.Equal(t, "", rec.Header().Get(echo.HeaderAccessControlAllowOrigin))
400+
continue
401+
}
402+
403+
if expected {
404+
assert.Equal(t, origin, rec.Header().Get(echo.HeaderAccessControlAllowOrigin))
405+
} else {
406+
assert.Equal(t, "", rec.Header().Get(echo.HeaderAccessControlAllowOrigin))
407+
}
408+
}
409+
}

0 commit comments

Comments
 (0)