From 713187fcbaa5e54ca831afdb69d9ed543a441ccf Mon Sep 17 00:00:00 2001 From: davidmdm Date: Wed, 3 Nov 2021 20:03:50 -0400 Subject: [PATCH 1/4] stream decompression instead of buffering --- middleware/decompress.go | 62 +++++++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/middleware/decompress.go b/middleware/decompress.go index c046359a2..b6dde7b81 100644 --- a/middleware/decompress.go +++ b/middleware/decompress.go @@ -86,35 +86,51 @@ func DecompressWithConfig(config DecompressConfig) echo.MiddlewareFunc { if config.Skipper(c) { return next(c) } - switch c.Request().Header.Get(echo.HeaderContentEncoding) { - case GZIPEncoding: - b := c.Request().Body - - i := pool.Get() - gr, ok := i.(*gzip.Reader) - if !ok { - return echo.NewHTTPError(http.StatusInternalServerError, i.(error).Error()) - } - if err := gr.Reset(b); err != nil { - pool.Put(gr) - if err == io.EOF { //ignore if body is empty - return next(c) - } - return err - } - var buf bytes.Buffer - io.Copy(&buf, gr) + if c.Request().Header.Get(echo.HeaderContentEncoding) != GZIPEncoding { + return next(c) + } - gr.Close() - pool.Put(gr) + b := c.Request().Body + + i := pool.Get() + gr, ok := i.(*gzip.Reader) + if !ok { + return echo.NewHTTPError(http.StatusInternalServerError, i.(error).Error()) + } - b.Close() // http.Request.Body is closed by the Server, but because we are replacing it, it must be closed here + if err := gr.Reset(b); err != nil { + pool.Put(gr) + if err == io.EOF { //ignore if body is empty + return next(c) + } + return err + } - r := ioutil.NopCloser(&buf) - c.Request().Body = r + c.Request().Body = readCloserCustom{ + gr, + func() error { + gr.Close() + b.Close() + pool.Put(gr) + return nil + }, } + return next(c) } + + } +} + +type readCloserCustom struct { + io.Reader + closeFunc func() error +} + +func (rc readCloserCustom) Close() error { + if rc.closeFunc == nil { + return nil } + return rc.closeFunc() } From 942b98097755a7d8cbae36421b480f8715ec210f Mon Sep 17 00:00:00 2001 From: davidmdm Date: Mon, 15 Nov 2021 13:45:25 -0500 Subject: [PATCH 2/4] simple body replace with gzip reader with deferred close --- middleware/decompress.go | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/middleware/decompress.go b/middleware/decompress.go index b6dde7b81..0adf72a66 100644 --- a/middleware/decompress.go +++ b/middleware/decompress.go @@ -107,30 +107,17 @@ func DecompressWithConfig(config DecompressConfig) echo.MiddlewareFunc { return err } - c.Request().Body = readCloserCustom{ - gr, - func() error { - gr.Close() - b.Close() - pool.Put(gr) - return nil - }, - } + defer func() error { + gr.Close() + b.Close() + pool.Put(gr) + return nil + }() + + c.Request().Body = gr return next(c) } } } - -type readCloserCustom struct { - io.Reader - closeFunc func() error -} - -func (rc readCloserCustom) Close() error { - if rc.closeFunc == nil { - return nil - } - return rc.closeFunc() -} From 71f28fdb7b8fee23561cbf4844bf34a8406f18c7 Mon Sep 17 00:00:00 2001 From: davidmdm Date: Tue, 16 Nov 2021 16:07:31 -0500 Subject: [PATCH 3/4] defer resource closes --- middleware/decompress.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/middleware/decompress.go b/middleware/decompress.go index 0adf72a66..2ff2fdb9b 100644 --- a/middleware/decompress.go +++ b/middleware/decompress.go @@ -91,29 +91,24 @@ func DecompressWithConfig(config DecompressConfig) echo.MiddlewareFunc { return next(c) } - b := c.Request().Body - i := pool.Get() gr, ok := i.(*gzip.Reader) if !ok { return echo.NewHTTPError(http.StatusInternalServerError, i.(error).Error()) } + defer pool.Put(gr) + defer gr.Close() + + b := c.Request().Body if err := gr.Reset(b); err != nil { - pool.Put(gr) if err == io.EOF { //ignore if body is empty return next(c) } return err } - defer func() error { - gr.Close() - b.Close() - pool.Put(gr) - return nil - }() - + defer b.Close() c.Request().Body = gr return next(c) From 77e02f062c62cf5af1d2c3d6f665be0b64e3cdf9 Mon Sep 17 00:00:00 2001 From: davidmdm Date: Wed, 1 Dec 2021 20:06:44 -0500 Subject: [PATCH 4/4] simply gzip.Reader pool --- middleware/decompress.go | 33 +++++++-------------------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/middleware/decompress.go b/middleware/decompress.go index 2ff2fdb9b..88ec70982 100644 --- a/middleware/decompress.go +++ b/middleware/decompress.go @@ -1,10 +1,8 @@ package middleware import ( - "bytes" "compress/gzip" "io" - "io/ioutil" "net/http" "sync" @@ -43,26 +41,7 @@ type DefaultGzipDecompressPool struct { } func (d *DefaultGzipDecompressPool) gzipDecompressPool() sync.Pool { - return sync.Pool{ - New: func() interface{} { - // create with an empty reader (but with GZIP header) - w, err := gzip.NewWriterLevel(ioutil.Discard, gzip.BestSpeed) - if err != nil { - return err - } - - b := new(bytes.Buffer) - w.Reset(b) - w.Flush() - w.Close() - - r, err := gzip.NewReader(bytes.NewReader(b.Bytes())) - if err != nil { - return err - } - return r - }, - } + return sync.Pool{New: func() interface{} { return new(gzip.Reader) }} } //Decompress decompresses request body based if content encoding type is set to "gzip" with default config @@ -82,6 +61,7 @@ func DecompressWithConfig(config DecompressConfig) echo.MiddlewareFunc { return func(next echo.HandlerFunc) echo.HandlerFunc { pool := config.GzipDecompressPool.gzipDecompressPool() + return func(c echo.Context) error { if config.Skipper(c) { return next(c) @@ -93,13 +73,13 @@ func DecompressWithConfig(config DecompressConfig) echo.MiddlewareFunc { i := pool.Get() gr, ok := i.(*gzip.Reader) - if !ok { + if !ok || gr == nil { return echo.NewHTTPError(http.StatusInternalServerError, i.(error).Error()) } defer pool.Put(gr) - defer gr.Close() b := c.Request().Body + defer b.Close() if err := gr.Reset(b); err != nil { if err == io.EOF { //ignore if body is empty @@ -108,11 +88,12 @@ func DecompressWithConfig(config DecompressConfig) echo.MiddlewareFunc { return err } - defer b.Close() + // only Close gzip reader if it was set to a proper gzip source otherwise it will panic on close. + defer gr.Close() + c.Request().Body = gr return next(c) } - } }