Skip to content

Commit fbb7286

Browse files
noamtvishr
authored andcommitted
Fix for #1334 (#1335)
* echo.context.cjson should encode the JSON before writing the status code #1334 : `response.Write` automatically sets status to `200` if a response code wasn't committed yet. This is convenient, but it ignores the fact that `response.Status` is a public field that may be set separately/before `response.Write` has been called A `response.Status` is by default `0`, or `200` if the response was reset, so `response.Write` should fallback to `200` only if a code wasn't set yet. * echo.context.cjson should encode the JSON before writing the status code #1334 : Writing the response code before encoding the payload is prone to error. If JSON encoding fails, the response code is already committed, the server is able to only modify the response body to reflect the error and the user receives an awkward response where the status is successful but the body reports an error. Instead - set the desired code on `c.response.Status`. If writing eventually takes place, the desired code is committed. If an error occurs, the server can still change the response.
1 parent e2671fe commit fbb7286

File tree

4 files changed

+53
-2
lines changed

4 files changed

+53
-2
lines changed

context.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ func (c *context) json(code int, i interface{}, indent string) error {
437437
enc.SetIndent("", indent)
438438
}
439439
c.writeContentType(MIMEApplicationJSONCharsetUTF8)
440-
c.response.WriteHeader(code)
440+
c.response.Status = code
441441
return enc.Encode(i)
442442
}
443443

context_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"errors"
99
"fmt"
1010
"io"
11+
"math"
1112
"mime/multipart"
1213
"net/http"
1314
"net/http/httptest"
@@ -374,6 +375,34 @@ func TestContext(t *testing.T) {
374375
assert.Equal(0, len(c.QueryParams()))
375376
}
376377

378+
func TestContext_JSON_CommitsCustomResponseCode(t *testing.T) {
379+
e := New()
380+
req := httptest.NewRequest(http.MethodGet, "/", nil)
381+
rec := httptest.NewRecorder()
382+
c := e.NewContext(req, rec).(*context)
383+
err := c.JSON(http.StatusCreated, user{1, "Jon Snow"})
384+
385+
assert := testify.New(t)
386+
if assert.NoError(err) {
387+
assert.Equal(http.StatusCreated, rec.Code)
388+
assert.Equal(MIMEApplicationJSONCharsetUTF8, rec.Header().Get(HeaderContentType))
389+
assert.Equal(userJSON+"\n", rec.Body.String())
390+
}
391+
}
392+
393+
func TestContext_JSON_DoesntCommitResponseCodePrematurely(t *testing.T) {
394+
e := New()
395+
req := httptest.NewRequest(http.MethodGet, "/", nil)
396+
rec := httptest.NewRecorder()
397+
c := e.NewContext(req, rec).(*context)
398+
err := c.JSON(http.StatusCreated, map[string]float64{"a": math.NaN()})
399+
400+
assert := testify.New(t)
401+
if assert.Error(err) {
402+
assert.False(c.response.Committed)
403+
}
404+
}
405+
377406
func TestContextCookie(t *testing.T) {
378407
e := New()
379408
req := httptest.NewRequest(http.MethodGet, "/", nil)

response.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ func (r *Response) WriteHeader(code int) {
6767
// Write writes the data to the connection as part of an HTTP reply.
6868
func (r *Response) Write(b []byte) (n int, err error) {
6969
if !r.Committed {
70-
r.WriteHeader(http.StatusOK)
70+
if r.Status == 0 {
71+
r.Status = http.StatusOK
72+
}
73+
r.WriteHeader(r.Status)
7174
}
7275
n, err = r.Writer.Write(b)
7376
r.Size += int64(n)

response_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,22 @@ func TestResponse(t *testing.T) {
2222
res.Write([]byte("test"))
2323
assert.Equal(t, "echo", rec.Header().Get(HeaderServer))
2424
}
25+
26+
func TestResponse_Write_FallsBackToDefaultStatus(t *testing.T) {
27+
e := New()
28+
rec := httptest.NewRecorder()
29+
res := &Response{echo: e, Writer: rec}
30+
31+
res.Write([]byte("test"))
32+
assert.Equal(t, http.StatusOK, rec.Code)
33+
}
34+
35+
func TestResponse_Write_UsesSetResponseCode(t *testing.T) {
36+
e := New()
37+
rec := httptest.NewRecorder()
38+
res := &Response{echo: e, Writer: rec}
39+
40+
res.Status = http.StatusBadRequest
41+
res.Write([]byte("test"))
42+
assert.Equal(t, http.StatusBadRequest, rec.Code)
43+
}

0 commit comments

Comments
 (0)