Skip to content

Commit ad70bdb

Browse files
author
woodsaj
committed
handle responseWriter being written to after closure.
fixes #2 If macaron.Recovery is being used and a panic is encountered, the responseWriter handler will complete without ever writing anything triggering the `defer grw.close()` to be called. However, macaron.Recovery will then continue to use the responseWriter for writing out the 500 error and stack trace. To handle this case we need to track if close() has been called, and if so then we should just write to the raw, uncompressed output stream.
1 parent 0b090e8 commit ad70bdb

File tree

2 files changed

+32
-1
lines changed

2 files changed

+32
-1
lines changed

gzip.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,15 @@ func Gziper() macaron.Handler {
7979
type gzipResponseWriter struct {
8080
w io.WriteCloser
8181
macaron.ResponseWriter
82+
closed bool
8283
}
8384

8485
func (grw *gzipResponseWriter) setup() {
8586
if grw.w != nil {
8687
return
8788
}
8889

89-
if grw.Header().Get(_HEADER_CONTENT_ENCODING) == "gzip" {
90+
if grw.closed || grw.Header().Get(_HEADER_CONTENT_ENCODING) == "gzip" {
9091
// another handler has already set the content-type to gzip,
9192
// so presumably they are going to write compressed data.
9293
// We dont want to compress the data a second time, so lets
@@ -101,6 +102,10 @@ func (grw *gzipResponseWriter) setup() {
101102
}
102103

103104
func (grw *gzipResponseWriter) close() {
105+
grw.closed = true
106+
if grw.w == nil {
107+
return
108+
}
104109
grw.w.Close()
105110
}
106111

gzip_test.go

+26
Original file line numberDiff line numberDiff line change
@@ -136,3 +136,29 @@ func Test_ResponseWriter_Hijack(t *testing.T) {
136136
So(hijackable.Hijacked, ShouldBeTrue)
137137
})
138138
}
139+
140+
func Test_GzipPanic(t *testing.T) {
141+
Convey("Gzip response content", t, func() {
142+
before := false
143+
144+
m := macaron.Classic()
145+
m.Use(Gziper())
146+
m.Use(func(r http.ResponseWriter) {
147+
r.(macaron.ResponseWriter).Before(func(rw macaron.ResponseWriter) {
148+
before = true
149+
})
150+
})
151+
m.Get("/", func(ctx *macaron.Context) { panic("test") })
152+
153+
// Gzip now.
154+
resp := httptest.NewRecorder()
155+
req, _ := http.NewRequest("GET", "/", nil)
156+
req.Header.Set(_HEADER_ACCEPT_ENCODING, "gzip")
157+
m.ServeHTTP(resp, req)
158+
_, ok := resp.HeaderMap[_HEADER_CONTENT_ENCODING]
159+
So(ok, ShouldBeFalse)
160+
161+
So(resp.Body.String(), ShouldContainSubstring, "PANIC: test")
162+
So(before, ShouldBeTrue)
163+
})
164+
}

0 commit comments

Comments
 (0)