From 963acc4b0ce297782405d4aefd6fe173ff657b1f Mon Sep 17 00:00:00 2001 From: Javier Provecho Fernandez Date: Mon, 9 Jan 2017 16:24:48 +0100 Subject: [PATCH] Fix #198 (#781) * Add new function to Render interface for writing content type only * Add support for the new function in Render interface for writing content-type only * Fix unhandled merge conflict in context_test.go * Update vendor.json --- context.go | 30 ++++++++--- context_test.go | 132 ++++++++++++++++++++++++++++++++++++++++++++- middleware_test.go | 2 +- render/data.go | 15 +++--- render/html.go | 7 ++- render/json.go | 29 ++++++---- render/redirect.go | 2 + render/render.go | 1 + render/text.go | 5 +- render/xml.go | 6 ++- render/yaml.go | 6 ++- vendor/vendor.json | 12 ++--- 12 files changed, 213 insertions(+), 34 deletions(-) diff --git a/context.go b/context.go index d19e1ee9..e937a4c7 100644 --- a/context.go +++ b/context.go @@ -17,7 +17,7 @@ import ( "github.com/gin-gonic/gin/binding" "github.com/gin-gonic/gin/render" - "github.com/manucorporat/sse" + "gopkg.in/gin-contrib/sse.v0" ) // Content-Type MIME of the most common data formats @@ -405,6 +405,19 @@ func (c *Context) requestHeader(key string) string { /******** RESPONSE RENDERING ********/ /************************************/ +// bodyAllowedForStatus is a copy of http.bodyAllowedForStatus non-exported function +func bodyAllowedForStatus(status int) bool { + switch { + case status >= 100 && status <= 199: + return false + case status == 204: + return false + case status == 304: + return false + } + return true +} + func (c *Context) Status(code int) { c.writermem.WriteHeader(code) } @@ -454,6 +467,13 @@ func (c *Context) Cookie(name string) (string, error) { func (c *Context) Render(code int, r render.Render) { c.Status(code) + + if !bodyAllowedForStatus(code) { + r.WriteContentType(c.Writer) + c.Writer.WriteHeaderNow() + return + } + if err := r.Render(c.Writer); err != nil { panic(err) } @@ -478,10 +498,7 @@ func (c *Context) IndentedJSON(code int, obj interface{}) { // JSON serializes the given struct as JSON into the response body. // It also sets the Content-Type as "application/json". func (c *Context) JSON(code int, obj interface{}) { - c.Status(code) - if err := render.WriteJSON(c.Writer, obj); err != nil { - panic(err) - } + c.Render(code, render.JSON{Data: obj}) } // XML serializes the given struct as XML into the response body. @@ -497,8 +514,7 @@ func (c *Context) YAML(code int, obj interface{}) { // String writes the given string into the response body. func (c *Context) String(code int, format string, values ...interface{}) { - c.Status(code) - render.WriteString(c.Writer, format, values) + c.Render(code, render.String{Format: format, Data: values}) } // Redirect returns a HTTP redirect to the specific location. diff --git a/context_test.go b/context_test.go index 7dc3085b..fd8ca4a2 100644 --- a/context_test.go +++ b/context_test.go @@ -7,6 +7,7 @@ package gin import ( "bytes" "errors" + "fmt" "html/template" "mime/multipart" "net/http" @@ -15,9 +16,9 @@ import ( "testing" "time" - "github.com/manucorporat/sse" "github.com/stretchr/testify/assert" "golang.org/x/net/context" + "gopkg.in/gin-contrib/sse.v0" ) var _ context.Context = &Context{} @@ -381,6 +382,35 @@ func TestContextGetCookie(t *testing.T) { assert.Equal(t, cookie, "gin") } +func TestContextBodyAllowedForStatus(t *testing.T) { + assert.Equal(t, false, bodyAllowedForStatus(102)) + assert.Equal(t, false, bodyAllowedForStatus(204)) + assert.Equal(t, false, bodyAllowedForStatus(304)) + assert.Equal(t, true, bodyAllowedForStatus(500)) +} + +type TestPanicRender struct { +} + +func (*TestPanicRender) Render(http.ResponseWriter) error { + return errors.New("TestPanicRender") +} + +func (*TestPanicRender) WriteContentType(http.ResponseWriter) {} + +func TestContextRenderPanicIfErr(t *testing.T) { + defer func() { + r := recover() + assert.Equal(t, "TestPanicRender", fmt.Sprint(r)) + }() + w := httptest.NewRecorder() + c, _ := CreateTestContext(w) + + c.Render(http.StatusOK, &TestPanicRender{}) + + assert.Fail(t, "Panic not detected") +} + // Tests that the response is serialized as JSON // and Content-Type is set to application/json func TestContextRenderJSON(t *testing.T) { @@ -394,6 +424,18 @@ func TestContextRenderJSON(t *testing.T) { assert.Equal(t, "application/json; charset=utf-8", w.HeaderMap.Get("Content-Type")) } +// Tests that no JSON is rendered if code is 204 +func TestContextRenderNoContentJSON(t *testing.T) { + w := httptest.NewRecorder() + c, _ := CreateTestContext(w) + + c.JSON(204, H{"foo": "bar"}) + + assert.Equal(t, 204, w.Code) + assert.Equal(t, "", w.Body.String()) + assert.Equal(t, "application/json; charset=utf-8", w.HeaderMap.Get("Content-Type")) +} + // Tests that the response is serialized as JSON // we change the content-type before func TestContextRenderAPIJSON(t *testing.T) { @@ -408,6 +450,19 @@ func TestContextRenderAPIJSON(t *testing.T) { assert.Equal(t, "application/vnd.api+json", w.HeaderMap.Get("Content-Type")) } +// Tests that no Custom JSON is rendered if code is 204 +func TestContextRenderNoContentAPIJSON(t *testing.T) { + w := httptest.NewRecorder() + c, _ := CreateTestContext(w) + + c.Header("Content-Type", "application/vnd.api+json") + c.JSON(204, H{"foo": "bar"}) + + assert.Equal(t, 204, w.Code) + assert.Equal(t, "", w.Body.String()) + assert.Equal(t, w.HeaderMap.Get("Content-Type"), "application/vnd.api+json") +} + // Tests that the response is serialized as JSON // and Content-Type is set to application/json func TestContextRenderIndentedJSON(t *testing.T) { @@ -421,6 +476,18 @@ func TestContextRenderIndentedJSON(t *testing.T) { assert.Equal(t, w.HeaderMap.Get("Content-Type"), "application/json; charset=utf-8") } +// Tests that no Custom JSON is rendered if code is 204 +func TestContextRenderNoContentIndentedJSON(t *testing.T) { + w := httptest.NewRecorder() + c, _ := CreateTestContext(w) + + c.IndentedJSON(204, H{"foo": "bar", "bar": "foo", "nested": H{"foo": "bar"}}) + + assert.Equal(t, 204, w.Code) + assert.Equal(t, "", w.Body.String()) + assert.Equal(t, w.HeaderMap.Get("Content-Type"), "application/json; charset=utf-8") +} + // Tests that the response executes the templates // and responds with Content-Type set to text/html func TestContextRenderHTML(t *testing.T) { @@ -436,6 +503,20 @@ func TestContextRenderHTML(t *testing.T) { assert.Equal(t, w.HeaderMap.Get("Content-Type"), "text/html; charset=utf-8") } +// Tests that no HTML is rendered if code is 204 +func TestContextRenderNoContentHTML(t *testing.T) { + w := httptest.NewRecorder() + c, router := CreateTestContext(w) + templ := template.Must(template.New("t").Parse(`Hello {{.name}}`)) + router.SetHTMLTemplate(templ) + + c.HTML(204, "t", H{"name": "alexandernyquist"}) + + assert.Equal(t, 204, w.Code) + assert.Equal(t, "", w.Body.String()) + assert.Equal(t, w.HeaderMap.Get("Content-Type"), "text/html; charset=utf-8") +} + // TestContextXML tests that the response is serialized as XML // and Content-Type is set to application/xml func TestContextRenderXML(t *testing.T) { @@ -449,6 +530,18 @@ func TestContextRenderXML(t *testing.T) { assert.Equal(t, w.HeaderMap.Get("Content-Type"), "application/xml; charset=utf-8") } +// Tests that no XML is rendered if code is 204 +func TestContextRenderNoContentXML(t *testing.T) { + w := httptest.NewRecorder() + c, _ := CreateTestContext(w) + + c.XML(204, H{"foo": "bar"}) + + assert.Equal(t, 204, w.Code) + assert.Equal(t, "", w.Body.String()) + assert.Equal(t, w.HeaderMap.Get("Content-Type"), "application/xml; charset=utf-8") +} + // TestContextString tests that the response is returned // with Content-Type set to text/plain func TestContextRenderString(t *testing.T) { @@ -462,6 +555,18 @@ func TestContextRenderString(t *testing.T) { assert.Equal(t, w.HeaderMap.Get("Content-Type"), "text/plain; charset=utf-8") } +// Tests that no String is rendered if code is 204 +func TestContextRenderNoContentString(t *testing.T) { + w := httptest.NewRecorder() + c, _ := CreateTestContext(w) + + c.String(204, "test %s %d", "string", 2) + + assert.Equal(t, 204, w.Code) + assert.Equal(t, "", w.Body.String()) + assert.Equal(t, w.HeaderMap.Get("Content-Type"), "text/plain; charset=utf-8") +} + // TestContextString tests that the response is returned // with Content-Type set to text/html func TestContextRenderHTMLString(t *testing.T) { @@ -476,6 +581,19 @@ func TestContextRenderHTMLString(t *testing.T) { assert.Equal(t, w.HeaderMap.Get("Content-Type"), "text/html; charset=utf-8") } +// Tests that no HTML String is rendered if code is 204 +func TestContextRenderNoContentHTMLString(t *testing.T) { + w := httptest.NewRecorder() + c, _ := CreateTestContext(w) + + c.Header("Content-Type", "text/html; charset=utf-8") + c.String(204, "%s %d", "string", 3) + + assert.Equal(t, 204, w.Code) + assert.Equal(t, "", w.Body.String()) + assert.Equal(t, w.HeaderMap.Get("Content-Type"), "text/html; charset=utf-8") +} + // TestContextData tests that the response can be written from `bytesting` // with specified MIME type func TestContextRenderData(t *testing.T) { @@ -489,6 +607,18 @@ func TestContextRenderData(t *testing.T) { assert.Equal(t, w.HeaderMap.Get("Content-Type"), "text/csv") } +// Tests that no Custom Data is rendered if code is 204 +func TestContextRenderNoContentData(t *testing.T) { + w := httptest.NewRecorder() + c, _ := CreateTestContext(w) + + c.Data(204, "text/csv", []byte(`foo,bar`)) + + assert.Equal(t, 204, w.Code) + assert.Equal(t, "", w.Body.String()) + assert.Equal(t, w.HeaderMap.Get("Content-Type"), "text/csv") +} + func TestContextRenderSSE(t *testing.T) { w := httptest.NewRecorder() c, _ := CreateTestContext(w) diff --git a/middleware_test.go b/middleware_test.go index 273d003d..c77f8276 100644 --- a/middleware_test.go +++ b/middleware_test.go @@ -10,8 +10,8 @@ import ( "testing" - "github.com/manucorporat/sse" "github.com/stretchr/testify/assert" + "gopkg.in/gin-contrib/sse.v0" ) func TestMiddlewareGeneralCase(t *testing.T) { diff --git a/render/data.go b/render/data.go index efa75d55..c296042c 100644 --- a/render/data.go +++ b/render/data.go @@ -11,10 +11,13 @@ type Data struct { Data []byte } -func (r Data) Render(w http.ResponseWriter) error { - if len(r.ContentType) > 0 { - w.Header()["Content-Type"] = []string{r.ContentType} - } - w.Write(r.Data) - return nil +// Render (Data) writes data with custom ContentType +func (r Data) Render(w http.ResponseWriter) (err error) { + r.WriteContentType(w) + _, err = w.Write(r.Data) + return +} + +func (r Data) WriteContentType(w http.ResponseWriter) { + writeContentType(w, []string{r.ContentType}) } diff --git a/render/html.go b/render/html.go index 8bfb23ac..a3cbda67 100644 --- a/render/html.go +++ b/render/html.go @@ -58,9 +58,14 @@ func (r HTMLDebug) loadTemplate() *template.Template { } func (r HTML) Render(w http.ResponseWriter) error { - writeContentType(w, htmlContentType) + r.WriteContentType(w) + if len(r.Name) == 0 { return r.Template.Execute(w, r.Data) } return r.Template.ExecuteTemplate(w, r.Name, r.Data) } + +func (r HTML) WriteContentType(w http.ResponseWriter) { + writeContentType(w, htmlContentType) +} diff --git a/render/json.go b/render/json.go index b652c4c8..3ee8b132 100644 --- a/render/json.go +++ b/render/json.go @@ -21,18 +21,15 @@ type ( var jsonContentType = []string{"application/json; charset=utf-8"} -func (r JSON) Render(w http.ResponseWriter) error { - return WriteJSON(w, r.Data) +func (r JSON) Render(w http.ResponseWriter) (err error) { + if err = WriteJSON(w, r.Data); err != nil { + panic(err) + } + return } -func (r IndentedJSON) Render(w http.ResponseWriter) error { +func (r JSON) WriteContentType(w http.ResponseWriter) { writeContentType(w, jsonContentType) - jsonBytes, err := json.MarshalIndent(r.Data, "", " ") - if err != nil { - return err - } - w.Write(jsonBytes) - return nil } func WriteJSON(w http.ResponseWriter, obj interface{}) error { @@ -44,3 +41,17 @@ func WriteJSON(w http.ResponseWriter, obj interface{}) error { w.Write(jsonBytes) return nil } + +func (r IndentedJSON) Render(w http.ResponseWriter) error { + r.WriteContentType(w) + jsonBytes, err := json.MarshalIndent(r.Data, "", " ") + if err != nil { + return err + } + w.Write(jsonBytes) + return nil +} + +func (r IndentedJSON) WriteContentType(w http.ResponseWriter) { + writeContentType(w, jsonContentType) +} diff --git a/render/redirect.go b/render/redirect.go index bd48d7d8..f874a351 100644 --- a/render/redirect.go +++ b/render/redirect.go @@ -22,3 +22,5 @@ func (r Redirect) Render(w http.ResponseWriter) error { http.Redirect(w, r.Request, r.Location, r.Code) return nil } + +func (r Redirect) WriteContentType(http.ResponseWriter) {} diff --git a/render/render.go b/render/render.go index 3808666a..7e997374 100644 --- a/render/render.go +++ b/render/render.go @@ -8,6 +8,7 @@ import "net/http" type Render interface { Render(http.ResponseWriter) error + WriteContentType(w http.ResponseWriter) } var ( diff --git a/render/text.go b/render/text.go index 5a9e280b..74cd26be 100644 --- a/render/text.go +++ b/render/text.go @@ -22,9 +22,12 @@ func (r String) Render(w http.ResponseWriter) error { return nil } +func (r String) WriteContentType(w http.ResponseWriter) { + writeContentType(w, plainContentType) +} + func WriteString(w http.ResponseWriter, format string, data []interface{}) { writeContentType(w, plainContentType) - if len(data) > 0 { fmt.Fprintf(w, format, data...) } else { diff --git a/render/xml.go b/render/xml.go index be22e6f2..cff1ac3e 100644 --- a/render/xml.go +++ b/render/xml.go @@ -16,6 +16,10 @@ type XML struct { var xmlContentType = []string{"application/xml; charset=utf-8"} func (r XML) Render(w http.ResponseWriter) error { - writeContentType(w, xmlContentType) + r.WriteContentType(w) return xml.NewEncoder(w).Encode(r.Data) } + +func (r XML) WriteContentType(w http.ResponseWriter) { + writeContentType(w, xmlContentType) +} diff --git a/render/yaml.go b/render/yaml.go index 46937d88..25d0ebd4 100644 --- a/render/yaml.go +++ b/render/yaml.go @@ -17,7 +17,7 @@ type YAML struct { var yamlContentType = []string{"application/x-yaml; charset=utf-8"} func (r YAML) Render(w http.ResponseWriter) error { - writeContentType(w, yamlContentType) + r.WriteContentType(w) bytes, err := yaml.Marshal(r.Data) if err != nil { @@ -27,3 +27,7 @@ func (r YAML) Render(w http.ResponseWriter) error { w.Write(bytes) return nil } + +func (r YAML) WriteContentType(w http.ResponseWriter) { + writeContentType(w, yamlContentType) +} diff --git a/vendor/vendor.json b/vendor/vendor.json index 3c6b0edb..e754a3fd 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -21,12 +21,6 @@ "revision": "8ee79997227bf9b34611aee7946ae64735e6fd93", "revisionTime": "2016-11-17T03:31:26Z" }, - { - "checksumSHA1": "b0T0Hzd+zYk+OCDTFMps+jwa/nY=", - "path": "github.com/manucorporat/sse", - "revision": "ee05b128a739a0fb76c7ebd3ae4810c1de808d6d", - "revisionTime": "2016-01-26T18:01:36Z" - }, { "checksumSHA1": "9if9IBLsxkarJ804NPWAzgskIAk=", "path": "github.com/manucorporat/stats", @@ -66,6 +60,12 @@ "revision": "478fcf54317e52ab69f40bb4c7a1520288d7f7ea", "revisionTime": "2016-12-05T15:46:50Z" }, + { + "checksumSHA1": "pyAPYrymvmZl0M/Mr4yfjOQjA8I=", + "path": "gopkg.in/gin-contrib/sse.v0", + "revision": "22d885f9ecc78bf4ee5d72b937e4bbcdc58e8cae", + "revisionTime": "2017-01-09T09:34:21Z" + }, { "checksumSHA1": "39V1idWER42Lmcmg2Uy40wMzOlo=", "comment": "v8.18.1",