git » gofer » commit 93594c6

http: Implement per-path timeouts

author Alberto Bertogli
2025-03-29 19:23:51 UTC
committer Alberto Bertogli
2025-03-29 19:54:00 UTC
parent 3d903020054f941815e262ef716ff89658690355

http: Implement per-path timeouts

This patch implements a new `timeouts` option, which lets users set
per-path timeouts on HTTP requests.

config/config.go +19 -0
config/config_test.go +16 -0
config/gofer.schema.cue +9 -0
config/gofer.yaml +10 -0
server/http.go +41 -0
test/01-be.yaml +7 -0
test/01-fe.yaml +10 -0
test/test.sh +9 -0
test/testdata/expected-printed-01-be-config +12 -0
test/testdata/sleep.sh +10 -0

diff --git a/config/config.go b/config/config.go
index 8fa6281..544fa29 100644
--- a/config/config.go
+++ b/config/config.go
@@ -36,6 +36,8 @@ type HTTP struct {
 	ReqLog map[string]string `yaml:",omitempty"`
 
 	RateLimit map[string]string `yaml:",omitempty"`
+
+	Timeouts map[string]Timeout `yaml:",omitempty"`
 }
 
 type HTTPS struct {
@@ -54,6 +56,11 @@ type AutoCerts struct {
 	AcmeURL  string   `yaml:",omitempty"`
 }
 
+type Timeout struct {
+	Read  time.Duration `yaml:",omitempty"`
+	Write time.Duration `yaml:",omitempty"`
+}
+
 type Route struct {
 	Dir        string   `yaml:",omitempty"`
 	File       string   `yaml:",omitempty"`
@@ -182,6 +189,18 @@ func (h HTTP) Check(c Config, addr string) []error {
 		}
 	}
 
+	// Verify timeouts are positive.
+	for path, timeout := range h.Timeouts {
+		if timeout.Read < 0 {
+			errs = append(errs,
+				fmt.Errorf("%q: %q: read timeout must be positive", addr, path))
+		}
+		if timeout.Write < 0 {
+			errs = append(errs,
+				fmt.Errorf("%q: %q: write timeout must be positive", addr, path))
+		}
+	}
+
 	return errs
 }
 
diff --git a/config/config_test.go b/config/config_test.go
index e9d7d02..6e58121 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -226,6 +226,22 @@ raw:
 `
 	expectErrs(t, `":1234": unknown ratelimit "lalala"`,
 		loadAndCheck(t, contents))
+
+	// Negative read and write timeouts.
+	contents = `
+http:
+  ":http":
+    routes:
+      "/":
+        file: "/dev/null"
+    timeouts:
+      "/":
+        read: "-1s"
+        write: "-1s"
+`
+	got := loadAndCheck(t, contents)
+	expectErrs(t, `":http": "/": read timeout must be positive`, got)
+	expectErrs(t, `":http": "/": write timeout must be positive`, got)
 }
 
 func loadAndCheck(t *testing.T, contents string) []error {
diff --git a/config/gofer.schema.cue b/config/gofer.schema.cue
index 30932f5..2e2191e 100644
--- a/config/gofer.schema.cue
+++ b/config/gofer.schema.cue
@@ -5,6 +5,10 @@
 // Example:
 //   cue vet /etc/gofer.schema.cue /etc/gofer.yaml
 
+import (
+	"time"
+)
+
 control_addr?: string
 
 reqlog?:
@@ -70,6 +74,11 @@ https?:
 
 	ratelimit?: [string]: string
 
+	timeouts?: [string]: {
+		read?: time.Duration
+		write?: time.Duration
+	}
+
 	...
 }
 
diff --git a/config/gofer.yaml b/config/gofer.yaml
index 007266f..f2d124b 100644
--- a/config/gofer.yaml
+++ b/config/gofer.yaml
@@ -129,6 +129,16 @@ http:
     reqlog:
       "/": "requests.log"
 
+    # Per-path timeouts. Values are strings representing durations (in Go
+    # format).  Read and write timeouts are supported.
+    timeouts:
+      "/":
+        read: "30s"
+        write: "30s"
+      "/upload/": 
+        read: "5m"
+        write: "60s"
+
 
 # HTTPS servers.
 https:
diff --git a/server/http.go b/server/http.go
index 35c0617..dffe7b7 100644
--- a/server/http.go
+++ b/server/http.go
@@ -106,6 +106,21 @@ func httpServer(addr string, conf config.HTTP) (*http.Server, error) {
 		srv.Handler = hdrMux
 	}
 
+	// Custom timeouts.
+	if len(conf.Timeouts) > 0 {
+		timeoutMux := http.NewServeMux()
+		for path, timeout := range conf.Timeouts {
+			timeoutMux.Handle(path, WithTimeout(srv.Handler, timeout))
+			log.Infof("%s timeout %q -> read:%s write:%s",
+				srv.Addr, path, timeout.Read, timeout.Write)
+		}
+
+		if _, ok := conf.Timeouts["/"]; !ok {
+			timeoutMux.Handle("/", srv.Handler)
+		}
+		srv.Handler = timeoutMux
+	}
+
 	// Logging for all entries.
 	// Because this will use the request logs if available, it needs to be
 	// wrapped by it.
@@ -436,6 +451,12 @@ func (w *statusWriter) Flush() {
 	}
 }
 
+// Unwrap is used by ResponseController to get the underlying
+// http.ResponseWriter.
+func (w *statusWriter) Unwrap() http.ResponseWriter {
+	return w.ResponseWriter
+}
+
 func SetHeader(parent http.Handler, hdrs map[string]string) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		tr, _ := trace.FromContext(r.Context())
@@ -546,3 +567,23 @@ func WithRateLimit(parent http.Handler, rl *ipratelimit.Limiter) http.Handler {
 		parent.ServeHTTP(w, r)
 	})
 }
+
+func WithTimeout(parent http.Handler, timeout config.Timeout) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		tr, _ := trace.FromContext(r.Context())
+
+		rc := http.NewResponseController(w)
+
+		if timeout.Read > 0 {
+			tr.Printf("read timeout: %v", timeout.Read)
+			rc.SetReadDeadline(time.Now().Add(timeout.Read))
+		}
+
+		if timeout.Write > 0 {
+			tr.Printf("write timeout: %v", timeout.Write)
+			rc.SetWriteDeadline(time.Now().Add(timeout.Write))
+		}
+
+		parent.ServeHTTP(w, r)
+	})
+}
diff --git a/test/01-be.yaml b/test/01-be.yaml
index 39a0bf0..fdbd337 100644
--- a/test/01-be.yaml
+++ b/test/01-be.yaml
@@ -34,6 +34,13 @@ http:
       "/status/543":
         status: 543
 
+      "/slow/10ms":
+        cgi: ["testdata/sleep.sh", "0.01"]
+      "/slow/100ms":
+        cgi: ["testdata/sleep.sh", "0.1"]
+      "/slow/500ms":
+        cgi: ["testdata/sleep.sh", "0.5"]
+
     auth:
       "/authdir/ñaca": "testdata/authdb.yaml"
       "/authdir/withoutindex/": "testdata/authdb.yaml"
diff --git a/test/01-fe.yaml b/test/01-fe.yaml
index 6520695..904d96c 100644
--- a/test/01-fe.yaml
+++ b/test/01-fe.yaml
@@ -31,6 +31,13 @@ _routes: &routes
       - from: "/rere/(.*)/zzz/(.*)"
         to: "http://example.com/dst/z/$2/z/$1"
         status: 308
+  "/timeout/":
+    proxy: "http://localhost:8450/slow/"
+
+_timeouts: &timeouts
+  "/timeout/":
+    read: "200ms"
+    write: "200ms"
 
 reqlog:
   "requests":
@@ -52,6 +59,7 @@ http:
       "/": "requests"
     ratelimit:
       "/rlme/": "rl"
+    timeouts: *timeouts
 
 https:
   ":8442":
@@ -59,6 +67,7 @@ https:
     routes: *routes
     reqlog:
       "/": "requests"
+    timeouts: *timeouts
     insecure_key_log_file: ".01-fe.8442.tls-secrets.txt"
 
   ":8443":
@@ -69,6 +78,7 @@ https:
     routes: *routes
     reqlog:
       "/": "requests"
+    timeouts: *timeouts
     insecure_key_log_file: ".01-fe.8443.tls-secrets.txt"
 
 
diff --git a/test/test.sh b/test/test.sh
index 5da5641..a829ecc 100755
--- a/test/test.sh
+++ b/test/test.sh
@@ -165,6 +165,15 @@ do
 
 	# Additional headers.
 	exp $base/file -hdrre "X-My-Header: my lovely header"
+
+	# Timeouts.
+	exp $base/timeout/10ms -status 200
+	exp $base/timeout/100ms -status 200
+	# The 500ms request times out, and this appears to the client as
+	# either EOF (over HTTP) or a TLS error (over HTTPS).
+	# This behaviours matches reaching the timeout of http.Server.
+	exp $base/timeout/500ms \
+		-clienterrorre "EOF|local error: tls: bad record MAC"
 done
 
 
diff --git a/test/testdata/expected-printed-01-be-config b/test/testdata/expected-printed-01-be-config
index 49b6c48..b1c8f02 100644
--- a/test/testdata/expected-printed-01-be-config
+++ b/test/testdata/expected-printed-01-be-config
@@ -22,6 +22,18 @@ http:
                 file: testdata/file
             /file/second:
                 file: testdata/dir/ñaca
+            /slow/10ms:
+                cgi:
+                    - testdata/sleep.sh
+                    - "0.01"
+            /slow/100ms:
+                cgi:
+                    - testdata/sleep.sh
+                    - "0.1"
+            /slow/500ms:
+                cgi:
+                    - testdata/sleep.sh
+                    - "0.5"
             /status/543:
                 status: 543
         auth:
diff --git a/test/testdata/sleep.sh b/test/testdata/sleep.sh
new file mode 100755
index 0000000..59f76ab
--- /dev/null
+++ b/test/testdata/sleep.sh
@@ -0,0 +1,10 @@
+#!/bin/bash
+
+set -e
+
+sleep $1
+
+echo "Content-type: text/plain"
+echo
+echo "Slept $1"
+