From d27ec4068edd6280ac46ce8aadce10ee55881c0a Mon Sep 17 00:00:00 2001
From: Mohammad Alian <ts-mohammad.alian@rakuten.com>
Date: Thu, 12 Aug 2021 19:56:04 +0900
Subject: [PATCH] return first if response is already committed in
 DefaultHTTPErrorHandler

---
 echo.go      | 21 ++++++++++++---------
 echo_test.go | 17 +++++++++++++++++
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/echo.go b/echo.go
index 246a62256..a28fa0c1a 100644
--- a/echo.go
+++ b/echo.go
@@ -358,6 +358,11 @@ func (e *Echo) Routers() map[string]*Router {
 // DefaultHTTPErrorHandler is the default HTTP error handler. It sends a JSON response
 // with status code.
 func (e *Echo) DefaultHTTPErrorHandler(err error, c Context) {
+
+	if c.Response().Committed {
+		return
+	}
+
 	he, ok := err.(*HTTPError)
 	if ok {
 		if he.Internal != nil {
@@ -384,15 +389,13 @@ func (e *Echo) DefaultHTTPErrorHandler(err error, c Context) {
 	}
 
 	// Send response
-	if !c.Response().Committed {
-		if c.Request().Method == http.MethodHead { // Issue #608
-			err = c.NoContent(he.Code)
-		} else {
-			err = c.JSON(code, message)
-		}
-		if err != nil {
-			e.Logger.Error(err)
-		}
+	if c.Request().Method == http.MethodHead { // Issue #608
+		err = c.NoContent(he.Code)
+	} else {
+		err = c.JSON(code, message)
+	}
+	if err != nil {
+		e.Logger.Error(err)
 	}
 }
 
diff --git a/echo_test.go b/echo_test.go
index dc553490b..f28915864 100644
--- a/echo_test.go
+++ b/echo_test.go
@@ -1124,6 +1124,15 @@ func TestDefaultHTTPErrorHandler(t *testing.T) {
 			"error":   "stackinfo",
 		})
 	})
+	e.Any("/early-return", func(c Context) error {
+		c.String(http.StatusOK, "OK")
+		return errors.New("ERROR")
+	})
+	e.GET("/internal-error", func(c Context) error {
+		err := errors.New("internal error message body")
+		return NewHTTPError(http.StatusBadRequest).SetInternal(err)
+	})
+
 	// With Debug=true plain response contains error message
 	c, b := request(http.MethodGet, "/plain", e)
 	assert.Equal(t, http.StatusInternalServerError, c)
@@ -1136,6 +1145,14 @@ func TestDefaultHTTPErrorHandler(t *testing.T) {
 	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)
+	// if the body is already set HTTPErrorHandler should not add anything to response body
+	c, b = request(http.MethodGet, "/early-return", e)
+	assert.Equal(t, http.StatusOK, c)
+	assert.Equal(t, "OK", b)
+	// internal error should be reflected in the message
+	c, b = request(http.MethodGet, "/internal-error", e)
+	assert.Equal(t, http.StatusBadRequest, c)
+	assert.Equal(t, "{\n  \"error\": \"code=400, message=Bad Request, internal=internal error message body\",\n  \"message\": \"Bad Request\"\n}\n", b)
 
 	e.Debug = false
 	// With Debug=false the error response is shortened