From b8db13849595c0ef07d5f66bb764579fffb062c5 Mon Sep 17 00:00:00 2001 From: Thibault Jamet Date: Wed, 7 Mar 2018 01:11:14 +0100 Subject: [PATCH 1/2] Fix intercepting headers in middlewares As explained in the TestInterceptedHeader test, in case a middleware filters out the headers, this middleware can be done inefficient in case one following handler is using c.String or other methods writing to the response body directly. This commit fixes the issue by using c.Writer when writing the Status as done in other c.Header, c.SetCookie and other response writers. The bug has been originally discovered using https://github.com/gin-contrib/gzip where a failing test has been added here: https://github.com/tjamet/gzip/blob/header/gzip_test.go#L71 Signed-off-by: Thibault Jamet --- context.go | 2 +- context_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/context.go b/context.go index 5dc7f8a0f5..496016c7da 100644 --- a/context.go +++ b/context.go @@ -711,7 +711,7 @@ func bodyAllowedForStatus(status int) bool { // Status sets the HTTP response code. func (c *Context) Status(code int) { - c.writermem.WriteHeader(code) + c.Writer.WriteHeader(code) } // Header is a intelligent shortcut for c.Writer.Header().Set(key, value). diff --git a/context_test.go b/context_test.go index 0da5fbe64e..47ecfb4ead 100644 --- a/context_test.go +++ b/context_test.go @@ -1812,3 +1812,42 @@ func TestContextResetInHandler(t *testing.T) { c.Next() }) } + +type interceptedWriter struct { + ResponseWriter + b *bytes.Buffer +} + +func (i interceptedWriter) WriteHeader(code int) { + i.Header().Del("X-Test") + i.ResponseWriter.WriteHeader(code) +} + +func TestInterceptedHeader(t *testing.T) { + w := httptest.NewRecorder() + c, r := CreateTestContext(w) + + r.Use(func(c *Context) { + i := interceptedWriter{ + ResponseWriter: c.Writer, + b: bytes.NewBuffer(nil), + } + c.Writer = i + c.Next() + c.Header("X-Test", "overridden") + c.Writer = i.ResponseWriter + }) + r.GET("/", func(c *Context) { + c.Header("X-Test", "original") + c.Header("X-Test-2", "present") + c.String(http.StatusOK, "hello world") + }) + c.Request = httptest.NewRequest("GET", "/", nil) + r.HandleContext(c) + // Result() has headers frozen when WriteHeaderNow() has been called + // Compared to this time, this is when the response headers will be flushed + // As response is flushed on c.String, the Header cannot be set by the first + // middleware. Assert this + assert.Equal(t, "", w.Result().Header.Get("X-Test")) + assert.Equal(t, "present", w.Result().Header.Get("X-Test-2")) +} From 520df2949b9b76a37c7b0e153e79d50f875b73ec Mon Sep 17 00:00:00 2001 From: Thibault Jamet Date: Wed, 7 Mar 2018 01:36:58 +0100 Subject: [PATCH 2/2] Skip Intercepted Header test for go <1.6 Signed-off-by: Thibault Jamet --- context_go17_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++++ context_test.go | 39 ---------------------------------- 2 files changed, 50 insertions(+), 39 deletions(-) create mode 100644 context_go17_test.go diff --git a/context_go17_test.go b/context_go17_test.go new file mode 100644 index 0000000000..eca089ce09 --- /dev/null +++ b/context_go17_test.go @@ -0,0 +1,50 @@ +// +build go1.7 + +package gin + +import ( + "bytes" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +type interceptedWriter struct { + ResponseWriter + b *bytes.Buffer +} + +func (i interceptedWriter) WriteHeader(code int) { + i.Header().Del("X-Test") + i.ResponseWriter.WriteHeader(code) +} +func TestInterceptedHeader(t *testing.T) { + w := httptest.NewRecorder() + c, r := CreateTestContext(w) + + r.Use(func(c *Context) { + i := interceptedWriter{ + ResponseWriter: c.Writer, + b: bytes.NewBuffer(nil), + } + c.Writer = i + c.Next() + c.Header("X-Test", "overridden") + c.Writer = i.ResponseWriter + }) + r.GET("/", func(c *Context) { + c.Header("X-Test", "original") + c.Header("X-Test-2", "present") + c.String(http.StatusOK, "hello world") + }) + c.Request = httptest.NewRequest("GET", "/", nil) + r.HandleContext(c) + // Result() has headers frozen when WriteHeaderNow() has been called + // Compared to this time, this is when the response headers will be flushed + // As response is flushed on c.String, the Header cannot be set by the first + // middleware. Assert this + assert.Equal(t, "", w.Result().Header.Get("X-Test")) + assert.Equal(t, "present", w.Result().Header.Get("X-Test-2")) +} diff --git a/context_test.go b/context_test.go index 47ecfb4ead..0da5fbe64e 100644 --- a/context_test.go +++ b/context_test.go @@ -1812,42 +1812,3 @@ func TestContextResetInHandler(t *testing.T) { c.Next() }) } - -type interceptedWriter struct { - ResponseWriter - b *bytes.Buffer -} - -func (i interceptedWriter) WriteHeader(code int) { - i.Header().Del("X-Test") - i.ResponseWriter.WriteHeader(code) -} - -func TestInterceptedHeader(t *testing.T) { - w := httptest.NewRecorder() - c, r := CreateTestContext(w) - - r.Use(func(c *Context) { - i := interceptedWriter{ - ResponseWriter: c.Writer, - b: bytes.NewBuffer(nil), - } - c.Writer = i - c.Next() - c.Header("X-Test", "overridden") - c.Writer = i.ResponseWriter - }) - r.GET("/", func(c *Context) { - c.Header("X-Test", "original") - c.Header("X-Test-2", "present") - c.String(http.StatusOK, "hello world") - }) - c.Request = httptest.NewRequest("GET", "/", nil) - r.HandleContext(c) - // Result() has headers frozen when WriteHeaderNow() has been called - // Compared to this time, this is when the response headers will be flushed - // As response is flushed on c.String, the Header cannot be set by the first - // middleware. Assert this - assert.Equal(t, "", w.Result().Header.Get("X-Test")) - assert.Equal(t, "present", w.Result().Header.Get("X-Test-2")) -}