ResponseWriterWrapper and HTTPInterfaces (#1644)

Signed-off-by: Tw <tw19881113@gmail.com>

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 _ httpserver.HTTPInterfaces = (*gzipResponseWriter)(nil)
-var _ http.Flusher = (*gzipResponseWriter)(nil)
-var _ http.CloseNotifier = (*gzipResponseWriter)(nil)
-var _ http.Hijacker = (*gzipResponseWriter)(nil)

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))
 			}

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 _ httpserver.HTTPInterfaces = (*responseWriterWrapper)(nil)
-var _ http.Flusher = (*responseWriterWrapper)(nil)
-var _ http.CloseNotifier = (*responseWriterWrapper)(nil)
-var _ http.Hijacker = (*responseWriterWrapper)(nil)

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,
+		ResponseWriterWrapper: &ResponseWriterWrapper{ResponseWriter: w},
-		status:         http.StatusOK,
+		status:                http.StatusOK,
-		start:          time.Now(),
+		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 _ HTTPInterfaces = (*ResponseRecorder)(nil)
-var _ http.Flusher = (*ResponseRecorder)(nil)
-var _ http.CloseNotifier = (*ResponseRecorder)(nil)
-var _ http.Hijacker = (*ResponseRecorder)(nil)

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)

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)
+	return w.ResponseWriterWrapper.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}
 }
 
 // Interface guards
-var (
+var _ httpserver.HTTPInterfaces = internalResponseWriter{}
-	_ http.Pusher        = internalResponseWriter{}
-	_ http.Flusher       = internalResponseWriter{}
-	_ http.CloseNotifier = internalResponseWriter{}
-	_ http.Hijacker      = internalResponseWriter{}
-)

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 (
+var _ httpserver.HTTPInterfaces = testResponseRecorder{}
-	_ http.Pusher        = testResponseRecorder{}
-	_ http.Flusher       = testResponseRecorder{}
-	_ http.CloseNotifier = testResponseRecorder{}
-	_ http.Hijacker      = testResponseRecorder{}
-)
 
 func BenchmarkProxy(b *testing.B) {
 	backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {