Repository: labstack/echo
Pull request #1743 (2020-12-30 11:03:24)

Timeout middleware implementation
There are occasions when you want to add a timeout to your handlers, so they get finished within a specified period of time.

codecov[bot]: # [Codecov](https://codecov.io/gh/labstack/echo/pull/1743?src=pr&el=h1) Report
> Merging [#1743](https://codecov.io/gh/labstack/echo/pull/1743?src=pr&el=desc) (360c2f7) into [master](https://codecov.io/gh/labstack/echo/commit/6119aecb16b39eae121ef620d665ca0ff00b21ba?el=desc) (6119aec) will **increase** coverage by `0.07%`.
> The diff coverage is `94.73%`.

[![Impacted file tree graph](https://codecov.io/gh/labstack/echo/pull/1743/graphs/tree.svg?width=650&height=150&src=pr&token=vqnT76l79i)](https://codecov.io/gh/labstack/echo/pull/1743?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1743      +/-   ##
==========================================
+ Coverage   86.66%   86.74%   +0.07%     
==========================================
  Files          29       30       +1     
  Lines        2063     2082      +19     
==========================================
+ Hits         1788     1806      +18     
- Misses        175      176       +1     
  Partials      100      100              
```


| [Impacted Files](https://codecov.io/gh/labstack/echo/pull/1743?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [middleware/timeout.go](https://codecov.io/gh/labstack/echo/pull/1743/diff?src=pr&el=tree#diff-bWlkZGxld2FyZS90aW1lb3V0Lmdv) | `94.73% <94.73%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/labstack/echo/pull/1743?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/labstack/echo/pull/1743?src=pr&el=footer). Last update [6119aec...360c2f7](https://codecov.io/gh/labstack/echo/pull/1743?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

ilijamt: Currently Go 1.12 is skipped due to missing Request.Clone, is this something that should really be added or can we live without it? 

I was considering adding a separate file for go 1.12 using Request.WithContext but that only creates a shallow clone, and I don't know what are the implications of it with other usecases.

lammel: We just need to clarify if a timeout middleware should be part of echo core. As we are also preparing a rate limiter in #1724  I guess this may fit into core too.

Patch:
diff --git a/bind.go b/bind.go
index acd2bed..16c3b7a 100644
--- a/bind.go
+++ b/bind.go
@@ -116,12 +116,13 @@ func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
 	return b.BindBody(c, i)
 }
 
-func (b *DefaultBinder) bindData(ptr interface{}, data map[string][]string, tag string) error {
-	if ptr == nil || len(data) == 0 {
+// bindData will bind data ONLY fields in destination struct that have EXPLICIT tag
+func (b *DefaultBinder) bindData(destination interface{}, data map[string][]string, tag string) error {
+	if destination == nil || len(data) == 0 {
 		return nil
 	}
-	typ := reflect.TypeOf(ptr).Elem()
-	val := reflect.ValueOf(ptr).Elem()
+	typ := reflect.TypeOf(destination).Elem()
+	val := reflect.ValueOf(destination).Elem()
 
 	// Map
 	if typ.Kind() == reflect.Map {
@@ -146,14 +147,15 @@ func (b *DefaultBinder) bindData(ptr interface{}, data map[string][]string, tag
 		inputFieldName := typeField.Tag.Get(tag)
 
 		if inputFieldName == "" {
-			inputFieldName = typeField.Name
-			// If tag is nil, we inspect if the field is a struct.
+			// If tag is nil, we inspect if the field is a not BindUnmarshaler struct and try to bind data into it (might contains fields with tags).
+			// structs that implement BindUnmarshaler are binded only when they have explicit tag
 			if _, ok := structField.Addr().Interface().(BindUnmarshaler); !ok && structFieldKind == reflect.Struct {
 				if err := b.bindData(structField.Addr().Interface(), data, tag); err != nil {
 					return err
 				}
-				continue
 			}
+			// does not have explicit tag and is not an ordinary struct - so move to next field
+			continue
 		}
 
 		inputValue, exists := data[inputFieldName]
diff --git a/middleware/timeout.go b/middleware/timeout.go
new file mode 100644
index 0000000..d146541
--- /dev/null
+++ b/middleware/timeout.go
@@ -0,0 +1,81 @@
+// +build go1.13
+
+package middleware
+
+import (
+	"context"
+	"github.com/labstack/echo/v4"
+	"time"
+)
+
+type (
+	// TimeoutConfig defines the config for Timeout middleware.
+	TimeoutConfig struct {
+		// Skipper defines a function to skip middleware.
+		Skipper Skipper
+		// ErrorHandler defines a function which is executed for a timeout
+		// It can be used to define a custom timeout error
+		ErrorHandler TimeoutErrorHandlerWithContext
+		// Timeout configures a timeout for the middleware, defaults to 0 for no timeout
+		Timeout time.Duration
+	}
+
+	// TimeoutErrorHandlerWithContext is an error handler that is used with the timeout middleware so we can
+	// handle the error as we see fit
+	TimeoutErrorHandlerWithContext func(error, echo.Context) error
+)
+
+var (
+	// DefaultTimeoutConfig is the default Timeout middleware config.
+	DefaultTimeoutConfig = TimeoutConfig{
+		Skipper:      DefaultSkipper,
+		Timeout:      0,
+		ErrorHandler: nil,
+	}
+)
+
+// Timeout returns a middleware which recovers from panics anywhere in the chain
+// and handles the control to the centralized HTTPErrorHandler.
+func Timeout() echo.MiddlewareFunc {
+	return TimeoutWithConfig(DefaultTimeoutConfig)
+}
+
+// TimeoutWithConfig returns a Timeout middleware with config.
+// See: `Timeout()`.
+func TimeoutWithConfig(config TimeoutConfig) echo.MiddlewareFunc {
+	// Defaults
+	if config.Skipper == nil {
+		config.Skipper = DefaultTimeoutConfig.Skipper
+	}
+
+	return func(next echo.HandlerFunc) echo.HandlerFunc {
+		return func(c echo.Context) error {
+			if config.Skipper(c) || config.Timeout == 0 {
+				return next(c)
+			}
+
+			ctx, cancel := context.WithTimeout(c.Request().Context(), config.Timeout)
+			defer cancel()
+
+			// this does a deep clone of the context, wondering if there is a better way to do this?
+			c.SetRequest(c.Request().Clone(ctx))
+
+			done := make(chan error, 1)
+			go func() {
+				// This goroutine will keep running even if this middleware times out and
+				// will be stopped when ctx.Done() is called down the next(c) call chain
+				done <- next(c)
+			}()
+
+			select {
+			case <-ctx.Done():
+				if config.ErrorHandler != nil {
+					return config.ErrorHandler(ctx.Err(), c)
+				}
+				return ctx.Err()
+			case err := <-done:
+				return err
+			}
+		}
+	}
+}


Test patch:
diff --git a/bind_test.go b/bind_test.go
index 345fbdf..e8868b3 100644
--- a/bind_test.go
+++ b/bind_test.go
@@ -160,6 +160,31 @@ var values = map[string][]string{
 	"ST":      {"bar"},
 }
 
+func TestToMultipleFields(t *testing.T) {
+	e := New()
+	req := httptest.NewRequest(http.MethodGet, "/?id=1&ID=2", nil)
+	rec := httptest.NewRecorder()
+	c := e.NewContext(req, rec)
+
+	type Root struct {
+		ID     int64 `query:"id"`
+		Child2 struct {
+			ID int64
+		}
+		Child1 struct {
+			ID int64 `query:"id"`
+		}
+	}
+
+	u := new(Root)
+	err := c.Bind(u)
+	if assert.NoError(t, err) {
+		assert.Equal(t, int64(1), u.ID)        // perfectly reasonable
+		assert.Equal(t, int64(1), u.Child1.ID) // untagged struct containing tagged field gets filled (by tag)
+		assert.Equal(t, int64(0), u.Child2.ID) // untagged struct containing untagged field should not be bind
+	}
+}
+
 func TestBindJSON(t *testing.T) {
 	assert := assert.New(t)
 	testBindOkay(assert, strings.NewReader(userJSON), MIMEApplicationJSON)
@@ -238,10 +263,13 @@ func TestBindUnmarshalParam(t *testing.T) {
 	rec := httptest.NewRecorder()
 	c := e.NewContext(req, rec)
 	result := struct {
-		T  Timestamp   `query:"ts"`
-		TA []Timestamp `query:"ta"`
-		SA StringArray `query:"sa"`
-		ST Struct
+		T         Timestamp   `query:"ts"`
+		TA        []Timestamp `query:"ta"`
+		SA        StringArray `query:"sa"`
+		ST        Struct
+		StWithTag struct {
+			Foo string `query:"st"`
+		}
 	}{}
 	err := c.Bind(&result)
 	ts := Timestamp(time.Date(2016, 12, 6, 19, 9, 5, 0, time.UTC))
@@ -252,7 +280,8 @@ func TestBindUnmarshalParam(t *testing.T) {
 		assert.Equal(ts, result.T)
 		assert.Equal(StringArray([]string{"one", "two", "three"}), result.SA)
 		assert.Equal([]Timestamp{ts, ts}, result.TA)
-		assert.Equal(Struct{"baz"}, result.ST)
+		assert.Equal(Struct{""}, result.ST)       // child struct does not have a field with matching tag
+		assert.Equal("baz", result.StWithTag.Foo) // child struct has field with matching tag
 	}
 }
 
@@ -274,7 +303,7 @@ func TestBindUnmarshalText(t *testing.T) {
 		assert.Equal(t, ts, result.T)
 		assert.Equal(t, StringArray([]string{"one", "two", "three"}), result.SA)
 		assert.Equal(t, []time.Time{ts, ts}, result.TA)
-		assert.Equal(t, Struct{"baz"}, result.ST)
+		assert.Equal(t, Struct{""}, result.ST) // field in child struct does not have tag
 	}
 }
 
@@ -323,11 +352,27 @@ func TestBindUnsupportedMediaType(t *testing.T) {
 }
 
 func TestBindbindData(t *testing.T) {
-	assert := assert.New(t)
+	a := assert.New(t)
 	ts := new(bindTestStruct)
 	b := new(DefaultBinder)
-	b.bindData(ts, values, "form")
-	assertBindTestStruct(assert, ts)
+	err := b.bindData(ts, values, "form")
+	a.NoError(err)
+
+	a.Equal(0, ts.I)
+	a.Equal(int8(0), ts.I8)
+	a.Equal(int16(0), ts.I16)
+	a.Equal(int32(0), ts.I32)
+	a.Equal(int64(0), ts.I64)
+	a.Equal(uint(0), ts.UI)
+	a.Equal(uint8(0), ts.UI8)
+	a.Equal(uint16(0), ts.UI16)
+	a.Equal(uint32(0), ts.UI32)
+	a.Equal(uint64(0), ts.UI64)
+	a.Equal(false, ts.B)
+	a.Equal(float32(0), ts.F32)
+	a.Equal(float64(0), ts.F64)
+	a.Equal("", ts.S)
+	a.Equal("", ts.cantSet)
 }
 
 func TestBindParam(t *testing.T) {
@@ -470,20 +515,6 @@ func TestBindSetFields(t *testing.T) {
 	}
 }
 
-func BenchmarkBindbindData(b *testing.B) {
-	b.ReportAllocs()
-	assert := assert.New(b)
-	ts := new(bindTestStruct)
-	binder := new(DefaultBinder)
-	var err error
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
-		err = binder.bindData(ts, values, "form")
-	}
-	assert.NoError(err)
-	assertBindTestStruct(assert, ts)
-}
-
 func BenchmarkBindbindDataWithTags(b *testing.B) {
 	b.ReportAllocs()
 	assert := assert.New(b)
@@ -560,8 +591,9 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
 	// these tests are to document this behaviour and detect further possible regressions when bind implementation is changed
 
 	type Opts struct {
-		ID   int    `json:"id"`
-		Node string `json:"node"`
+		ID   int    `json:"id" form:"id" query:"id"`
+		Node string `json:"node" form:"node" query:"node" param:"node"`
+		Lang string
 	}
 
 	var testCases = []struct {
@@ -727,8 +759,8 @@ func TestDefaultBinder_BindBody(t *testing.T) {
 	// these tests are to document this behaviour and detect further possible regressions when bind implementation is changed
 
 	type Node struct {
-		ID   int    `json:"id" xml:"id"`
-		Node string `json:"node" xml:"node"`
+		ID   int    `json:"id" xml:"id" form:"id" query:"id"`
+		Node string `json:"node" xml:"node" form:"node" query:"node" param:"node"`
 	}
 	type Nodes struct {
 		Nodes []Node `xml:"node" form:"node"`
@@ -824,7 +856,7 @@ func TestDefaultBinder_BindBody(t *testing.T) {
 			expectError:      "code=400, message=Syntax error: line=1, error=XML syntax error on line 1: unexpected EOF, internal=XML syntax error on line 1: unexpected EOF",
 		},
 		{
-			name:             "ok, FORM POST bind to struct with: path + query + empty body",
+			name:             "ok, FORM POST bind to struct with: path + query + body",
 			givenURL:         "/api/real_node/endpoint?node=xxx",
 			givenMethod:      http.MethodPost,
 			givenContentType: MIMEApplicationForm,
diff --git a/middleware/timeout_test.go b/middleware/timeout_test.go
new file mode 100644
index 0000000..c0e9459
--- /dev/null
+++ b/middleware/timeout_test.go
@@ -0,0 +1,177 @@
+// +build go1.13
+
+package middleware
+
+import (
+	"context"
+	"errors"
+	"github.com/labstack/echo/v4"
+	"github.com/stretchr/testify/assert"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"reflect"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestTimeoutSkipper(t *testing.T) {
+	t.Parallel()
+	m := TimeoutWithConfig(TimeoutConfig{
+		Skipper: func(context echo.Context) bool {
+			return true
+		},
+	})
+
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	rec := httptest.NewRecorder()
+
+	e := echo.New()
+	c := e.NewContext(req, rec)
+
+	err := m(func(c echo.Context) error {
+		assert.NotEqual(t, "*context.timerCtx", reflect.TypeOf(c.Request().Context()).String())
+		return nil
+	})(c)
+
+	assert.NoError(t, err)
+}
+
+func TestTimeoutWithTimeout0(t *testing.T) {
+	t.Parallel()
+	m := TimeoutWithConfig(TimeoutConfig{
+		Timeout: 0,
+	})
+
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	rec := httptest.NewRecorder()
+
+	e := echo.New()
+	c := e.NewContext(req, rec)
+
+	err := m(func(c echo.Context) error {
+		assert.NotEqual(t, "*context.timerCtx", reflect.TypeOf(c.Request().Context()).String())
+		return nil
+	})(c)
+
+	assert.NoError(t, err)
+}
+
+func TestTimeoutIsCancelable(t *testing.T) {
+	t.Parallel()
+	m := TimeoutWithConfig(TimeoutConfig{
+		Timeout: time.Minute,
+	})
+
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	rec := httptest.NewRecorder()
+
+	e := echo.New()
+	c := e.NewContext(req, rec)
+
+	err := m(func(c echo.Context) error {
+		assert.EqualValues(t, "*context.timerCtx", reflect.TypeOf(c.Request().Context()).String())
+		return nil
+	})(c)
+
+	assert.NoError(t, err)
+}
+
+func TestTimeoutErrorOutInHandler(t *testing.T) {
+	t.Parallel()
+	m := Timeout()
+
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	rec := httptest.NewRecorder()
+
+	e := echo.New()
+	c := e.NewContext(req, rec)
+
+	err := m(func(c echo.Context) error {
+		return errors.New("err")
+	})(c)
+
+	assert.Error(t, err)
+}
+
+func TestTimeoutTimesOutAfterPredefinedTimeoutWithErrorHandler(t *testing.T) {
+	t.Parallel()
+	m := TimeoutWithConfig(TimeoutConfig{
+		Timeout: time.Second,
+		ErrorHandler: func(err error, e echo.Context) error {
+			assert.EqualError(t, err, context.DeadlineExceeded.Error())
+			return errors.New("err")
+		},
+	})
+
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	rec := httptest.NewRecorder()
+
+	e := echo.New()
+	c := e.NewContext(req, rec)
+
+	err := m(func(c echo.Context) error {
+		time.Sleep(time.Minute)
+		return nil
+	})(c)
+
+	assert.EqualError(t, err, errors.New("err").Error())
+}
+
+func TestTimeoutTimesOutAfterPredefinedTimeout(t *testing.T) {
+	t.Parallel()
+	m := TimeoutWithConfig(TimeoutConfig{
+		Timeout: time.Second,
+	})
+
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	rec := httptest.NewRecorder()
+
+	e := echo.New()
+	c := e.NewContext(req, rec)
+
+	err := m(func(c echo.Context) error {
+		time.Sleep(time.Minute)
+		return nil
+	})(c)
+
+	assert.EqualError(t, err, context.DeadlineExceeded.Error())
+}
+
+func TestTimeoutTestRequestClone(t *testing.T) {
+	t.Parallel()
+	req := httptest.NewRequest(http.MethodPost, "/uri?query=value", strings.NewReader(url.Values{"form": {"value"}}.Encode()))
+	req.AddCookie(&http.Cookie{Name: "cookie", Value: "value"})
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	rec := httptest.NewRecorder()
+
+	m := TimeoutWithConfig(TimeoutConfig{
+		// Timeout has to be defined or the whole flow for timeout middleware will be skipped
+		Timeout: time.Second,
+	})
+
+	e := echo.New()
+	c := e.NewContext(req, rec)
+
+	err := m(func(c echo.Context) error {
+		// Cookie test
+		cookie, err := c.Request().Cookie("cookie")
+		if assert.NoError(t, err) {
+			assert.EqualValues(t, "cookie", cookie.Name)
+			assert.EqualValues(t, "value", cookie.Value)
+		}
+
+		// Form values
+		if assert.NoError(t, c.Request().ParseForm()) {
+			assert.EqualValues(t, "value", c.Request().FormValue("form"))
+		}
+
+		// Query string
+		assert.EqualValues(t, "value", c.Request().URL.Query()["query"][0])
+		return nil
+	})(c)
+
+	assert.NoError(t, err)
+
+}