Merge branch 'master' into panic_router_find_fails_on_params_with_no_root

Author: pafuent

.github/stale.yml

@@ -1,17 +1,19 @@
 # Number of days of inactivity before an issue becomes stale
 daysUntilStale: 60
 # Number of days of inactivity before a stale issue is closed
-daysUntilClose: 7
+daysUntilClose: 30
 # Issues with these labels will never be considered stale
 exemptLabels:
   - pinned
   - security
+  - bug
+  - enhancement
 # Label to use when marking an issue as stale
-staleLabel: wontfix
+staleLabel: stale
 # Comment to post when marking an issue as stale. Set to `false` to disable
 markComment: >
   This issue has been automatically marked as stale because it has not had
-  recent activity. It will be closed if no further activity occurs. Thank you
-  for your contributions.
+  recent activity. It will be closed within a month if no further activity occurs.
+  Thank you for your contributions.
 # Comment to post when closing a stale issue. Set to `false` to disable
-closeComment: false
+closeComment: false

.github/workflows/echo.yml

@@ -4,20 +4,28 @@ on:
   push:
     branches:
       - master
+    paths:
+      - '**.go'
+      - 'go.*'
+      - '_fixture/**'
+      - '.github/**'
+      - 'codecov.yml'
   pull_request:
     branches:
       - master
-
-env:
-  GO111MODULE: on
-  GOPROXY: https://proxy.golang.org
+    paths:
+      - '**.go'
+      - 'go.*'
+      - '_fixture/**'
+      - '.github/**'
+      - 'codecov.yml'
 
 jobs:
   test:
     strategy:
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        go: [1.12, 1.13, 1.14]
+        go: [1.12, 1.13, 1.14, 1.15]
     name: ${{ matrix.os }} @ Go ${{ matrix.go }}
     runs-on: ${{ matrix.os }}
     steps:
@@ -28,10 +36,15 @@ jobs:
 
       - name: Set GOPATH and PATH
         run: |
-          echo "::set-env name=GOPATH::$(dirname $GITHUB_WORKSPACE)"
-          echo "::add-path::$(dirname $GITHUB_WORKSPACE)/bin"
+          echo "GOPATH=$(dirname $GITHUB_WORKSPACE)" >> $GITHUB_ENV
+          echo "$(dirname $GITHUB_WORKSPACE)/bin" >> $GITHUB_PATH
         shell: bash
 
+      - name: Set build variables
+        run: |
+          echo "GOPROXY=https://proxy.golang.org" >> $GITHUB_ENV
+          echo "GO111MODULE=on" >> $GITHUB_ENV
+
       - name: Checkout Code
         uses: actions/checkout@v1
         with:
@@ -51,3 +64,55 @@ jobs:
         with:
           token:
           fail_ci_if_error: false
+  benchmark:
+    needs: test
+    strategy:
+      matrix:
+        os: [ubuntu-latest]
+        go: [1.15]
+    name: Benchmark comparison ${{ matrix.os }} @ Go ${{ matrix.go }}
+    runs-on: ${{ matrix.os }}
+    steps:
+      - name: Set up Go ${{ matrix.go }}
+        uses: actions/setup-go@v1
+        with:
+          go-version: ${{ matrix.go }}
+
+      - name: Set GOPATH and PATH
+        run: |
+          echo "GOPATH=$(dirname $GITHUB_WORKSPACE)" >> $GITHUB_ENV
+          echo "$(dirname $GITHUB_WORKSPACE)/bin" >> $GITHUB_PATH
+        shell: bash
+
+      - name: Set build variables
+        run: |
+          echo "GOPROXY=https://proxy.golang.org" >> $GITHUB_ENV
+          echo "GO111MODULE=on" >> $GITHUB_ENV
+
+      - name: Checkout Code (Previous)
+        uses: actions/checkout@v2
+        with:
+          ref: ${{ github.base_ref }}
+          path: previous
+
+      - name: Checkout Code (New)
+        uses: actions/checkout@v2
+        with:
+          path: new
+
+      - name: Install Dependencies
+        run: go get -v golang.org/x/perf/cmd/benchstat
+
+      - name: Run Benchmark (Previous)
+        run: |
+          cd previous
+          go test -run="-" -bench=".*" -count=8 ./... > benchmark.txt
+
+      - name: Run Benchmark (New)
+        run: |
+          cd new
+          go test -run="-" -bench=".*" -count=8 ./... > benchmark.txt
+
+      - name: Run Benchstat
+        run: |
+          benchstat previous/benchmark.txt new/benchmark.txt

.travis.yml

@@ -1,3 +1,7 @@
+arch:
+  - amd64
+  - ppc64le
+  
 language: go
 go:
   - 1.14.x

_fixture/_fixture/README.md

@@ -0,0 +1 @@
+This directory is used for the static middleware test

codecov.yml

@@ -0,0 +1,11 @@
+coverage:
+  status:
+    project:
+      default:
+        threshold: 1%
+    patch:
+      default:
+        threshold: 1%
+
+comment:
+  require_changes: true

context.go

@@ -365,7 +365,7 @@ func (c *context) FormFile(name string) (*multipart.FileHeader, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer f.Close()
+	f.Close()
 	return fh, nil
 }
 

echo.go

@@ -362,10 +362,12 @@ func (e *Echo) DefaultHTTPErrorHandler(err error, c Context) {
 	// Issue #1426
 	code := he.Code
 	message := he.Message
-	if e.Debug {
-		message = err.Error()
-	} else if m, ok := message.(string); ok {
-		message = Map{"message": m}
+	if m, ok := he.Message.(string); ok {
+		if e.Debug {
+			message = Map{"message": m, "error": err.Error()}
+		} else {
+			message = Map{"message": m}
+		}
 	}
 
 	// Send response
@@ -570,7 +572,7 @@ func (e *Echo) Reverse(name string, params ...interface{}) string {
 	for _, r := range e.router.routes {
 		if r.Name == name {
 			for i, l := 0, len(r.Path); i < l; i++ {
-				if r.Path[i] == ':' && n < ln {
+				if (r.Path[i] == ':' || r.Path[i] == '*') && n < ln {
 					for ; i < l && r.Path[i] != '/'; i++ {
 					}
 					uri.WriteString(fmt.Sprintf("%v", params[n]))

echo_test.go

@@ -277,10 +277,12 @@ func TestEchoURL(t *testing.T) {
 	e := New()
 	static := func(Context) error { return nil }
 	getUser := func(Context) error { return nil }
+	getAny := func(Context) error { return nil }
 	getFile := func(Context) error { return nil }
 
 	e.GET("/static/file", static)
 	e.GET("/users/:id", getUser)
+	e.GET("/documents/*", getAny)
 	g := e.Group("/group")
 	g.GET("/users/:uid/files/:fid", getFile)
 
@@ -289,6 +291,9 @@ func TestEchoURL(t *testing.T) {
 	assert.Equal("/static/file", e.URL(static))
 	assert.Equal("/users/:id", e.URL(getUser))
 	assert.Equal("/users/1", e.URL(getUser, "1"))
+	assert.Equal("/users/1", e.URL(getUser, "1"))
+	assert.Equal("/documents/foo.txt", e.URL(getAny, "foo.txt"))
+	assert.Equal("/documents/*", e.URL(getAny))
 	assert.Equal("/group/users/1/files/:fid", e.URL(getFile, "1"))
 	assert.Equal("/group/users/1/files/1", e.URL(getFile, "1", "1"))
 }
@@ -568,6 +573,49 @@ 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)
@@ -609,3 +657,28 @@ func TestEchoShutdown(t *testing.T) {
 	err := <-errCh
 	assert.Equal(t, err.Error(), "http: Server closed")
 }
+
+func TestEchoReverse(t *testing.T) {
+	assert := assert.New(t)
+
+	e := New()
+	dummyHandler := func(Context) error { return nil }
+
+	e.GET("/static", dummyHandler).Name = "/static"
+	e.GET("/static/*", dummyHandler).Name = "/static/*"
+	e.GET("/params/:foo", dummyHandler).Name = "/params/:foo"
+	e.GET("/params/:foo/bar/:qux", dummyHandler).Name = "/params/:foo/bar/:qux"
+	e.GET("/params/:foo/bar/:qux/*", dummyHandler).Name = "/params/:foo/bar/:qux/*"
+
+	assert.Equal("/static", e.Reverse("/static"))
+	assert.Equal("/static", e.Reverse("/static", "missing param"))
+	assert.Equal("/static/*", e.Reverse("/static/*"))
+	assert.Equal("/static/foo.txt", e.Reverse("/static/*", "foo.txt"))
+
+	assert.Equal("/params/:foo", e.Reverse("/params/:foo"))
+	assert.Equal("/params/one", e.Reverse("/params/:foo", "one"))
+	assert.Equal("/params/:foo/bar/:qux", e.Reverse("/params/:foo/bar/:qux"))
+	assert.Equal("/params/one/bar/:qux", e.Reverse("/params/:foo/bar/:qux", "one"))
+	assert.Equal("/params/one/bar/two", e.Reverse("/params/:foo/bar/:qux", "one", "two"))
+	assert.Equal("/params/one/bar/two/three", e.Reverse("/params/:foo/bar/:qux/*", "one", "two", "three"))
+}

middleware/compress.go

@@ -8,6 +8,7 @@ import (
 	"net"
 	"net/http"
 	"strings"
+	"sync"
 
 	"github.com/labstack/echo/v4"
 )
@@ -58,6 +59,8 @@ func GzipWithConfig(config GzipConfig) echo.MiddlewareFunc {
 		config.Level = DefaultGzipConfig.Level
 	}
 
+	pool := gzipPool(config)
+
 	return func(next echo.HandlerFunc) echo.HandlerFunc {
 		return func(c echo.Context) error {
 			if config.Skipper(c) {
@@ -68,11 +71,13 @@ func GzipWithConfig(config GzipConfig) echo.MiddlewareFunc {
 			res.Header().Add(echo.HeaderVary, echo.HeaderAcceptEncoding)
 			if strings.Contains(c.Request().Header.Get(echo.HeaderAcceptEncoding), gzipScheme) {
 				res.Header().Set(echo.HeaderContentEncoding, gzipScheme) // Issue #806
-				rw := res.Writer
-				w, err := gzip.NewWriterLevel(rw, config.Level)
-				if err != nil {
-					return err
+				i := pool.Get()
+				w, ok := i.(*gzip.Writer)
+				if !ok {
+					return echo.NewHTTPError(http.StatusInternalServerError, i.(error).Error())
 				}
+				rw := res.Writer
+				w.Reset(rw)
 				defer func() {
 					if res.Size == 0 {
 						if res.Header().Get(echo.HeaderContentEncoding) == gzipScheme {
@@ -85,6 +90,7 @@ func GzipWithConfig(config GzipConfig) echo.MiddlewareFunc {
 						w.Reset(ioutil.Discard)
 					}
 					w.Close()
+					pool.Put(w)
 				}()
 				grw := &gzipResponseWriter{Writer: w, ResponseWriter: rw}
 				res.Writer = grw
@@ -126,3 +132,15 @@ func (w *gzipResponseWriter) Push(target string, opts *http.PushOptions) error {
 	}
 	return http.ErrNotSupported
 }
+
+func gzipPool(config GzipConfig) sync.Pool {
+	return sync.Pool{
+		New: func() interface{} {
+			w, err := gzip.NewWriterLevel(ioutil.Discard, config.Level)
+			if err != nil {
+				return err
+			}
+			return w
+		},
+	}
+}