Skip to content

Commit bb72c6d

Browse files
tylerjltylerjl
committed
Attempt a more direct approach for sendfile patch
1 parent 924970d commit bb72c6d

File tree

4 files changed

+274
-12
lines changed

4 files changed

+274
-12
lines changed

‎default.nix

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
(import
2+
(
3+
let lock = builtins.fromJSON (builtins.readFile ./flake.lock); in
4+
fetchTarball {
5+
url = "https://github.com/edolstra/flake-compat/archive/${lock.nodes.flake-compat.locked.rev}.tar.gz";
6+
sha256 = lock.nodes.flake-compat.locked.narHash;
7+
}
8+
)
9+
{ src = ./.; }
10+
).defaultNix

‎flake.lock

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎flake.nix

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
devshell.url = "github:numtide/devshell";
44
nixos-shell.url = "github:Mic92/nixos-shell";
55
nixpkgs.url = "github:NixOS/nixpkgs/nixos-22.05";
6+
flake-compat = {
7+
url = "github:edolstra/flake-compat";
8+
flake = false;
9+
};
610
flake-utils.url = "github:numtide/flake-utils";
711
};
812

@@ -14,6 +18,7 @@
1418
overlays = [
1519
inputs.devshell.overlay
1620
self.overlays.default
21+
self.overlays.caddy
1722
];
1823
};
1924
in with pkgs; rec {
@@ -44,29 +49,27 @@
4449
wrk2
4550
];
4651
};
47-
packages.static-html = pkgs.buildEnv {
48-
name = "static-html";
49-
paths = [ ./static ];
52+
packages = {
53+
static-html = pkgs.buildEnv {
54+
name = "static-html";
55+
paths = [ ./static ];
56+
};
5057
};
5158
})) // (let
5259
system = "x86_64-linux";
5360
pkgs = import nixpkgs {
5461
inherit system;
5562
overlays = [
5663
self.overlays.default
57-
self.overlays.caddyMaster
64+
self.overlays.caddy
5865
];
5966
};
6067
in {
61-
overlays.caddyMaster = prev: final: {
68+
overlays.caddy = prev: final: {
6269
caddyMaster = final.caddy.overrideAttrs (old: {
63-
version = "master";
64-
src = prev.fetchFromGitHub {
65-
owner = "caddyserver";
66-
repo = "caddy";
67-
rev = "dd9813c65be3af8e222bda6e63f5eea9232c6eee";
68-
sha256 = "sha256-Z9A2DRdX0LWjIKdHAHk2IRxsUzvC90Gf5ohFLXNHcsw=";
69-
};
70+
patches = [
71+
./sendfile.patch
72+
];
7073
});
7174
};
7275
overlays.default = prev: final: {

‎sendfile.patch

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

0 commit comments

Comments
 (0)