Skip to content

Commit bf5d368

Browse files
raphaelraphael
authored
Fix gzip middleware so that headers are written properly. (goadesign#1392)
* Need to buffer gzip content to compute proper content length * Do not write headers prior to compressing so that content type and content encoding headers can be written properly.
1 parent 563a70b commit bf5d368

File tree

7 files changed

+55
-72
lines changed

7 files changed

+55
-72
lines changed

‎encoding.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,7 @@ func (decoder *HTTPDecoder) Decode(v interface{}, body io.Reader, contentType st
126126
// the decoderPool will handle whether or not a pool is actually in use
127127
d := p.Get(body)
128128
defer p.Put(d)
129-
if err := d.Decode(v); err != nil {
130-
return err
131-
}
132-
133-
return nil
129+
return d.Decode(v)
134130
}
135131

136132
// Register sets a specific decoder to be used for the specified content types. If a decoder is

‎encoding/gogoprotobuf/encoding.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,7 @@ func (dec *ProtoDecoder) Decode(v interface{}) error {
5454
}
5555
dec.pBuf.SetBuf(dec.bBuf.Bytes())
5656

57-
if err = dec.pBuf.Unmarshal(msg); err != nil {
58-
return err
59-
}
60-
return nil
57+
return dec.pBuf.Unmarshal(msg)
6158
}
6259

6360
// Reset stores the new reader and resets its bytes.Buffer and proto.Buffer

‎goagen/gen_app/writers.go

+2-8
Original file line numberDiff line numberDiff line change
@@ -312,10 +312,7 @@ func (w *ControllersWriter) WriteInitService(encoders, decoders []*EncoderTempla
312312
"Encoders": encoders,
313313
"Decoders": decoders,
314314
}
315-
if err := w.ExecuteTemplate("service", serviceT, nil, ctx); err != nil {
316-
return err
317-
}
318-
return nil
315+
return w.ExecuteTemplate("service", serviceT, nil, ctx)
319316
}
320317

321318
// Execute writes the handlers GoGenerator
@@ -400,10 +397,7 @@ func (w *MediaTypesWriter) Execute(mt *design.MediaTypeDefinition) error {
400397
if err != nil {
401398
return err
402399
}
403-
if err := w.ExecuteTemplate("mediatype", mediaTypeT, fn, p); err != nil {
404-
return err
405-
}
406-
return nil
400+
return w.ExecuteTemplate("mediatype", mediaTypeT, fn, p)
407401
})
408402
if err != nil {
409403
return err

‎goagen/gen_client/generator.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -608,10 +608,7 @@ func (g *Generator) generateMediaTypes(pkgDir string, funcs template.FuncMap) (e
608608
if err != nil {
609609
return err
610610
}
611-
if err := typeDecodeTmpl.Execute(mtWr.SourceFile, p); err != nil {
612-
return err
613-
}
614-
return nil
611+
return typeDecodeTmpl.Execute(mtWr.SourceFile, p)
615612
})
616613
return err
617614
})

‎middleware/gzip/middleware.go

+25-23
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"context"
77
"io/ioutil"
88
"net/http"
9+
"strconv"
910
"strings"
1011
"sync"
1112

@@ -29,7 +30,7 @@ const (
2930
type gzipResponseWriter struct {
3031
http.ResponseWriter
3132
gzw *gzip.Writer
32-
buf *bytes.Buffer
33+
buf, gzbuf bytes.Buffer
3334
pool *sync.Pool
3435
statusCode int
3536
shouldCompress *bool
@@ -74,11 +75,8 @@ func (grw *gzipResponseWriter) Write(b []byte) (int, error) {
7475
// Retrieve gzip writer from the pool. Reset it to use the ResponseWriter.
7576
// This allows us to re-use an already allocated buffer rather than
7677
// allocating a new buffer for every request.
77-
grw.Header().Set(headerContentEncoding, encodingGzip)
78-
grw.Header().Set(headerVary, headerAcceptEncoding)
79-
8078
gz := grw.pool.Get().(*gzip.Writer)
81-
gz.Reset(grw.ResponseWriter)
79+
gz.Reset(&grw.gzbuf)
8280
grw.gzw = gz
8381

8482
// Write buffer
@@ -94,7 +92,6 @@ func (grw *gzipResponseWriter) Write(b []byte) (int, error) {
9492

9593
func (grw *gzipResponseWriter) WriteHeader(n int) {
9694
grw.statusCode = n
97-
grw.ResponseWriter.WriteHeader(n)
9895
}
9996

10097
type (
@@ -225,9 +222,9 @@ func MinSize(n int) Option {
225222
}
226223
}
227224

228-
// Middleware encodes the response using Gzip encoding and sets all the appropriate
229-
// headers. If the Content-Type is not set, it will be set by calling
230-
// http.DetectContentType on the data being written.
225+
// Middleware encodes the response using Gzip encoding and sets all the
226+
// appropriate headers. If the Content-Type is not set, it will be set by
227+
// calling http.DetectContentType on the data being written.
231228
func Middleware(level int, o ...Option) goa.Middleware {
232229
opts := options{
233230
minSize: 256,
@@ -272,7 +269,6 @@ func Middleware(level int, o ...Option) goa.Middleware {
272269
grw := &gzipResponseWriter{
273270
ResponseWriter: w,
274271
pool: &gzipPool,
275-
buf: bytes.NewBuffer(nil),
276272
statusCode: http.StatusOK,
277273
o: opts,
278274
}
@@ -287,23 +283,29 @@ func Middleware(level int, o ...Option) goa.Middleware {
287283
return
288284
}
289285

290-
// Delete the content length after we know we have been written to.
291-
grw.Header().Del(headerContentLength)
286+
// Check for uncompressed data
292287
if grw.buf.Len() > 0 {
293-
_, err = grw.ResponseWriter.Write(grw.buf.Bytes())
294-
if err != nil {
295-
return err
296-
}
288+
w.Header().Set(headerContentLength, strconv.Itoa(grw.buf.Len()))
289+
w.WriteHeader(grw.statusCode)
290+
_, err = w.Write(grw.buf.Bytes())
291+
return
297292
}
298293

299-
// Flush+recycle gzip writer
300-
if grw.gzw != nil {
301-
err := grw.gzw.Close()
302-
if err != nil {
303-
return err
304-
}
305-
gzipPool.Put(grw.gzw)
294+
// Check for compressed data
295+
if grw.gzbuf.Len() == 0 {
296+
return
306297
}
298+
299+
w.Header().Set(headerContentLength, strconv.Itoa(grw.gzbuf.Len()))
300+
w.Header().Set(headerContentEncoding, encodingGzip)
301+
w.Header().Set(headerVary, headerAcceptEncoding)
302+
w.WriteHeader(grw.statusCode)
303+
if err = grw.gzw.Close(); err != nil {
304+
return
305+
}
306+
_, err = w.Write(grw.gzbuf.Bytes())
307+
gzipPool.Put(grw.gzw)
308+
307309
return
308310
}
309311
}

‎middleware/gzip/middleware_test.go

+24-24
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ var _ = Describe("Gzip", func() {
6767

6868
gzr, err := gzip.NewReader(bytes.NewReader(rw.Body))
6969
Ω(err).ShouldNot(HaveOccurred())
70-
buf := bytes.NewBuffer(nil)
71-
io.Copy(buf, gzr)
70+
var buf bytes.Buffer
71+
_, err = io.Copy(&buf, gzr)
7272
Ω(err).ShouldNot(HaveOccurred())
7373
Ω(buf.String()).Should(Equal("gzip me!"))
7474
})
@@ -89,8 +89,8 @@ var _ = Describe("Gzip", func() {
8989

9090
gzr, err := gzip.NewReader(bytes.NewReader(rw.Body))
9191
Ω(err).ShouldNot(HaveOccurred())
92-
buf := bytes.NewBuffer(nil)
93-
io.Copy(buf, gzr)
92+
var buf bytes.Buffer
93+
_, err = io.Copy(&buf, gzr)
9494
Ω(err).ShouldNot(HaveOccurred())
9595
Ω(buf.String()).Should(Equal("gzip me!"))
9696
})
@@ -111,8 +111,8 @@ var _ = Describe("Gzip", func() {
111111

112112
gzr, err := gzip.NewReader(bytes.NewReader(rw.Body))
113113
Ω(err).ShouldNot(HaveOccurred())
114-
buf := bytes.NewBuffer(nil)
115-
io.Copy(buf, gzr)
114+
var buf bytes.Buffer
115+
_, err = io.Copy(&buf, gzr)
116116
Ω(err).ShouldNot(HaveOccurred())
117117
Ω(buf.String()).Should(Equal("gzip me!"))
118118
})
@@ -134,8 +134,8 @@ var _ = Describe("Gzip", func() {
134134

135135
gzr, err := gzip.NewReader(bytes.NewReader(rw.Body))
136136
Ω(err).ShouldNot(HaveOccurred())
137-
buf := bytes.NewBuffer(nil)
138-
io.Copy(buf, gzr)
137+
var buf bytes.Buffer
138+
_, err = io.Copy(&buf, gzr)
139139
Ω(err).ShouldNot(HaveOccurred())
140140
Ω(buf.String()).Should(Equal("gzip me!"))
141141
})
@@ -162,8 +162,8 @@ var _ = Describe("Gzip", func() {
162162

163163
gzr, err := gzip.NewReader(bytes.NewReader(rw.Body))
164164
Ω(err).ShouldNot(HaveOccurred())
165-
buf := bytes.NewBuffer(nil)
166-
io.Copy(buf, gzr)
165+
var buf bytes.Buffer
166+
_, err = io.Copy(&buf, gzr)
167167
Ω(err).ShouldNot(HaveOccurred())
168168
Ω(buf.String()).Should(Equal(strings.Repeat("gzip me!", 128)))
169169
})
@@ -201,8 +201,8 @@ var _ = Describe("NotGzip", func() {
201201
Ω(resp.Status).Should(Equal(http.StatusOK))
202202
Ω(resp.Header().Get("Content-Encoding")).ShouldNot(Equal("gzip"))
203203

204-
buf := bytes.NewBuffer(nil)
205-
io.Copy(buf, bytes.NewBuffer(rw.Body))
204+
var buf bytes.Buffer
205+
_, err = io.Copy(&buf, bytes.NewBuffer(rw.Body))
206206
Ω(err).ShouldNot(HaveOccurred())
207207
Ω(buf.String()).Should(Equal("gzip data"))
208208
})
@@ -221,8 +221,8 @@ var _ = Describe("NotGzip", func() {
221221
Ω(resp.Status).Should(Equal(http.StatusOK))
222222
Ω(resp.Header().Get("Content-Encoding")).ShouldNot(Equal("gzip"))
223223

224-
buf := bytes.NewBuffer(nil)
225-
io.Copy(buf, bytes.NewBuffer(rw.Body))
224+
var buf bytes.Buffer
225+
_, err = io.Copy(&buf, bytes.NewBuffer(rw.Body))
226226
Ω(err).ShouldNot(HaveOccurred())
227227
Ω(buf.String()).Should(Equal("gzip me!"))
228228
})
@@ -241,8 +241,8 @@ var _ = Describe("NotGzip", func() {
241241
Ω(resp.Status).Should(Equal(http.StatusBadRequest))
242242
Ω(resp.Header().Get("Content-Encoding")).ShouldNot(Equal("gzip"))
243243

244-
buf := bytes.NewBuffer(nil)
245-
io.Copy(buf, bytes.NewBuffer(rw.Body))
244+
var buf bytes.Buffer
245+
_, err = io.Copy(&buf, bytes.NewBuffer(rw.Body))
246246
Ω(err).ShouldNot(HaveOccurred())
247247
Ω(buf.String()).Should(Equal("gzip me!"))
248248
})
@@ -261,8 +261,8 @@ var _ = Describe("NotGzip", func() {
261261
Ω(resp.Status).Should(Equal(http.StatusOK))
262262
Ω(resp.Header().Get("Content-Encoding")).ShouldNot(Equal("gzip"))
263263

264-
buf := bytes.NewBuffer(nil)
265-
io.Copy(buf, bytes.NewBuffer(rw.Body))
264+
var buf bytes.Buffer
265+
_, err = io.Copy(&buf, bytes.NewBuffer(rw.Body))
266266
Ω(err).ShouldNot(HaveOccurred())
267267
Ω(buf.String()).Should(Equal("gzip me!"))
268268
})
@@ -282,8 +282,8 @@ var _ = Describe("NotGzip", func() {
282282
Ω(resp.Status).Should(Equal(http.StatusOK))
283283
Ω(resp.Header().Get("Content-Encoding")).ShouldNot(Equal("gzip"))
284284

285-
buf := bytes.NewBuffer(nil)
286-
io.Copy(buf, bytes.NewBuffer(rw.Body))
285+
var buf bytes.Buffer
286+
_, err = io.Copy(&buf, bytes.NewBuffer(rw.Body))
287287
Ω(err).ShouldNot(HaveOccurred())
288288
Ω(buf.String()).Should(Equal("gzip me!"))
289289
})
@@ -302,8 +302,8 @@ var _ = Describe("NotGzip", func() {
302302
Ω(resp.Status).Should(Equal(http.StatusOK))
303303
Ω(resp.Header().Get("Content-Encoding")).ShouldNot(Equal("gzip"))
304304

305-
buf := bytes.NewBuffer(nil)
306-
io.Copy(buf, bytes.NewBuffer(rw.Body))
305+
var buf bytes.Buffer
306+
_, err = io.Copy(&buf, bytes.NewBuffer(rw.Body))
307307
Ω(err).ShouldNot(HaveOccurred())
308308
Ω(buf.String()).Should(Equal("gzip me!"))
309309
})
@@ -340,8 +340,8 @@ var _ = Describe("NotGzip", func() {
340340
Ω(resp.Status).Should(Equal(http.StatusOK))
341341
Ω(resp.Header().Get("Content-Encoding")).ShouldNot(Equal("gzip"))
342342

343-
buf := bytes.NewBuffer(nil)
344-
io.Copy(buf, bytes.NewBuffer(rw.Body))
343+
var buf bytes.Buffer
344+
_, err = io.Copy(&buf, bytes.NewBuffer(rw.Body))
345345
Ω(err).ShouldNot(HaveOccurred())
346346
Ω(buf.String()).Should(Equal("gzip me!"))
347347
})

‎service.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,7 @@ func (service *Service) ListenAndServeTLS(addr, certFile, keyFile string) error
206206

207207
// Serve accepts incoming HTTP connections on the listener l, invoking the service mux handler for each.
208208
func (service *Service) Serve(l net.Listener) error {
209-
if err := http.Serve(l, service.Mux); err != nil {
210-
return err
211-
}
212-
return nil
209+
return http.Serve(l, service.Mux)
213210
}
214211

215212
// NewController returns a controller for the given resource. This method is mainly intended for

0 commit comments

Comments
 (0)