Fixed #424, #427

Signed-off-by: Vishal Rana <vr@labstack.com>

echo.go

@@ -131,9 +131,7 @@ type (
 	}
 )
 
-//--------------
 // HTTP methods
-//--------------
 const (
 	CONNECT = "CONNECT"
 	DELETE  = "DELETE"
@@ -146,9 +144,7 @@ const (
 	TRACE   = "TRACE"
 )
 
-//-------------
 // Media types
-//-------------
 const (
 	ApplicationJSON                  = "application/json"
 	ApplicationJSONCharsetUTF8       = ApplicationJSON + "; " + CharsetUTF8
@@ -167,16 +163,12 @@ const (
 	OctetStream                      = "application/octet-stream"
 )
 
-//---------
 // Charset
-//---------
 const (
 	CharsetUTF8 = "charset=utf-8"
 )
 
-//---------
 // Headers
-//---------
 const (
 	AcceptEncoding     = "Accept-Encoding"
 	Authorization      = "Authorization"
@@ -207,9 +199,7 @@ var (
 	}
 )
 
-//--------
 // Errors
-//--------
 var (
 	ErrUnsupportedMediaType  = NewHTTPError(http.StatusUnsupportedMediaType)
 	ErrNotFound              = NewHTTPError(http.StatusNotFound)
@@ -219,9 +209,7 @@ var (
 	ErrInvalidRedirectCode   = errors.New("invalid redirect status code")
 )
 
-//----------------
 // Error handlers
-//----------------
 var (
 	notFoundHandler = HandlerFunc(func(c Context) error {
 		return ErrNotFound

middleware/compress.go

@@ -48,14 +48,22 @@ func GzipFromConfig(config GzipConfig) echo.MiddlewareFunc {
 		return echo.HandlerFunc(func(c echo.Context) error {
 			c.Response().Header().Add(echo.Vary, echo.AcceptEncoding)
 			if strings.Contains(c.Request().Header().Get(echo.AcceptEncoding), scheme) {
-				w := pool.Get().(*gzip.Writer)
-				w.Reset(c.Response().Writer())
+				rw := c.Response().Writer()
+				gw := pool.Get().(*gzip.Writer)
+				gw.Reset(rw)
 				defer func() {
-					w.Close()
-					pool.Put(w)
-					w.Close()
+					if c.Response().Size() == 0 {
+						// We have to reset response to it's pristine state when
+						// nothing is written to body or error is returned.
+						// See issue #424, #407.
+						c.Response().SetWriter(rw)
+						c.Response().Header().Del(echo.ContentEncoding)
+						gw.Reset(ioutil.Discard)
+					}
+					gw.Close()
+					pool.Put(gw)
 				}()
-				g := gzipResponseWriter{Response: c.Response(), Writer: w}
+				g := gzipResponseWriter{Response: c.Response(), Writer: gw}
 				c.Response().Header().Set(echo.ContentEncoding, scheme)
 				c.Response().SetWriter(g)
 			}

middleware/compress_test.go

@@ -3,6 +3,8 @@ package middleware
 import (
 	"bytes"
 	"compress/gzip"
+	"io/ioutil"
+	"net/http"
 	"testing"
 
 	"github.com/labstack/echo"
@@ -10,31 +12,12 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-type closeNotifyingRecorder struct {
-	*test.ResponseRecorder
-	closed chan bool
-}
-
-func newCloseNotifyingRecorder() *closeNotifyingRecorder {
-	return &closeNotifyingRecorder{
-		test.NewResponseRecorder(),
-		make(chan bool, 1),
-	}
-}
-
-func (c *closeNotifyingRecorder) close() {
-	c.closed <- true
-}
-
-func (c *closeNotifyingRecorder) CloseNotify() <-chan bool {
-	return c.closed
-}
-
 func TestGzip(t *testing.T) {
 	e := echo.New()
 	req := test.NewRequest(echo.GET, "/", nil)
 	rec := test.NewResponseRecorder()
 	c := echo.NewContext(req, rec, e)
+
 	// Skip if no Accept-Encoding header
 	h := Gzip()(echo.HandlerFunc(func(c echo.Context) error {
 		c.Response().Write([]byte("test")) // For Content-Type sniffing
@@ -50,7 +33,6 @@ func TestGzip(t *testing.T) {
 
 	// Gzip
 	h.Handle(c)
-	// assert.Equal(t, http.StatusOK, rec.Status())
 	assert.Equal(t, "gzip", rec.Header().Get(echo.ContentEncoding))
 	assert.Contains(t, rec.Header().Get(echo.ContentType), echo.TextPlain)
 	r, err := gzip.NewReader(rec.Body)
@@ -62,79 +44,37 @@ func TestGzip(t *testing.T) {
 	}
 }
 
-// func TestGzipFlush(t *testing.T) {
-// 	res := test.NewResponseRecorder()
-// 	buf := new(bytes.Buffer)
-// 	w := gzip.NewWriter(buf)
-// 	gw := gzipWriter{Writer: w, ResponseWriter: res}
-//
-// 	n0 := buf.Len()
-// 	if n0 != 0 {
-// 		t.Fatalf("buffer size = %d before writes; want 0", n0)
-// 	}
-//
-// 	if err := gw.Flush(); err != nil {
-// 		t.Fatal(err)
-// 	}
-//
-// 	n1 := buf.Len()
-// 	if n1 == 0 {
-// 		t.Fatal("no data after first flush")
-// 	}
-//
-// 	gw.Write([]byte("x"))
-//
-// 	n2 := buf.Len()
-// 	if n1 != n2 {
-// 		t.Fatalf("after writing a single byte, size changed from %d to %d; want no change", n1, n2)
-// 	}
-//
-// 	if err := gw.Flush(); err != nil {
-// 		t.Fatal(err)
-// 	}
-//
-// 	n3 := buf.Len()
-// 	if n2 == n3 {
-// 		t.Fatal("Flush didn't flush any data")
-// 	}
-// }
+func TestGzipNoContent(t *testing.T) {
+	e := echo.New()
+	req := test.NewRequest(echo.GET, "/", nil)
+	rec := test.NewResponseRecorder()
+	c := echo.NewContext(req, rec, e)
+	h := Gzip()(echo.HandlerFunc(func(c echo.Context) error {
+		return c.NoContent(http.StatusOK)
+	}))
+	h.Handle(c)
 
-// func TestGzipCloseNotify(t *testing.T) {
-// 	rec := newCloseNotifyingRecorder()
-// 	buf := new(bytes.Buffer)
-// 	w := gzip.NewWriter(buf)
-// 	gw := gzipWriter{Writer: w, ResponseWriter: rec}
-// 	closed := false
-// 	notifier := gw.CloseNotify()
-// 	rec.close()
-//
-// 	select {
-// 	case <-notifier:
-// 		closed = true
-// 	case <-time.After(time.Second):
-// 	}
-//
-// 	assert.Equal(t, closed, true)
-// }
-//
-// func BenchmarkGzip(b *testing.B) {
-// 	b.StopTimer()
-// 	b.ReportAllocs()
-//
-// 	h := func(c echo.Context) error {
-// 		c.Response().Write([]byte("test")) // For Content-Type sniffing
-// 		return nil
-// 	}
-// 	req, _ := http.NewRequest(echo.GET, "/", nil)
-// 	req.Header().Set(echo.AcceptEncoding, "gzip")
-//
-// 	b.StartTimer()
-//
-// 	for i := 0; i < b.N; i++ {
-// 		e := echo.New()
-// 		res := test.NewResponseRecorder()
-// 		c := echo.NewContext(req, res, e)
-// 		Gzip()(h)(c)
-// 	}
-//
-// }
+	assert.Empty(t, rec.Header().Get(echo.ContentEncoding))
+	assert.Empty(t, rec.Header().Get(echo.ContentType))
+	b, err := ioutil.ReadAll(rec.Body)
+	if assert.NoError(t, err) {
+		assert.Equal(t, 0, len(b))
+	}
+}
+
+func TestGzipErrorReturned(t *testing.T) {
+	e := echo.New()
+	e.Use(Gzip())
+	e.Get("/", echo.HandlerFunc(func(c echo.Context) error {
+		return echo.NewHTTPError(http.StatusInternalServerError, "error")
+	}))
+	req := test.NewRequest(echo.GET, "/", nil)
+	rec := test.NewResponseRecorder()
+	e.ServeHTTP(req, rec)
+
+	assert.Empty(t, rec.Header().Get(echo.ContentEncoding))
+	b, err := ioutil.ReadAll(rec.Body)
+	if assert.NoError(t, err) {
+		assert.Equal(t, "error", string(b))
+	}
+}

middleware/logger.go

@@ -78,7 +78,7 @@ func LoggerFromConfig(config LoggerConfig) echo.MiddlewareFunc {
 
 			start := time.Now()
 			if err := next.Handle(c); err != nil {
-				return err
+				c.Error(err)
 			}
 			stop := time.Now()
 			method := []byte(req.Method())