From e99c57b9f0b5f9b822c3747aec4181055ad16b8d Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Mon, 1 Mar 2021 02:56:44 +0330 Subject: [PATCH 1/2] Store original error as internal field for proxy errors Signed-off-by: Seena Fallah --- middleware/proxy_1_11.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/middleware/proxy_1_11.go b/middleware/proxy_1_11.go index a43927817..019060f7e 100644 --- a/middleware/proxy_1_11.go +++ b/middleware/proxy_1_11.go @@ -17,7 +17,9 @@ func proxyHTTP(tgt *ProxyTarget, c echo.Context, config ProxyConfig) http.Handle if tgt.Name != "" { desc = fmt.Sprintf("%s(%s)", tgt.Name, tgt.URL.String()) } - c.Set("_error", echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("remote %s unreachable, could not forward: %v", desc, err))) + 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 From 01fbe2a035d3586925c5d82f5beac694a919ad15 Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Tue, 23 Feb 2021 17:37:12 +0330 Subject: [PATCH 2/2] Avoid context canceled errors Return 499 Client Closed Request when the client has closed the request before the server could send a response Signed-off-by: Seena Fallah --- middleware/proxy_1_11.go | 26 +++++++++++++++++++++++--- middleware/proxy_1_11_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/middleware/proxy_1_11.go b/middleware/proxy_1_11.go index 019060f7e..17d142d8d 100644 --- a/middleware/proxy_1_11.go +++ b/middleware/proxy_1_11.go @@ -3,13 +3,22 @@ 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) { @@ -17,9 +26,20 @@ func proxyHTTP(tgt *ProxyTarget, c echo.Context, config ProxyConfig) http.Handle if tgt.Name != "" { desc = fmt.Sprintf("%s(%s)", tgt.Name, tgt.URL.String()) } - httpError := echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("remote %s unreachable, could not forward: %v", desc, err)) - httpError.Internal = err - c.Set("_error", httpError) + // 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 diff --git a/middleware/proxy_1_11_test.go b/middleware/proxy_1_11_test.go index 26feaabaa..c3541d5e8 100644 --- a/middleware/proxy_1_11_test.go +++ b/middleware/proxy_1_11_test.go @@ -3,10 +3,13 @@ package middleware import ( + "context" "net/http" "net/http/httptest" "net/url" + "sync" "testing" + "time" "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" @@ -51,3 +54,33 @@ func TestProxy_1_11(t *testing.T) { 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) +}