@marcel91200 / caddy Mirror · read-only / commits / 72dd0fb0
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 11, 04:45 UTC

timeouts: fix idle writer terminate h2 response writers between writes (#8119)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Francis Lavoie <lavofr@gmail.com>

WeidiDeng committed Oct 3, 2026 at 13:06 UTC 72dd0fb067f6d7826c7f79907670ba4a713bfe37
4 files changed +292 -13
modules/caddyhttp/idletimeout.go
+89 -7
@@ -74,6 +74,9 @@ func (d *IdleDeadline) next() (deadline time.Time) {
74 // IdleTimeoutReader wraps a request body with IdleDeadline, resetting
75 // the read deadline before every Read call instead of bounding the
76 // whole body transfer with a single hard deadline.
77 +// The deadline is cleared after an error (most likely io.EOF) is encountered when reading the body
78 +// to prevent context canceled error for h1 requests.
79 +// see: https://github.com/caddyserver/caddy/issues/8103
80 type IdleTimeoutReader struct {
81 io.ReadCloser
82 Ctrl *http.ResponseController
@@ -166,10 +169,17 @@ func (r *IdleTimeoutReader) clearDeadlineLocked() {
169 }
170
171 // IdleTimeoutWriter wraps a ResponseWriter with IdleDeadline, resetting
169 -// the write deadline before every Write call, the same way
170 -// IdleTimeoutReader does for reads. A handler that pauses between
171 -// writes (e.g. streaming or SSE) is unaffected, since with MinRate == 0
172 -// the deadline only bounds the duration of the write actually in flight.
172 +// the write deadline before every Write, ReadFrom and Flush call, the same
173 +// way IdleTimeoutReader does for reads. With MinRate == 0, the deadline
174 +// only bounds the write or flush actually in flight, so a handler that
175 +// pauses between writes (e.g. streaming or SSE) is unaffected.
176 +//
177 +// That relies on a deadline left armed between writes being harmless,
178 +// which holds for HTTP/1 and HTTP/3, where it is only checked by the
179 +// next write on the connection or stream. On HTTP/2 the write deadline
180 +// is a timer that resets the stream when it fires, even with no write
181 +// in flight, so ClearBetweenWrites must be set for HTTP/2 requests
182 +// (see #8118).
183 //
184 // MaxChunk bounds how much a single underlying Write/ReadFrom call is
185 // allowed to cover; zero uses DefaultMaxWriteChunk. SetWriteDeadline
@@ -191,7 +201,16 @@ type IdleTimeoutWriter struct {
201 MaxChunk int
202 Logger *zap.Logger
203
204 + // ClearBetweenWrites makes the writer clear its idle deadline after
205 + // every successful write or flush, putting back HardDeadline if any,
206 + // and arm a final one in HandlerDone if written data may still be
207 + // buffered. It is required for HTTP/2 and unnecessary (and costs a
208 + // deadline update per write) for other protocols.
209 + ClearBetweenWrites bool
210 +
211 unsupported bool
212 + deadlineSet bool
213 + unflushed bool
214 }
215
216 func (w *IdleTimeoutWriter) resetDeadline() {
@@ -204,6 +223,43 @@ func (w *IdleTimeoutWriter) resetDeadline() {
223 if c := w.Logger.Check(zapcore.DebugLevel, "could not set write deadline"); c != nil {
224 c.Write(zap.Error(err))
225 }
226 + return
227 + }
228 + w.deadlineSet = true
229 +}
230 +
231 +// clearDeadline replaces the idle deadline set by resetDeadline with
232 +// HardDeadline, which is zero (no deadline) unless a hard ceiling is
233 +// configured. It does nothing unless ClearBetweenWrites is set.
234 +func (w *IdleTimeoutWriter) clearDeadline() {
235 + if !w.ClearBetweenWrites || !w.deadlineSet || w.unsupported {
236 + return
237 + }
238 +
239 + // once the hard deadline has passed, the deadline set by
240 + // resetDeadline (capped to it) has already expired; keep it
241 + if !w.Deadline.HardDeadline.IsZero() && !time.Now().Before(w.Deadline.HardDeadline) {
242 + return
243 + }
244 +
245 + if err := w.Ctrl.SetWriteDeadline(w.Deadline.HardDeadline); err != nil {
246 + w.unsupported = true
247 + if c := w.Logger.Check(zapcore.DebugLevel, "could not clear write deadline"); c != nil {
248 + c.Write(zap.Error(err))
249 + }
250 + return
251 + }
252 + w.deadlineSet = false
253 +}
254 +
255 +// HandlerDone arms a final idle deadline if ClearBetweenWrites is set
256 +// and data written since the last flush may still be buffered, so that
257 +// the flush net/http does after the handler returns is bounded too. It
258 +// must be called before the installing handler returns, since the
259 +// response controller can't be used after that.
260 +func (w *IdleTimeoutWriter) HandlerDone() {
261 + if w.ClearBetweenWrites && w.unflushed && !w.deadlineSet {
262 + w.resetDeadline()
263 }
264 }
265
@@ -227,10 +283,14 @@ func (w *IdleTimeoutWriter) Write(p []byte) (int, error) {
283 total += n
284 w.Deadline.transferred += int64(n)
285 p = p[n:]
286 + if n > 0 {
287 + w.unflushed = true
288 + }
289 if err != nil {
290 return total, err
291 }
292 }
293 + w.clearDeadline()
294 return total, nil
295 }
296
@@ -242,18 +302,40 @@ func (w *IdleTimeoutWriter) ReadFrom(r io.Reader) (int64, error) {
302 n, err := w.ResponseWriterWrapper.ReadFrom(io.LimitReader(r, int64(maxChunk)))
303 total += n
304 w.Deadline.transferred += n
305 + if n > 0 {
306 + w.unflushed = true
307 + }
308 if err != nil {
309 + // the error may come from r rather than the client, in
310 + // which case the response can still be written to
311 + w.clearDeadline()
312 return total, err
313 }
314 if n < int64(maxChunk) {
315 + w.clearDeadline()
316 return total, nil
317 }
318 }
319 }
320
321 +// FlushError flushes the underlying writer under the idle deadline. A
322 +// small write may only fill a buffer, leaving the flush as the call
323 +// that actually blocks on a client that stopped reading.
324 +func (w *IdleTimeoutWriter) FlushError() error {
325 + w.resetDeadline()
326 + err := http.NewResponseController(w.ResponseWriter).Flush()
327 + if err != nil {
328 + return err
329 + }
330 + w.unflushed = false
331 + w.clearDeadline()
332 + return nil
333 +}
334 +
335 // Interface guards
336 var (
256 - _ io.ReadCloser = (*IdleTimeoutReader)(nil)
257 - _ http.ResponseWriter = (*IdleTimeoutWriter)(nil)
258 - _ io.ReaderFrom = (*IdleTimeoutWriter)(nil)
337 + _ io.ReadCloser = (*IdleTimeoutReader)(nil)
338 + _ http.ResponseWriter = (*IdleTimeoutWriter)(nil)
339 + _ io.ReaderFrom = (*IdleTimeoutWriter)(nil)
340 + _ interface{ FlushError() error } = (*IdleTimeoutWriter)(nil)
341 )
modules/caddyhttp/idletimeout_test.go
+191
@@ -15,6 +15,7 @@
15 package caddyhttp
16
17 import (
18 + "bufio"
19 "bytes"
20 "errors"
21 "fmt"
@@ -571,6 +572,196 @@ func TestIdleTimeoutWriter_HardDeadlineCapsIdleReset(t *testing.T) {
572 }
573 }
574
575 +// TestIdleTimeoutWriter_Streaming covers #8118: on HTTP/2 the write
576 +// deadline is a timer that resets the stream when it fires, so a
577 +// deadline left armed between writes kills a stream that merely pauses.
578 +func TestIdleTimeoutWriter_Streaming(t *testing.T) {
579 + const idle = 100 * time.Millisecond
580 + const interval = 250 * time.Millisecond
581 + const count = 3
582 +
583 + srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
584 + wrapped := &IdleTimeoutWriter{
585 + ResponseWriterWrapper: &ResponseWriterWrapper{ResponseWriter: w},
586 + Ctrl: http.NewResponseController(w),
587 + Deadline: IdleDeadline{Timeout: idle},
588 + Logger: zap.NewNop(),
589 + ClearBetweenWrites: r.ProtoMajor == 2,
590 + }
591 + defer wrapped.HandlerDone()
592 + wrapped.Header().Set("Content-Type", "text/event-stream")
593 + rc := http.NewResponseController(wrapped)
594 +
595 + for i := range count {
596 + time.Sleep(interval)
597 + if _, err := fmt.Fprintf(wrapped, "data: %d\n\n", i); err != nil {
598 + t.Logf("write error: %v", err)
599 + return
600 + }
601 + if err := rc.Flush(); err != nil {
602 + t.Logf("flush error: %v", err)
603 + return
604 + }
605 + }
606 + }))
607 + srv.EnableHTTP2 = true
608 + srv.StartTLS()
609 + defer srv.Close()
610 +
611 + resp, err := srv.Client().Get(srv.URL)
612 + require.NoError(t, err)
613 + defer resp.Body.Close()
614 + require.Equal(t, 2, resp.ProtoMajor)
615 +
616 + scanner := bufio.NewScanner(resp.Body)
617 + var linesRead int
618 + for scanner.Scan() {
619 + linesRead++
620 + }
621 + require.NoError(t, scanner.Err())
622 + require.Equal(t, 2*count, linesRead)
623 +}
624 +
625 +// writeDeadlineRecorder records every write deadline set on it.
626 +type writeDeadlineRecorder struct {
627 + *httptest.ResponseRecorder
628 + deadlines []time.Time
629 +}
630 +
631 +func (w *writeDeadlineRecorder) SetWriteDeadline(deadline time.Time) error {
632 + w.deadlines = append(w.deadlines, deadline)
633 + return nil
634 +}
635 +
636 +func TestIdleTimeoutWriter_ClearBetweenWrites(t *testing.T) {
637 + const idle = time.Hour
638 + hard := time.Now().Add(2 * time.Hour)
639 +
640 + // the values recorded for each SetWriteDeadline call
641 + const (
642 + idleSet = "idle"
643 + zeroSet = "zero"
644 + hardSet = "hard"
645 + otherSet = "other"
646 + )
647 +
648 + for i, tc := range []struct {
649 + name string
650 + clear bool
651 + hard bool
652 + ops func(w *IdleTimeoutWriter) error
653 + expected []string
654 + }{
655 + {
656 + name: "write keeps the deadline when not clearing",
657 + ops: func(w *IdleTimeoutWriter) error { _, err := w.Write([]byte("x")); return err },
658 + expected: []string{idleSet},
659 + },
660 + {
661 + name: "write clears the deadline",
662 + clear: true,
663 + ops: func(w *IdleTimeoutWriter) error { _, err := w.Write([]byte("x")); return err },
664 + expected: []string{idleSet, zeroSet},
665 + },
666 + {
667 + name: "write puts back the hard deadline",
668 + clear: true,
669 + hard: true,
670 + ops: func(w *IdleTimeoutWriter) error { _, err := w.Write([]byte("x")); return err },
671 + expected: []string{idleSet, hardSet},
672 + },
673 + {
674 + name: "empty write leaves the deadline alone",
675 + clear: true,
676 + hard: true,
677 + ops: func(w *IdleTimeoutWriter) error { _, err := w.Write(nil); return err },
678 + expected: nil,
679 + },
680 + {
681 + name: "read from clears the deadline",
682 + clear: true,
683 + ops: func(w *IdleTimeoutWriter) error { _, err := w.ReadFrom(bytes.NewReader([]byte("x"))); return err },
684 + expected: []string{idleSet, zeroSet},
685 + },
686 + {
687 + name: "flush is bounded by the idle deadline",
688 + ops: func(w *IdleTimeoutWriter) error { return w.FlushError() },
689 + expected: []string{idleSet},
690 + },
691 + {
692 + name: "flush clears the deadline",
693 + clear: true,
694 + ops: func(w *IdleTimeoutWriter) error { return w.FlushError() },
695 + expected: []string{idleSet, zeroSet},
696 + },
697 + {
698 + name: "handler done arms a deadline for unflushed writes",
699 + clear: true,
700 + ops: func(w *IdleTimeoutWriter) error {
701 + _, err := w.Write([]byte("x"))
702 + w.HandlerDone()
703 + return err
704 + },
705 + expected: []string{idleSet, zeroSet, idleSet},
706 + },
707 + {
708 + name: "handler done does nothing after a flush",
709 + clear: true,
710 + ops: func(w *IdleTimeoutWriter) error {
711 + if _, err := w.Write([]byte("x")); err != nil {
712 + return err
713 + }
714 + err := w.FlushError()
715 + w.HandlerDone()
716 + return err
717 + },
718 + expected: []string{idleSet, zeroSet, idleSet, zeroSet},
719 + },
720 + {
721 + name: "handler done does nothing when not clearing",
722 + ops: func(w *IdleTimeoutWriter) error {
723 + _, err := w.Write([]byte("x"))
724 + w.HandlerDone()
725 + return err
726 + },
727 + expected: []string{idleSet},
728 + },
729 + } {
730 + rec := &writeDeadlineRecorder{ResponseRecorder: httptest.NewRecorder()}
731 + w := &IdleTimeoutWriter{
732 + ResponseWriterWrapper: &ResponseWriterWrapper{ResponseWriter: rec},
733 + Ctrl: http.NewResponseController(rec),
734 + Deadline: IdleDeadline{Timeout: idle},
735 + Logger: zap.NewNop(),
736 + ClearBetweenWrites: tc.clear,
737 + }
738 + if tc.hard {
739 + w.Deadline.HardDeadline = hard
740 + }
741 +
742 + before := time.Now()
743 + if err := tc.ops(w); err != nil {
744 + t.Errorf("Test %d (%s): unexpected error: %v", i, tc.name, err)
745 + }
746 + after := time.Now()
747 +
748 + var actual []string
749 + for _, d := range rec.deadlines {
750 + switch {
751 + case d.IsZero():
752 + actual = append(actual, zeroSet)
753 + case d.Equal(hard):
754 + actual = append(actual, hardSet)
755 + case !d.Before(before.Add(idle)) && !d.After(after.Add(idle)):
756 + actual = append(actual, idleSet)
757 + default:
758 + actual = append(actual, otherSet)
759 + }
760 + }
761 + assert.Equal(t, tc.expected, actual, "Test %d (%s)", i, tc.name)
762 + }
763 +}
764 +
765 // readFromCounter is a fake http.ResponseWriter that records how many
766 // times ReadFrom was called on it and the size of the largest call, so
767 // tests can assert on chunking behavior deterministically instead of
modules/caddyhttp/server.go
+6 -3
@@ -602,7 +602,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
602 r.Body = idleReader
603 }
604 if s.WriteIdleTimeout > 0 {
605 - w = &IdleTimeoutWriter{
605 + idleWriter := &IdleTimeoutWriter{
606 ResponseWriterWrapper: &ResponseWriterWrapper{ResponseWriter: w},
607 Ctrl: rc,
608 Deadline: IdleDeadline{
@@ -611,9 +611,12 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
611 MinRate: s.WriteMinRate,
612 HardDeadline: writeHardDeadline,
613 },
614 - MaxChunk: s.MaxWriteChunk,
615 - Logger: s.logger,
614 + MaxChunk: s.MaxWriteChunk,
615 + Logger: s.logger,
616 + ClearBetweenWrites: r.ProtoMajor == 2,
617 }
618 + defer idleWriter.HandlerDone()
619 + w = idleWriter
620 }
621
622 // set the Server header
modules/caddyhttp/timeouts/timeouts.go
+6 -3
@@ -107,7 +107,7 @@ func (t Timeouts) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht
107 r.Body = idleReader
108 }
109 if t.WriteTimeout > 0 {
110 - w = &caddyhttp.IdleTimeoutWriter{
110 + idleWriter := &caddyhttp.IdleTimeoutWriter{
111 ResponseWriterWrapper: &caddyhttp.ResponseWriterWrapper{ResponseWriter: w},
112 Ctrl: rc,
113 Deadline: caddyhttp.IdleDeadline{
@@ -115,9 +115,12 @@ func (t Timeouts) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht
115 Timeout: t.WriteTimeout,
116 MinRate: t.WriteMinRate,
117 },
118 - MaxChunk: t.MaxWriteChunk,
119 - Logger: t.logger,
118 + MaxChunk: t.MaxWriteChunk,
119 + Logger: t.logger,
120 + ClearBetweenWrites: r.ProtoMajor == 2,
121 }
122 + defer idleWriter.HandlerDone()
123 + w = idleWriter
124 }
125
126 return next.ServeHTTP(w, r)