@marcel91200 / caddy Mirror · read-only / commits / dd9813c6
Fork 0

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS

Mirrored from https://github.com/Slazee/caddy.git

Synced Nov 13, 13:00 UTC

caddyhttp: ensure ResponseWriterWrapper and ResponseRecorder use ReadFrom if the underlying response writer implements it. (#5022)

Doing so allows for splice/sendfile optimizations when available. Fixes #4731 Co-authored-by: flga <flga@users.noreply.github.com> Co-authored-by: Matthew Holt <mholt@users.noreply.github.com>

fleandro committed Sep 7, 2022 at 21:13 UTC dd9813c65be3af8e222bda6e63f5eea9232c6eee
2 files changed +200 -2
modules/caddyhttp/responsewriter.go
+35 -2
@@ -62,6 +62,16 @@ func (rww *ResponseWriterWrapper) Push(target string, opts *http.PushOptions) er
62 return ErrNotImplemented
63 }
64
65 +// ReadFrom implements io.ReaderFrom. It simply calls the underlying
66 +// ResponseWriter's ReadFrom method if there is one, otherwise it defaults
67 +// to io.Copy.
68 +func (rww *ResponseWriterWrapper) ReadFrom(r io.Reader) (n int64, err error) {
69 + if rf, ok := rww.ResponseWriter.(io.ReaderFrom); ok {
70 + return rf.ReadFrom(r)
71 + }
72 + return io.Copy(rww.ResponseWriter, r)
73 +}
74 +
75 // HTTPInterfaces mix all the interfaces that middleware ResponseWriters need to support.
76 type HTTPInterfaces interface {
77 http.ResponseWriter
@@ -188,9 +198,26 @@ func (rr *responseRecorder) Write(data []byte) (int, error) {
198 } else {
199 n, err = rr.buf.Write(data)
200 }
191 - if err == nil {
192 - rr.size += n
201 +
202 + rr.size += n
203 + return n, err
204 +}
205 +
206 +func (rr *responseRecorder) ReadFrom(r io.Reader) (int64, error) {
207 + rr.WriteHeader(http.StatusOK)
208 + var n int64
209 + var err error
210 + if rr.stream {
211 + if rf, ok := rr.ResponseWriter.(io.ReaderFrom); ok {
212 + n, err = rf.ReadFrom(r)
213 + } else {
214 + n, err = io.Copy(rr.ResponseWriter, r)
215 + }
216 + } else {
217 + n, err = rr.buf.ReadFrom(r)
218 }
219 +
220 + rr.size += int(n)
221 return n, err
222 }
223
@@ -251,4 +278,10 @@ type ShouldBufferFunc func(status int, header http.Header) bool
278 var (
279 _ HTTPInterfaces = (*ResponseWriterWrapper)(nil)
280 _ ResponseRecorder = (*responseRecorder)(nil)
281 +
282 + // Implementing ReaderFrom can be such a significant
283 + // optimization that it should probably be required!
284 + // see PR #5022 (25%-50% speedup)
285 + _ io.ReaderFrom = (*ResponseWriterWrapper)(nil)
286 + _ io.ReaderFrom = (*responseRecorder)(nil)
287 )
modules/caddyhttp/responsewriter_test.go new
+165
@@ -0,0 +1,165 @@
1 +package caddyhttp
2 +
3 +import (
4 + "bytes"
5 + "fmt"
6 + "io"
7 + "net/http"
8 + "strings"
9 + "testing"
10 +)
11 +
12 +type responseWriterSpy interface {
13 + http.ResponseWriter
14 + Written() string
15 + CalledReadFrom() bool
16 +}
17 +
18 +var (
19 + _ responseWriterSpy = (*baseRespWriter)(nil)
20 + _ responseWriterSpy = (*readFromRespWriter)(nil)
21 +)
22 +
23 +// a barebones http.ResponseWriter mock
24 +type baseRespWriter []byte
25 +
26 +func (brw *baseRespWriter) Write(d []byte) (int, error) {
27 + *brw = append(*brw, d...)
28 + return len(d), nil
29 +}
30 +func (brw *baseRespWriter) Header() http.Header { return nil }
31 +func (brw *baseRespWriter) WriteHeader(statusCode int) {}
32 +func (brw *baseRespWriter) Written() string { return string(*brw) }
33 +func (brw *baseRespWriter) CalledReadFrom() bool { return false }
34 +
35 +// an http.ResponseWriter mock that supports ReadFrom
36 +type readFromRespWriter struct {
37 + baseRespWriter
38 + called bool
39 +}
40 +
41 +func (rf *readFromRespWriter) ReadFrom(r io.Reader) (int64, error) {
42 + rf.called = true
43 + return io.Copy(&rf.baseRespWriter, r)
44 +}
45 +
46 +func (rf *readFromRespWriter) CalledReadFrom() bool { return rf.called }
47 +
48 +func TestResponseWriterWrapperReadFrom(t *testing.T) {
49 + tests := map[string]struct {
50 + responseWriter responseWriterSpy
51 + wantReadFrom bool
52 + }{
53 + "no ReadFrom": {
54 + responseWriter: &baseRespWriter{},
55 + wantReadFrom: false,
56 + },
57 + "has ReadFrom": {
58 + responseWriter: &readFromRespWriter{},
59 + wantReadFrom: true,
60 + },
61 + }
62 + for name, tt := range tests {
63 + t.Run(name, func(t *testing.T) {
64 + // what we expect middlewares to do:
65 + type myWrapper struct {
66 + *ResponseWriterWrapper
67 + }
68 +
69 + wrapped := myWrapper{
70 + ResponseWriterWrapper: &ResponseWriterWrapper{ResponseWriter: tt.responseWriter},
71 + }
72 +
73 + const srcData = "boo!"
74 + // hides everything but Read, since strings.Reader implements WriteTo it would
75 + // take precedence over our ReadFrom.
76 + src := struct{ io.Reader }{strings.NewReader(srcData)}
77 +
78 + fmt.Println(name)
79 + if _, err := io.Copy(wrapped, src); err != nil {
80 + t.Errorf("Copy() err = %v", err)
81 + }
82 +
83 + if got := tt.responseWriter.Written(); got != srcData {
84 + t.Errorf("data = %q, want %q", got, srcData)
85 + }
86 +
87 + if tt.responseWriter.CalledReadFrom() != tt.wantReadFrom {
88 + if tt.wantReadFrom {
89 + t.Errorf("ReadFrom() should have been called")
90 + } else {
91 + t.Errorf("ReadFrom() should not have been called")
92 + }
93 + }
94 + })
95 + }
96 +}
97 +
98 +func TestResponseRecorderReadFrom(t *testing.T) {
99 + tests := map[string]struct {
100 + responseWriter responseWriterSpy
101 + shouldBuffer bool
102 + wantReadFrom bool
103 + }{
104 + "buffered plain": {
105 + responseWriter: &baseRespWriter{},
106 + shouldBuffer: true,
107 + wantReadFrom: false,
108 + },
109 + "streamed plain": {
110 + responseWriter: &baseRespWriter{},
111 + shouldBuffer: false,
112 + wantReadFrom: false,
113 + },
114 + "buffered ReadFrom": {
115 + responseWriter: &readFromRespWriter{},
116 + shouldBuffer: true,
117 + wantReadFrom: false,
118 + },
119 + "streamed ReadFrom": {
120 + responseWriter: &readFromRespWriter{},
121 + shouldBuffer: false,
122 + wantReadFrom: true,
123 + },
124 + }
125 + for name, tt := range tests {
126 + t.Run(name, func(t *testing.T) {
127 + var buf bytes.Buffer
128 +
129 + rr := NewResponseRecorder(tt.responseWriter, &buf, func(status int, header http.Header) bool {
130 + return tt.shouldBuffer
131 + })
132 +
133 + const srcData = "boo!"
134 + // hides everything but Read, since strings.Reader implements WriteTo it would
135 + // take precedence over our ReadFrom.
136 + src := struct{ io.Reader }{strings.NewReader(srcData)}
137 +
138 + if _, err := io.Copy(rr, src); err != nil {
139 + t.Errorf("Copy() err = %v", err)
140 + }
141 +
142 + wantStreamed := srcData
143 + wantBuffered := ""
144 + if tt.shouldBuffer {
145 + wantStreamed = ""
146 + wantBuffered = srcData
147 + }
148 +
149 + if got := tt.responseWriter.Written(); got != wantStreamed {
150 + t.Errorf("streamed data = %q, want %q", got, wantStreamed)
151 + }
152 + if got := buf.String(); got != wantBuffered {
153 + t.Errorf("buffered data = %q, want %q", got, wantBuffered)
154 + }
155 +
156 + if tt.responseWriter.CalledReadFrom() != tt.wantReadFrom {
157 + if tt.wantReadFrom {
158 + t.Errorf("ReadFrom() should have been called")
159 + } else {
160 + t.Errorf("ReadFrom() should not have been called")
161 + }
162 + }
163 + })
164 + }
165 +}