From 8e7a36de456f6d0ada7210acc63a71a1a008ea48 Mon Sep 17 00:00:00 2001 From: Tw Date: Fri, 5 May 2017 10:42:06 -0500 Subject: [PATCH] ResponseWriterWrapper and HTTPInterfaces (#1644) Signed-off-by: Tw --- caddyhttp/gzip/gzip.go | 52 ++------------- caddyhttp/gzip/responsefilter_test.go | 2 +- caddyhttp/header/header.go | 56 +++------------- caddyhttp/httpserver/recorder.go | 56 ++-------------- caddyhttp/httpserver/responsewriterwrapper.go | 65 +++++++++++++++++++ caddyhttp/internalsrv/internal.go | 57 ++-------------- caddyhttp/proxy/proxy_test.go | 19 ++---- 7 files changed, 98 insertions(+), 209 deletions(-) create mode 100644 caddyhttp/httpserver/responsewriterwrapper.go diff --git a/caddyhttp/gzip/gzip.go b/caddyhttp/gzip/gzip.go index 48172c72..110b704a 100644 --- a/caddyhttp/gzip/gzip.go +++ b/caddyhttp/gzip/gzip.go @@ -3,9 +3,7 @@ package gzip import ( - "bufio" "io" - "net" "net/http" "strings" @@ -58,7 +56,10 @@ outer: // original form. gzipWriter := getWriter(c.Level) defer putWriter(c.Level, gzipWriter) - gz := &gzipResponseWriter{Writer: gzipWriter, ResponseWriter: w} + gz := &gzipResponseWriter{ + Writer: gzipWriter, + ResponseWriterWrapper: &httpserver.ResponseWriterWrapper{ResponseWriter: w}, + } var rw http.ResponseWriter // if no response filter is used @@ -92,7 +93,7 @@ outer: // with a gzip.Writer to compress the output. type gzipResponseWriter struct { io.Writer - http.ResponseWriter + *httpserver.ResponseWriterWrapper statusCodeWritten bool } @@ -104,7 +105,7 @@ func (w *gzipResponseWriter) WriteHeader(code int) { w.Header().Del("Content-Length") w.Header().Set("Content-Encoding", "gzip") w.Header().Add("Vary", "Accept-Encoding") - w.ResponseWriter.WriteHeader(code) + w.ResponseWriterWrapper.WriteHeader(code) w.statusCodeWritten = true } @@ -120,44 +121,5 @@ func (w *gzipResponseWriter) Write(b []byte) (int, error) { return n, err } -// Hijack implements http.Hijacker. It simply wraps the underlying -// ResponseWriter's Hijack method if there is one, or returns an error. -func (w *gzipResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { - if hj, ok := w.ResponseWriter.(http.Hijacker); ok { - return hj.Hijack() - } - return nil, nil, httpserver.NonHijackerError{Underlying: w.ResponseWriter} -} - -// Flush implements http.Flusher. It simply wraps the underlying -// ResponseWriter's Flush method if there is one, or panics. -func (w *gzipResponseWriter) Flush() { - if f, ok := w.ResponseWriter.(http.Flusher); ok { - f.Flush() - } else { - panic(httpserver.NonFlusherError{Underlying: w.ResponseWriter}) // should be recovered at the beginning of middleware stack - } -} - -// CloseNotify implements http.CloseNotifier. -// It just inherits the underlying ResponseWriter's CloseNotify method. -func (w *gzipResponseWriter) CloseNotify() <-chan bool { - if cn, ok := w.ResponseWriter.(http.CloseNotifier); ok { - return cn.CloseNotify() - } - panic(httpserver.NonCloseNotifierError{Underlying: w.ResponseWriter}) -} - -func (w *gzipResponseWriter) Push(target string, opts *http.PushOptions) error { - if pusher, hasPusher := w.ResponseWriter.(http.Pusher); hasPusher { - return pusher.Push(target, opts) - } - - return httpserver.NonFlusherError{Underlying: w.ResponseWriter} -} - // Interface guards -var _ http.Pusher = (*gzipResponseWriter)(nil) -var _ http.Flusher = (*gzipResponseWriter)(nil) -var _ http.CloseNotifier = (*gzipResponseWriter)(nil) -var _ http.Hijacker = (*gzipResponseWriter)(nil) +var _ httpserver.HTTPInterfaces = (*gzipResponseWriter)(nil) diff --git a/caddyhttp/gzip/responsefilter_test.go b/caddyhttp/gzip/responsefilter_test.go index 206826f2..43a51bd1 100644 --- a/caddyhttp/gzip/responsefilter_test.go +++ b/caddyhttp/gzip/responsefilter_test.go @@ -33,7 +33,7 @@ func TestLengthFilter(t *testing.T) { for j, filter := range filters { r := httptest.NewRecorder() r.Header().Set("Content-Length", fmt.Sprint(ts.length)) - wWriter := NewResponseFilterWriter([]ResponseFilter{filter}, &gzipResponseWriter{gzip.NewWriter(r), r, false}) + wWriter := NewResponseFilterWriter([]ResponseFilter{filter}, &gzipResponseWriter{gzip.NewWriter(r), &httpserver.ResponseWriterWrapper{ResponseWriter: r}, false}) if filter.ShouldCompress(wWriter) != ts.shouldCompress[j] { t.Errorf("Test %v: Expected %v found %v", i, ts.shouldCompress[j], filter.ShouldCompress(r)) } diff --git a/caddyhttp/header/header.go b/caddyhttp/header/header.go index a2685fbb..3967dd38 100644 --- a/caddyhttp/header/header.go +++ b/caddyhttp/header/header.go @@ -4,8 +4,6 @@ package header import ( - "bufio" - "net" "net/http" "strings" @@ -23,7 +21,9 @@ type Headers struct { // setting headers on the response according to the configured rules. func (h Headers) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { replacer := httpserver.NewReplacer(r, nil, "") - rww := &responseWriterWrapper{ResponseWriter: w} + rww := &responseWriterWrapper{ + ResponseWriterWrapper: &httpserver.ResponseWriterWrapper{ResponseWriter: w}, + } for _, rule := range h.Rules { if httpserver.Path(r.URL.Path).Matches(rule.Path) { for name := range rule.Headers { @@ -62,20 +62,20 @@ type headerOperation func(http.Header) // responseWriterWrapper wraps the real ResponseWriter. // It defers header operations until writeHeader type responseWriterWrapper struct { - http.ResponseWriter + *httpserver.ResponseWriterWrapper ops []headerOperation wroteHeader bool } func (rww *responseWriterWrapper) Header() http.Header { - return rww.ResponseWriter.Header() + return rww.ResponseWriterWrapper.Header() } func (rww *responseWriterWrapper) Write(d []byte) (int, error) { if !rww.wroteHeader { rww.WriteHeader(http.StatusOK) } - return rww.ResponseWriter.Write(d) + return rww.ResponseWriterWrapper.Write(d) } func (rww *responseWriterWrapper) WriteHeader(status int) { @@ -91,7 +91,7 @@ func (rww *responseWriterWrapper) WriteHeader(status int) { op(h) } - rww.ResponseWriter.WriteHeader(status) + rww.ResponseWriterWrapper.WriteHeader(status) } // delHeader deletes the existing header according to the key @@ -106,45 +106,5 @@ func (rww *responseWriterWrapper) delHeader(key string) { }) } -// Hijack implements http.Hijacker. It simply wraps the underlying -// ResponseWriter's Hijack method if there is one, or returns an error. -func (rww *responseWriterWrapper) Hijack() (net.Conn, *bufio.ReadWriter, error) { - if hj, ok := rww.ResponseWriter.(http.Hijacker); ok { - return hj.Hijack() - } - return nil, nil, httpserver.NonHijackerError{Underlying: rww.ResponseWriter} -} - -// Flush implements http.Flusher. It simply wraps the underlying -// ResponseWriter's Flush method if there is one, or panics. -func (rww *responseWriterWrapper) Flush() { - if f, ok := rww.ResponseWriter.(http.Flusher); ok { - f.Flush() - } else { - panic(httpserver.NonFlusherError{Underlying: rww.ResponseWriter}) // should be recovered at the beginning of middleware stack - } -} - -// CloseNotify implements http.CloseNotifier. -// It just inherits the underlying ResponseWriter's CloseNotify method. -// It panics if the underlying ResponseWriter is not a CloseNotifier. -func (rww *responseWriterWrapper) CloseNotify() <-chan bool { - if cn, ok := rww.ResponseWriter.(http.CloseNotifier); ok { - return cn.CloseNotify() - } - panic(httpserver.NonCloseNotifierError{Underlying: rww.ResponseWriter}) -} - -func (rww *responseWriterWrapper) Push(target string, opts *http.PushOptions) error { - if pusher, hasPusher := rww.ResponseWriter.(http.Pusher); hasPusher { - return pusher.Push(target, opts) - } - - return httpserver.NonPusherError{Underlying: rww.ResponseWriter} -} - // Interface guards -var _ http.Pusher = (*responseWriterWrapper)(nil) -var _ http.Flusher = (*responseWriterWrapper)(nil) -var _ http.CloseNotifier = (*responseWriterWrapper)(nil) -var _ http.Hijacker = (*responseWriterWrapper)(nil) +var _ httpserver.HTTPInterfaces = (*responseWriterWrapper)(nil) diff --git a/caddyhttp/httpserver/recorder.go b/caddyhttp/httpserver/recorder.go index 2bb919fc..da89056c 100644 --- a/caddyhttp/httpserver/recorder.go +++ b/caddyhttp/httpserver/recorder.go @@ -1,8 +1,6 @@ package httpserver import ( - "bufio" - "net" "net/http" "time" ) @@ -20,7 +18,7 @@ import ( // // Beware when accessing the Replacer value; it may be nil! type ResponseRecorder struct { - http.ResponseWriter + *ResponseWriterWrapper Replacer Replacer status int size int @@ -35,9 +33,9 @@ type ResponseRecorder struct { // of 200 to cover the default case. func NewResponseRecorder(w http.ResponseWriter) *ResponseRecorder { return &ResponseRecorder{ - ResponseWriter: w, - status: http.StatusOK, - start: time.Now(), + ResponseWriterWrapper: &ResponseWriterWrapper{ResponseWriter: w}, + status: http.StatusOK, + start: time.Now(), } } @@ -45,13 +43,13 @@ func NewResponseRecorder(w http.ResponseWriter) *ResponseRecorder { // underlying ResponseWriter's WriteHeader method. func (r *ResponseRecorder) WriteHeader(status int) { r.status = status - r.ResponseWriter.WriteHeader(status) + r.ResponseWriterWrapper.WriteHeader(status) } // Write is a wrapper that records the size of the body // that gets written. func (r *ResponseRecorder) Write(buf []byte) (int, error) { - n, err := r.ResponseWriter.Write(buf) + n, err := r.ResponseWriterWrapper.Write(buf) if err == nil { r.size += n } @@ -68,45 +66,5 @@ func (r *ResponseRecorder) Status() int { return r.status } -// Hijack implements http.Hijacker. It simply wraps the underlying -// ResponseWriter's Hijack method if there is one, or returns an error. -func (r *ResponseRecorder) Hijack() (net.Conn, *bufio.ReadWriter, error) { - if hj, ok := r.ResponseWriter.(http.Hijacker); ok { - return hj.Hijack() - } - return nil, nil, NonHijackerError{Underlying: r.ResponseWriter} -} - -// Flush implements http.Flusher. It simply wraps the underlying -// ResponseWriter's Flush method if there is one, or does nothing. -func (r *ResponseRecorder) Flush() { - if f, ok := r.ResponseWriter.(http.Flusher); ok { - f.Flush() - } else { - panic(NonFlusherError{Underlying: r.ResponseWriter}) // should be recovered at the beginning of middleware stack - } -} - -// CloseNotify implements http.CloseNotifier. -// It just inherits the underlying ResponseWriter's CloseNotify method. -func (r *ResponseRecorder) CloseNotify() <-chan bool { - if cn, ok := r.ResponseWriter.(http.CloseNotifier); ok { - return cn.CloseNotify() - } - panic(NonCloseNotifierError{Underlying: r.ResponseWriter}) -} - -// Push resource to client -func (r *ResponseRecorder) Push(target string, opts *http.PushOptions) error { - if pusher, hasPusher := r.ResponseWriter.(http.Pusher); hasPusher { - return pusher.Push(target, opts) - } - - return NonPusherError{Underlying: r.ResponseWriter} -} - // Interface guards -var _ http.Pusher = (*ResponseRecorder)(nil) -var _ http.Flusher = (*ResponseRecorder)(nil) -var _ http.CloseNotifier = (*ResponseRecorder)(nil) -var _ http.Hijacker = (*ResponseRecorder)(nil) +var _ HTTPInterfaces = (*ResponseRecorder)(nil) diff --git a/caddyhttp/httpserver/responsewriterwrapper.go b/caddyhttp/httpserver/responsewriterwrapper.go new file mode 100644 index 00000000..350d0e6c --- /dev/null +++ b/caddyhttp/httpserver/responsewriterwrapper.go @@ -0,0 +1,65 @@ +package httpserver + +import ( + "bufio" + "net" + "net/http" +) + +// ResponseWriterWrapper wrappers underlying ResponseWriter +// and inherits its Hijacker/Pusher/CloseNotifier/Flusher as well. +type ResponseWriterWrapper struct { + http.ResponseWriter +} + +// Hijack implements http.Hijacker. It simply wraps the underlying +// ResponseWriter's Hijack method if there is one, or returns an error. +func (rww *ResponseWriterWrapper) Hijack() (net.Conn, *bufio.ReadWriter, error) { + if hj, ok := rww.ResponseWriter.(http.Hijacker); ok { + return hj.Hijack() + } + return nil, nil, NonHijackerError{Underlying: rww.ResponseWriter} +} + +// Flush implements http.Flusher. It simply wraps the underlying +// ResponseWriter's Flush method if there is one, or panics. +func (rww *ResponseWriterWrapper) Flush() { + if f, ok := rww.ResponseWriter.(http.Flusher); ok { + f.Flush() + } else { + panic(NonFlusherError{Underlying: rww.ResponseWriter}) + } +} + +// CloseNotify implements http.CloseNotifier. +// It just inherits the underlying ResponseWriter's CloseNotify method. +// It panics if the underlying ResponseWriter is not a CloseNotifier. +func (rww *ResponseWriterWrapper) CloseNotify() <-chan bool { + if cn, ok := rww.ResponseWriter.(http.CloseNotifier); ok { + return cn.CloseNotify() + } + panic(NonCloseNotifierError{Underlying: rww.ResponseWriter}) +} + +// Push implements http.Pusher. +// It just inherits the underlying ResponseWriter's Push method. +// It panics if the underlying ResponseWriter is not a Pusher. +func (rww *ResponseWriterWrapper) Push(target string, opts *http.PushOptions) error { + if pusher, hasPusher := rww.ResponseWriter.(http.Pusher); hasPusher { + return pusher.Push(target, opts) + } + + return NonPusherError{Underlying: rww.ResponseWriter} +} + +// HTTPInterfaces mix all the interfaces that middleware ResponseWriters need to support. +type HTTPInterfaces interface { + http.ResponseWriter + http.Pusher + http.Flusher + http.CloseNotifier + http.Hijacker +} + +// Interface guards +var _ HTTPInterfaces = (*ResponseWriterWrapper)(nil) diff --git a/caddyhttp/internalsrv/internal.go b/caddyhttp/internalsrv/internal.go index d811ff3b..c228aa70 100644 --- a/caddyhttp/internalsrv/internal.go +++ b/caddyhttp/internalsrv/internal.go @@ -7,8 +7,6 @@ package internalsrv import ( - "bufio" - "net" "net/http" "github.com/mholt/caddy/caddyhttp/httpserver" @@ -44,7 +42,7 @@ func (i Internal) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) // Use internal response writer to ignore responses that will be // redirected to internal locations - iw := internalResponseWriter{ResponseWriter: w} + iw := internalResponseWriter{ResponseWriterWrapper: &httpserver.ResponseWriterWrapper{ResponseWriter: w}} status, err := i.Next.ServeHTTP(iw, r) for c := 0; c < maxRedirectCount && isInternalRedirect(iw); c++ { @@ -69,7 +67,7 @@ func (i Internal) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) // calls to Write and WriteHeader if the response should be redirected to an // internal location. type internalResponseWriter struct { - http.ResponseWriter + *httpserver.ResponseWriterWrapper } // ClearHeader removes script headers that would interfere with follow up @@ -84,7 +82,7 @@ func (w internalResponseWriter) ClearHeader() { // internal location. func (w internalResponseWriter) WriteHeader(code int) { if !isInternalRedirect(w) { - w.ResponseWriter.WriteHeader(code) + w.ResponseWriterWrapper.WriteHeader(code) } } @@ -94,53 +92,8 @@ func (w internalResponseWriter) Write(b []byte) (int, error) { if isInternalRedirect(w) { return 0, nil } - return w.ResponseWriter.Write(b) -} - -// Hijack implements http.Hijacker. It simply wraps the underlying -// ResponseWriter's Hijack method if there is one, or returns an error. -func (w internalResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { - if hj, ok := w.ResponseWriter.(http.Hijacker); ok { - return hj.Hijack() - } - return nil, nil, httpserver.NonHijackerError{Underlying: w.ResponseWriter} -} - -// Flush implements http.Flusher. It simply wraps the underlying -// ResponseWriter's Flush method if there is one, or panics. -func (w internalResponseWriter) Flush() { - if f, ok := w.ResponseWriter.(http.Flusher); ok { - f.Flush() - } else { - panic(httpserver.NonFlusherError{Underlying: w.ResponseWriter}) - } -} - -// CloseNotify implements http.CloseNotifier. -// It just inherits the underlying ResponseWriter's CloseNotify method. -// It panics if the underlying ResponseWriter is not a CloseNotifier. -func (w internalResponseWriter) CloseNotify() <-chan bool { - if cn, ok := w.ResponseWriter.(http.CloseNotifier); ok { - return cn.CloseNotify() - } - panic(httpserver.NonCloseNotifierError{Underlying: w.ResponseWriter}) -} - -// Push implements http.Pusher. -// It just inherits the underlying ResponseWriter's Push method. -// It panics if the underlying ResponseWriter is not a Pusher. -func (w internalResponseWriter) Push(target string, opts *http.PushOptions) error { - if pusher, hasPusher := w.ResponseWriter.(http.Pusher); hasPusher { - return pusher.Push(target, opts) - } - - return httpserver.NonPusherError{Underlying: w.ResponseWriter} + return w.ResponseWriterWrapper.Write(b) } // Interface guards -var ( - _ http.Pusher = internalResponseWriter{} - _ http.Flusher = internalResponseWriter{} - _ http.CloseNotifier = internalResponseWriter{} - _ http.Hijacker = internalResponseWriter{} -) +var _ httpserver.HTTPInterfaces = internalResponseWriter{} diff --git a/caddyhttp/proxy/proxy_test.go b/caddyhttp/proxy/proxy_test.go index 60805379..ca207a45 100644 --- a/caddyhttp/proxy/proxy_test.go +++ b/caddyhttp/proxy/proxy_test.go @@ -100,7 +100,9 @@ func TestReverseProxy(t *testing.T) { // Make sure {upstream} placeholder is set r.Body = ioutil.NopCloser(strings.NewReader("test")) - rr := httpserver.NewResponseRecorder(testResponseRecorder{httptest.NewRecorder()}) + rr := httpserver.NewResponseRecorder(testResponseRecorder{ + ResponseWriterWrapper: &httpserver.ResponseWriterWrapper{ResponseWriter: httptest.NewRecorder()}, + }) rr.Replacer = httpserver.NewReplacer(r, rr, "-") p.ServeHTTP(rr, r) @@ -1315,24 +1317,13 @@ func (c *fakeConn) Write(b []byte) (int, error) { return c.writeBuf.Write // testResponseRecorder wraps `httptest.ResponseRecorder`, // also implements `http.CloseNotifier`, `http.Hijacker` and `http.Pusher`. type testResponseRecorder struct { - *httptest.ResponseRecorder + *httpserver.ResponseWriterWrapper } func (testResponseRecorder) CloseNotify() <-chan bool { return nil } -func (t testResponseRecorder) Hijack() (net.Conn, *bufio.ReadWriter, error) { - return nil, nil, httpserver.NonHijackerError{Underlying: t} -} -func (t testResponseRecorder) Push(target string, opts *http.PushOptions) error { - return httpserver.NonPusherError{Underlying: t} -} // Interface guards -var ( - _ http.Pusher = testResponseRecorder{} - _ http.Flusher = testResponseRecorder{} - _ http.CloseNotifier = testResponseRecorder{} - _ http.Hijacker = testResponseRecorder{} -) +var _ httpserver.HTTPInterfaces = testResponseRecorder{} func BenchmarkProxy(b *testing.B) { backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {