fix(uploads): bound the silence in an upload body with a re-armed read deadline (#56) #63

Merged
mathias merged 3 commits from fix/56-upload-idle-read-deadline into main 2026-10-05 04:59:29 +00:00
3 changed files with 28 additions and 7 deletions
Showing only changes of commit b1887e7594 - Show all commits
+6
View File
@@ -1,5 +1,11 @@
package web package web
import "time"
// DefaultUploadIdleTimeout exposes the production idle bound (#56) to the black-box tests: // DefaultUploadIdleTimeout exposes the production idle bound (#56) to the black-box tests:
// cmd/tapir never sets App.UploadIdleTimeout, so this constant IS the deployed value. // cmd/tapir never sets App.UploadIdleTimeout, so this constant IS the deployed value.
const DefaultUploadIdleTimeout = defaultUploadIdleTimeout const DefaultUploadIdleTimeout = defaultUploadIdleTimeout
// UploadIdleTimeoutForTest exposes the bound an App resolves to (#56), so a test can pin the
// value the zero-value App -- production's -- actually uses rather than only the constant.
func (a *App) UploadIdleTimeoutForTest() time.Duration { return a.uploadIdleTimeout() }
+10 -4
View File
@@ -142,6 +142,15 @@ func (a *App) handleUploadPost(w http.ResponseWriter, r *http.Request) {
// hostile one; a legitimate upload, however slow, keeps delivering bytes. // hostile one; a legitimate upload, however slow, keeps delivering bytes.
const defaultUploadIdleTimeout = 60 * time.Second const defaultUploadIdleTimeout = 60 * time.Second
// uploadIdleTimeout is the idle bound this App applies: UploadIdleTimeout when set, else
// defaultUploadIdleTimeout. cmd/tapir leaves it unset, so production runs the default.
func (a *App) uploadIdleTimeout() time.Duration {
if a.UploadIdleTimeout <= 0 {
return defaultUploadIdleTimeout
}
return a.UploadIdleTimeout
}
// idleBoundedBody bounds the SILENCE in an upload body, not its total duration (#56): every // idleBoundedBody bounds the SILENCE in an upload body, not its total duration (#56): every
// read first pushes the connection's read deadline idle into the future, so a body that // read first pushes the connection's read deadline idle into the future, so a body that
// keeps moving is never cut, and one that stops fails its read with a timeout. That read // keeps moving is never cut, and one that stops fails its read with a timeout. That read
@@ -152,10 +161,7 @@ const defaultUploadIdleTimeout = 60 * time.Second
// the body is returned unbounded and that is logged: a control that silently does not // the body is returned unbounded and that is logged: a control that silently does not
// apply is worse than one that says so. // apply is worse than one that says so.
func (a *App) idleBoundedBody(w http.ResponseWriter, body io.ReadCloser) io.ReadCloser { func (a *App) idleBoundedBody(w http.ResponseWriter, body io.ReadCloser) io.ReadCloser {
idle := a.UploadIdleTimeout idle := a.uploadIdleTimeout()
if idle <= 0 {
idle = defaultUploadIdleTimeout
}
rc := http.NewResponseController(w) rc := http.NewResponseController(w)
if err := rc.SetReadDeadline(time.Now().Add(idle)); err != nil { if err := rc.SetReadDeadline(time.Now().Add(idle)); err != nil {
a.logger().Warn("upload idle bound unavailable", "err", err) a.logger().Warn("upload idle bound unavailable", "err", err)
+12 -3
View File
@@ -121,9 +121,18 @@ func TestDefaultUploadIdleTimeoutIsSixtySeconds(t *testing.T) {
require.Equal(t, 60*time.Second, web.DefaultUploadIdleTimeout) require.Equal(t, 60*time.Second, web.DefaultUploadIdleTimeout)
} }
// With UploadIdleTimeout unset the default applies: a body with 100 ms gaps between reads is // The bound an App actually resolves to, not the constant: production's App leaves
// accepted (a default in milliseconds or nanoseconds would cut it). // UploadIdleTimeout unset, so the zero value must resolve to exactly 60 s, and a set value
func TestUploadPost_AnUnsetIdleTimeoutAppliesTheDefaultAndAcceptsABodyWithSubSecondGaps(t *testing.T) { // must be used as given.
func TestAppUploadIdleTimeout_ResolvesUnsetToSixtySecondsAndHonoursASetValue(t *testing.T) {
require.Equal(t, 60*time.Second, (&web.App{}).UploadIdleTimeoutForTest())
require.Equal(t, 60*time.Second, (&web.App{UploadIdleTimeout: -time.Second}).UploadIdleTimeoutForTest())
require.Equal(t, 300*time.Millisecond, (&web.App{UploadIdleTimeout: 300 * time.Millisecond}).UploadIdleTimeoutForTest())
}
// With UploadIdleTimeout unset, a body with 100 ms gaps between reads is not cut (a resolved
// bound in milliseconds or nanoseconds would cut it). The exact value is pinned above.
func TestUploadPost_AnUnsetIdleTimeoutDoesNotCutABodyWithSubSecondGaps(t *testing.T) {
addr := newIdleBoundServer(t, 0) addr := newIdleBoundServer(t, 0)
body, ct := multipartBody(t, bytes.Repeat([]byte("a"), 1000)) body, ct := multipartBody(t, bytes.Repeat([]byte("a"), 1000))