Skip to content

Commit e17303b

Browse files
committed
Context: json should not send status code before serialization is complete
1 parent 096ce41 commit e17303b

4 files changed

Lines changed: 50 additions & 4 deletions

File tree

context.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,8 @@ func (c *Context) Response() http.ResponseWriter {
139139
return c.response
140140
}
141141

142-
// SetResponse sets `*http.ResponseWriter`. Some middleware require that given ResponseWriter implements following
143-
// method `Unwrap() http.ResponseWriter` which eventually should return echo.Response instance.
142+
// SetResponse sets `*http.ResponseWriter`. Some context methods and/or middleware require that given ResponseWriter implements following
143+
// method `Unwrap() http.ResponseWriter` which eventually should return *echo.Response instance.
144144
func (c *Context) SetResponse(r http.ResponseWriter) {
145145
c.response = r
146146
}
@@ -453,7 +453,16 @@ func (c *Context) jsonPBlob(code int, callback string, i any) (err error) {
453453

454454
func (c *Context) json(code int, i any, indent string) error {
455455
c.writeContentType(MIMEApplicationJSON)
456-
c.response.WriteHeader(code)
456+
457+
if r, err := UnwrapResponse(c.response); err == nil {
458+
// *echo.Response can delay sending status code until the first Write is called. As serialization can fail, we should delay
459+
// sending the status code to the client until serialization is complete (first Write would be an indication it succeeded)
460+
// Unsuccessful serialization error needs to go through the error handler and get a proper status code there.
461+
r.Status = code
462+
} else {
463+
return fmt.Errorf("json: response does not unwrap to *echo.Response")
464+
}
465+
457466
return c.echo.JSONSerializer.Serialize(c, i, indent)
458467
}
459468

context_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,24 @@ func TestContextRenderTemplate(t *testing.T) {
138138
}
139139
}
140140

141+
func TestContextRenderTemplateError(t *testing.T) {
142+
// we test that when template rendering fails, no response is sent to the client yet, so the global error handler can decide what to do
143+
e := New()
144+
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(userJSON))
145+
rec := httptest.NewRecorder()
146+
c := e.NewContext(req, rec)
147+
148+
tmpl := &Template{
149+
templates: template.Must(template.New("hello").Parse("Hello, {{.}}!")),
150+
}
151+
c.Echo().Renderer = tmpl
152+
err := c.Render(http.StatusOK, "not_existing", "Jon Snow")
153+
154+
assert.EqualError(t, err, `template: no template "not_existing" associated with template "hello"`)
155+
assert.Equal(t, http.StatusOK, rec.Code) // status code must not be sent to the client
156+
assert.Empty(t, rec.Body.String()) // body must not be sent to the client
157+
}
158+
141159
func TestContextRenderErrorsOnNoRenderer(t *testing.T) {
142160
e := New()
143161
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(userJSON))
@@ -222,6 +240,9 @@ func TestContextJSONErrorsOut(t *testing.T) {
222240

223241
err := c.JSON(http.StatusOK, make(chan bool))
224242
assert.EqualError(t, err, "json: unsupported type: chan bool")
243+
244+
assert.Equal(t, http.StatusOK, rec.Code) // status code must not be sent to the client
245+
assert.Empty(t, rec.Body.String()) // body must not be sent to the client
225246
}
226247

227248
func TestContextJSONPretty(t *testing.T) {

response.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func UnwrapResponse(rw http.ResponseWriter) (*Response, error) {
126126
rw = t.Unwrap()
127127
continue
128128
default:
129-
return nil, errors.New("ResponseWriter does not implement 'Unwrap() http.ResponseWriter' interface")
129+
return nil, errors.New("ResponseWriter does not implement 'Unwrap() http.ResponseWriter' interface or unwrap to *echo.Response")
130130
}
131131
}
132132
}

response_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,3 +115,19 @@ func TestResponse_FlushPanics(t *testing.T) {
115115
res.Flush()
116116
})
117117
}
118+
119+
func TestResponse_UnwrapResponse(t *testing.T) {
120+
orgRes := NewResponse(httptest.NewRecorder(), nil)
121+
res, err := UnwrapResponse(orgRes)
122+
123+
assert.NotNil(t, res)
124+
assert.NoError(t, err)
125+
}
126+
127+
func TestResponse_UnwrapResponse_error(t *testing.T) {
128+
rw := new(testResponseWriter)
129+
res, err := UnwrapResponse(rw)
130+
131+
assert.Nil(t, res)
132+
assert.EqualError(t, err, "ResponseWriter does not implement 'Unwrap() http.ResponseWriter' interface or unwrap to *echo.Response")
133+
}

0 commit comments

Comments
 (0)