[misc] Bound the upload server's request timeouts

http.Server was built with only Addr and Handler, so every timeout was infinite.
Behind a reverse proxy that is survivable because the proxy has its own; serving
TLS directly, which SERVER_CERT_FILE now allows, it means a slow client can hold
a connection and its goroutine indefinitely.

ReadHeaderTimeout and IdleTimeout are short. ReadTimeout is 10 minutes: it has
to clear a 150 MiB upload on a slow link, so it is a ceiling on a stalled
connection rather than a throughput rule. WriteTimeout is deliberately left
unset for the same reason.

Reported by CodeRabbit (CWE-400) on #7514.
This commit is contained in:
riccardom
2026-09-14 09:20:03 +02:00
parent b731521e4f
commit 7e2b71d7f8
+18 -1
View File
@@ -25,6 +25,11 @@ const (
// front of it.
certFileVar = "SERVER_CERT_FILE"
keyFileVar = "SERVER_KEY_FILE"
// readTimeout bounds a whole request. It has to clear a 150 MiB upload on a
// slow link, so it is generous rather than tight; it exists to put a ceiling
// on a connection that stalls forever, not to police throughput.
readTimeout = 10 * time.Minute
)
type Server struct {
@@ -55,7 +60,19 @@ func NewServer() *Server {
}
return &Server{
srv: &http.Server{Addr: address, Handler: mux},
srv: &http.Server{
Addr: address,
Handler: mux,
// A deployment terminating TLS in front of this server gets timeouts
// from its proxy; one serving TLS directly (certFileVar below) has
// only these, and without them a slow client holds a connection and
// its goroutine for as long as it likes. The write side is left
// alone on purpose: uploads run to 150 MiB and a deadline there
// would cut off slow but legitimate ones.
ReadHeaderTimeout: 10 * time.Second,
ReadTimeout: readTimeout,
IdleTimeout: 60 * time.Second,
},
certFile: certFile,
keyFile: keyFile,
}