From a4ab482b605fb260385212b9bcbcfcdef00a0c27 Mon Sep 17 00:00:00 2001 From: Martti T Date: Fri, 16 Apr 2021 12:38:12 +0300 Subject: [PATCH] Add Go 1.16 to CI and drop 1.12 specific code (#1850) * Correct incorrect years in CHANGELOG.md * CI tests with last 4 versions. Remove 1.12 and below specific code * Rename proxy test --- .github/workflows/echo.yml | 8 +-- CHANGELOG.md | 6 +-- echo_go1.13_test.go | 28 ----------- echo_test.go | 17 +++++++ middleware/csrf.go | 2 +- middleware/csrf_samesite.go | 12 ----- middleware/csrf_samesite_1.12.go | 12 ----- middleware/csrf_samesite_test.go | 33 ------------ middleware/csrf_test.go | 20 ++++++++ middleware/proxy.go | 37 ++++++++++++++ middleware/proxy_1_11.go | 47 ----------------- middleware/proxy_1_11_n.go | 14 ------ middleware/proxy_1_11_test.go | 86 -------------------------------- middleware/proxy_test.go | 73 +++++++++++++++++++++++++++ middleware/timeout.go | 2 - middleware/timeout_test.go | 2 - 16 files changed, 156 insertions(+), 243 deletions(-) delete mode 100644 echo_go1.13_test.go delete mode 100644 middleware/csrf_samesite.go delete mode 100644 middleware/csrf_samesite_1.12.go delete mode 100644 middleware/csrf_samesite_test.go delete mode 100644 middleware/proxy_1_11.go delete mode 100644 middleware/proxy_1_11_n.go delete mode 100644 middleware/proxy_1_11_test.go diff --git a/.github/workflows/echo.yml b/.github/workflows/echo.yml index fb8c5020..ec551756 100644 --- a/.github/workflows/echo.yml +++ b/.github/workflows/echo.yml @@ -25,7 +25,9 @@ jobs: strategy: matrix: os: [ubuntu-latest, macos-latest, windows-latest] - go: [1.12, 1.13, 1.14, 1.15] + # Each major Go release is supported until there are two newer major releases. https://golang.org/doc/devel/release.html#policy + # Echo tests with last four major releases + go: [1.13, 1.14, 1.15, 1.16] name: ${{ matrix.os }} @ Go ${{ matrix.go }} runs-on: ${{ matrix.os }} steps: @@ -59,7 +61,7 @@ jobs: go test -race --coverprofile=coverage.coverprofile --covermode=atomic ./... - name: Upload coverage to Codecov - if: success() && matrix.go == 1.15 && matrix.os == 'ubuntu-latest' + if: success() && matrix.go == 1.16 && matrix.os == 'ubuntu-latest' uses: codecov/codecov-action@v1 with: token: @@ -69,7 +71,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - go: [1.15] + go: [1.16] name: Benchmark comparison ${{ matrix.os }} @ Go ${{ matrix.go }} runs-on: ${{ matrix.os }} steps: diff --git a/CHANGELOG.md b/CHANGELOG.md index c1be77a9..a3117b80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## v4.2.2 - 2020-04-07 +## v4.2.2 - 2021-04-07 **Fixes** @@ -10,7 +10,7 @@ * Fix panic in redirect middleware on short host name (#1813) * Fix timeout middleware docs (#1836) -## v4.2.1 - 2020-03-08 +## v4.2.1 - 2021-03-08 **Important notes** @@ -32,7 +32,7 @@ A performance regression has been fixed, even bringing better performance than b This release was made possible by our **contributors**: aldas, clwluvw, lammel, Le0tk0k, maciej-jezierski, rkilingr, stffabi, withshubh -## v4.2.0 - 2020-02-11 +## v4.2.0 - 2021-02-11 **Important notes** diff --git a/echo_go1.13_test.go b/echo_go1.13_test.go deleted file mode 100644 index 3c488bc6..00000000 --- a/echo_go1.13_test.go +++ /dev/null @@ -1,28 +0,0 @@ -// +build go1.13 - -package echo - -import ( - "errors" - "net/http" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestHTTPError_Unwrap(t *testing.T) { - t.Run("non-internal", func(t *testing.T) { - err := NewHTTPError(http.StatusBadRequest, map[string]interface{}{ - "code": 12, - }) - - assert.Nil(t, errors.Unwrap(err)) - }) - t.Run("internal", func(t *testing.T) { - err := NewHTTPError(http.StatusBadRequest, map[string]interface{}{ - "code": 12, - }) - err.SetInternal(errors.New("internal error")) - assert.Equal(t, "internal error", errors.Unwrap(err).Error()) - }) -} diff --git a/echo_test.go b/echo_test.go index 58ecea74..ba498831 100644 --- a/echo_test.go +++ b/echo_test.go @@ -957,6 +957,23 @@ func TestHTTPError(t *testing.T) { }) } +func TestHTTPError_Unwrap(t *testing.T) { + t.Run("non-internal", func(t *testing.T) { + err := NewHTTPError(http.StatusBadRequest, map[string]interface{}{ + "code": 12, + }) + + assert.Nil(t, errors.Unwrap(err)) + }) + t.Run("internal", func(t *testing.T) { + err := NewHTTPError(http.StatusBadRequest, map[string]interface{}{ + "code": 12, + }) + err.SetInternal(errors.New("internal error")) + assert.Equal(t, "internal error", errors.Unwrap(err).Error()) + }) +} + func TestDefaultHTTPErrorHandler(t *testing.T) { e := New() e.Debug = true diff --git a/middleware/csrf.go b/middleware/csrf.go index 60f809a0..7804997d 100644 --- a/middleware/csrf.go +++ b/middleware/csrf.go @@ -110,7 +110,7 @@ func CSRFWithConfig(config CSRFConfig) echo.MiddlewareFunc { if config.CookieMaxAge == 0 { config.CookieMaxAge = DefaultCSRFConfig.CookieMaxAge } - if config.CookieSameSite == SameSiteNoneMode { + if config.CookieSameSite == http.SameSiteNoneMode { config.CookieSecure = true } diff --git a/middleware/csrf_samesite.go b/middleware/csrf_samesite.go deleted file mode 100644 index 9a27dc43..00000000 --- a/middleware/csrf_samesite.go +++ /dev/null @@ -1,12 +0,0 @@ -// +build go1.13 - -package middleware - -import ( - "net/http" -) - -const ( - // SameSiteNoneMode required to be redefined for Go 1.12 support (see #1524) - SameSiteNoneMode http.SameSite = http.SameSiteNoneMode -) diff --git a/middleware/csrf_samesite_1.12.go b/middleware/csrf_samesite_1.12.go deleted file mode 100644 index 22076dd6..00000000 --- a/middleware/csrf_samesite_1.12.go +++ /dev/null @@ -1,12 +0,0 @@ -// +build !go1.13 - -package middleware - -import ( - "net/http" -) - -const ( - // SameSiteNoneMode required to be redefined for Go 1.12 support (see #1524) - SameSiteNoneMode http.SameSite = 4 -) diff --git a/middleware/csrf_samesite_test.go b/middleware/csrf_samesite_test.go deleted file mode 100644 index 26c5bc45..00000000 --- a/middleware/csrf_samesite_test.go +++ /dev/null @@ -1,33 +0,0 @@ -// +build go1.13 - -package middleware - -import ( - "net/http" - "net/http/httptest" - "testing" - - "github.com/labstack/echo/v4" - "github.com/stretchr/testify/assert" -) - -// Test for SameSiteModeNone moved to separate file for Go 1.12 support -func TestCSRFWithSameSiteModeNone(t *testing.T) { - e := echo.New() - req := httptest.NewRequest(http.MethodGet, "/", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) - - csrf := CSRFWithConfig(CSRFConfig{ - CookieSameSite: SameSiteNoneMode, - }) - - h := csrf(func(c echo.Context) error { - return c.String(http.StatusOK, "test") - }) - - r := h(c) - assert.NoError(t, r) - assert.Regexp(t, "SameSite=None", rec.Header()["Set-Cookie"]) - assert.Regexp(t, "Secure", rec.Header()["Set-Cookie"]) -} diff --git a/middleware/csrf_test.go b/middleware/csrf_test.go index ebe4dbcd..af1d2639 100644 --- a/middleware/csrf_test.go +++ b/middleware/csrf_test.go @@ -138,3 +138,23 @@ func TestCSRFWithSameSiteDefaultMode(t *testing.T) { fmt.Println(rec.Header()["Set-Cookie"]) assert.NotRegexp(t, "SameSite=", rec.Header()["Set-Cookie"]) } + +func TestCSRFWithSameSiteModeNone(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + csrf := CSRFWithConfig(CSRFConfig{ + CookieSameSite: http.SameSiteNoneMode, + }) + + h := csrf(func(c echo.Context) error { + return c.String(http.StatusOK, "test") + }) + + r := h(c) + assert.NoError(t, r) + assert.Regexp(t, "SameSite=None", rec.Header()["Set-Cookie"]) + assert.Regexp(t, "Secure", rec.Header()["Set-Cookie"]) +} diff --git a/middleware/proxy.go b/middleware/proxy.go index 6f01f3a7..6cfd6731 100644 --- a/middleware/proxy.go +++ b/middleware/proxy.go @@ -1,13 +1,16 @@ package middleware import ( + "context" "fmt" "io" "math/rand" "net" "net/http" + "net/http/httputil" "net/url" "regexp" + "strings" "sync" "sync/atomic" "time" @@ -264,3 +267,37 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc { } } } + +// StatusCodeContextCanceled is a custom HTTP status code for situations +// where a client unexpectedly closed the connection to the server. +// As there is no standard error code for "client closed connection", but +// various well-known HTTP clients and server implement this HTTP code we use +// 499 too instead of the more problematic 5xx, which does not allow to detect this situation +const StatusCodeContextCanceled = 499 + +func proxyHTTP(tgt *ProxyTarget, c echo.Context, config ProxyConfig) http.Handler { + proxy := httputil.NewSingleHostReverseProxy(tgt.URL) + proxy.ErrorHandler = func(resp http.ResponseWriter, req *http.Request, err error) { + desc := tgt.URL.String() + if tgt.Name != "" { + desc = fmt.Sprintf("%s(%s)", tgt.Name, tgt.URL.String()) + } + // If the client canceled the request (usually by closing the connection), we can report a + // client error (4xx) instead of a server error (5xx) to correctly identify the situation. + // The Go standard library (at of late 2020) wraps the exported, standard + // context.Canceled error with unexported garbage value requiring a substring check, see + // https://github.com/golang/go/blob/6965b01ea248cabb70c3749fd218b36089a21efb/src/net/net.go#L416-L430 + if err == context.Canceled || strings.Contains(err.Error(), "operation was canceled") { + httpError := echo.NewHTTPError(StatusCodeContextCanceled, fmt.Sprintf("client closed connection: %v", err)) + httpError.Internal = err + c.Set("_error", httpError) + } else { + httpError := echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("remote %s unreachable, could not forward: %v", desc, err)) + httpError.Internal = err + c.Set("_error", httpError) + } + } + proxy.Transport = config.Transport + proxy.ModifyResponse = config.ModifyResponse + return proxy +} diff --git a/middleware/proxy_1_11.go b/middleware/proxy_1_11.go deleted file mode 100644 index 17d142d8..00000000 --- a/middleware/proxy_1_11.go +++ /dev/null @@ -1,47 +0,0 @@ -// +build go1.11 - -package middleware - -import ( - "context" - "fmt" - "net/http" - "net/http/httputil" - "strings" - - "github.com/labstack/echo/v4" -) - -// StatusCodeContextCanceled is a custom HTTP status code for situations -// where a client unexpectedly closed the connection to the server. -// As there is no standard error code for "client closed connection", but -// various well-known HTTP clients and server implement this HTTP code we use -// 499 too instead of the more problematic 5xx, which does not allow to detect this situation -const StatusCodeContextCanceled = 499 - -func proxyHTTP(tgt *ProxyTarget, c echo.Context, config ProxyConfig) http.Handler { - proxy := httputil.NewSingleHostReverseProxy(tgt.URL) - proxy.ErrorHandler = func(resp http.ResponseWriter, req *http.Request, err error) { - desc := tgt.URL.String() - if tgt.Name != "" { - desc = fmt.Sprintf("%s(%s)", tgt.Name, tgt.URL.String()) - } - // If the client canceled the request (usually by closing the connection), we can report a - // client error (4xx) instead of a server error (5xx) to correctly identify the situation. - // The Go standard library (at of late 2020) wraps the exported, standard - // context.Canceled error with unexported garbage value requiring a substring check, see - // https://github.com/golang/go/blob/6965b01ea248cabb70c3749fd218b36089a21efb/src/net/net.go#L416-L430 - if err == context.Canceled || strings.Contains(err.Error(), "operation was canceled") { - httpError := echo.NewHTTPError(StatusCodeContextCanceled, fmt.Sprintf("client closed connection: %v", err)) - httpError.Internal = err - c.Set("_error", httpError) - } else { - httpError := echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("remote %s unreachable, could not forward: %v", desc, err)) - httpError.Internal = err - c.Set("_error", httpError) - } - } - proxy.Transport = config.Transport - proxy.ModifyResponse = config.ModifyResponse - return proxy -} diff --git a/middleware/proxy_1_11_n.go b/middleware/proxy_1_11_n.go deleted file mode 100644 index 9a78929f..00000000 --- a/middleware/proxy_1_11_n.go +++ /dev/null @@ -1,14 +0,0 @@ -// +build !go1.11 - -package middleware - -import ( - "net/http" - "net/http/httputil" - - "github.com/labstack/echo/v4" -) - -func proxyHTTP(t *ProxyTarget, c echo.Context, config ProxyConfig) http.Handler { - return httputil.NewSingleHostReverseProxy(t.URL) -} diff --git a/middleware/proxy_1_11_test.go b/middleware/proxy_1_11_test.go deleted file mode 100644 index c3541d5e..00000000 --- a/middleware/proxy_1_11_test.go +++ /dev/null @@ -1,86 +0,0 @@ -// +build go1.11 - -package middleware - -import ( - "context" - "net/http" - "net/http/httptest" - "net/url" - "sync" - "testing" - "time" - - "github.com/labstack/echo/v4" - "github.com/stretchr/testify/assert" -) - -func TestProxy_1_11(t *testing.T) { - // Setup - url1, _ := url.Parse("http://127.0.0.1:27121") - url2, _ := url.Parse("http://127.0.0.1:27122") - - targets := []*ProxyTarget{ - { - Name: "target 1", - URL: url1, - }, - { - Name: "target 2", - URL: url2, - }, - } - rb := NewRandomBalancer(nil) - // must add targets: - for _, target := range targets { - assert.True(t, rb.AddTarget(target)) - } - - // must ignore duplicates: - for _, target := range targets { - assert.False(t, rb.AddTarget(target)) - } - - // Random - e := echo.New() - e.Use(Proxy(rb)) - req := httptest.NewRequest(http.MethodGet, "/", nil) - rec := httptest.NewRecorder() - - // Remote unreachable - rec = httptest.NewRecorder() - req.URL.Path = "/api/users" - e.ServeHTTP(rec, req) - assert.Equal(t, "/api/users", req.URL.Path) - assert.Equal(t, http.StatusBadGateway, rec.Code) -} - -func TestClientCancelConnectionResultsHTTPCode499(t *testing.T) { - var timeoutStop sync.WaitGroup - timeoutStop.Add(1) - HTTPTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - timeoutStop.Wait() // wait until we have canceled the request - w.WriteHeader(http.StatusOK) - })) - defer HTTPTarget.Close() - targetURL, _ := url.Parse(HTTPTarget.URL) - target := &ProxyTarget{ - Name: "target", - URL: targetURL, - } - rb := NewRandomBalancer(nil) - assert.True(t, rb.AddTarget(target)) - e := echo.New() - e.Use(Proxy(rb)) - rec := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodGet, "/", nil) - ctx, cancel := context.WithCancel(req.Context()) - req = req.WithContext(ctx) - go func() { - time.Sleep(10 * time.Millisecond) - cancel() - }() - e.ServeHTTP(rec, req) - timeoutStop.Done() - assert.Equal(t, 499, rec.Code) -} diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go index 93daf735..7939fc5c 100644 --- a/middleware/proxy_test.go +++ b/middleware/proxy_test.go @@ -2,6 +2,7 @@ package middleware import ( "bytes" + "context" "fmt" "io/ioutil" "net" @@ -9,7 +10,9 @@ import ( "net/http/httptest" "net/url" "regexp" + "sync" "testing" + "time" "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" @@ -302,3 +305,73 @@ func TestProxyRewriteRegex(t *testing.T) { }) } } + +func TestProxyError(t *testing.T) { + // Setup + url1, _ := url.Parse("http://127.0.0.1:27121") + url2, _ := url.Parse("http://127.0.0.1:27122") + + targets := []*ProxyTarget{ + { + Name: "target 1", + URL: url1, + }, + { + Name: "target 2", + URL: url2, + }, + } + rb := NewRandomBalancer(nil) + // must add targets: + for _, target := range targets { + assert.True(t, rb.AddTarget(target)) + } + + // must ignore duplicates: + for _, target := range targets { + assert.False(t, rb.AddTarget(target)) + } + + // Random + e := echo.New() + e.Use(Proxy(rb)) + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + + // Remote unreachable + rec = httptest.NewRecorder() + req.URL.Path = "/api/users" + e.ServeHTTP(rec, req) + assert.Equal(t, "/api/users", req.URL.Path) + assert.Equal(t, http.StatusBadGateway, rec.Code) +} + +func TestClientCancelConnectionResultsHTTPCode499(t *testing.T) { + var timeoutStop sync.WaitGroup + timeoutStop.Add(1) + HTTPTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + timeoutStop.Wait() // wait until we have canceled the request + w.WriteHeader(http.StatusOK) + })) + defer HTTPTarget.Close() + targetURL, _ := url.Parse(HTTPTarget.URL) + target := &ProxyTarget{ + Name: "target", + URL: targetURL, + } + rb := NewRandomBalancer(nil) + assert.True(t, rb.AddTarget(target)) + e := echo.New() + e.Use(Proxy(rb)) + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx, cancel := context.WithCancel(req.Context()) + req = req.WithContext(ctx) + go func() { + time.Sleep(10 * time.Millisecond) + cancel() + }() + e.ServeHTTP(rec, req) + timeoutStop.Done() + assert.Equal(t, 499, rec.Code) +} diff --git a/middleware/timeout.go b/middleware/timeout.go index 5d23ff45..d56e463b 100644 --- a/middleware/timeout.go +++ b/middleware/timeout.go @@ -1,5 +1,3 @@ -// +build go1.13 - package middleware import ( diff --git a/middleware/timeout_test.go b/middleware/timeout_test.go index 8f8fa304..f9f1826b 100644 --- a/middleware/timeout_test.go +++ b/middleware/timeout_test.go @@ -1,5 +1,3 @@ -// +build go1.13 - package middleware import (