Make response write and artifact download timeouts configurable

config.example.yaml

@@ -14,6 +14,14 @@ base_url: "http://localhost:8080"
 # Set to "0" to disable the timeout. Default: "30s".
 # http_timeout: "30s"
 
+# Response write deadline, including download and scan time before sending data.
+# Set to "0" to disable. Default: "5m".
+# write_timeout: "5m"
+
+# Timeout for each upstream artifact download while serving requests; excludes
+# scanning. Set to "0" to disable. Default: "5m". Does not limit proxy mirror.
+# artifact_timeout: "5m"
+
 # Largest upstream artifact to fetch (e.g. "2GB"). Larger ones are refused
 # and not cached. Default: unlimited.
 # artifact_max_size: ""

docs/configuration.md

@@ -22,6 +22,22 @@ See `config.example.yaml` in the repository root for a complete example.
 | `trust_forwarded_for` | `PROXY_TRUST_FORWARDED_FOR` | - | `false` | Attribute requests to the leftmost `X-Forwarded-For` entry instead of the TCP peer address, in the structured log, the access log and the request-source table on `/ui/analytics`. |
 | `ui_request_sources` | `PROXY_UI_REQUEST_SOURCES` | - | `false` | Show the request-source table on `/ui/analytics`, which reports caller addresses, the tool each ran and how much each pulled. |
 | `ui_failure_details` | `PROXY_UI_FAILURE_DETAILS` | - | `false` | List the latest failures behind each failure count on `/ui/analytics`: the package, its upstream URL or storage path, and the error. URLs are shown without credentials or query strings. |
+| `http_timeout` | `PROXY_HTTP_TIMEOUT` | - | `30s` | Timeout for upstream metadata and pass-through requests. |
+| `write_timeout` | `PROXY_WRITE_TIMEOUT` | - | `5m` | Response write deadline, including download and scanning time before the response starts. |
+| `artifact_timeout` | `PROXY_ARTIFACT_TIMEOUT` | - | `5m` | Timeout for each upstream artifact download while serving requests; excludes subsequent scanning. Does not limit `proxy mirror`. |
+
+These timeouts accept Go durations such as `30s`, `5m`, or `1h`. An omitted value uses the default; `"0"` disables that timeout. Invalid or negative values are rejected at startup. Environment variables override file values.
+
+For long scans, increase `write_timeout` to cover the download, scan, and response transfer. `scanning.timeout` controls the scan separately; changing it alone does not extend the response deadline. `artifact_timeout` is independent of both. For example:
+
+```yaml
+write_timeout: "18m"
+artifact_timeout: "10m"
+scanning:
+  timeout: "16m"
+```
+
+These are independent limits, not reserved time slices: a download approaching 10 minutes followed by a 16-minute scan would still exceed an 18-minute response deadline. Any reverse proxy in front of the server also needs sufficient timeout allowance. The upstream transport's existing response-header timeout remains unchanged.
 
 ## Storage
 

internal/config/config.go

@@ -186,6 +186,15 @@ type Config struct {
 	// response headers within 60 seconds.
 	HTTPTimeout string `json:"http_timeout" yaml:"http_timeout"`
 
+	// WriteTimeout bounds response writes, including time spent downloading and
+	// scanning before the response starts. Default: "5m"; "0" disables it.
+	WriteTimeout string `json:"write_timeout" yaml:"write_timeout"`
+
+	// ArtifactTimeout bounds each upstream artifact download while serving
+	// requests, excluding scanning. Default: "5m"; "0" disables it.
+	// The mirror command keeps its unlimited artifact download timeout.
+	ArtifactTimeout string `json:"artifact_timeout" yaml:"artifact_timeout"`
+
 	// MirrorAPI enables the /api/mirror endpoints for starting mirror jobs via HTTP.
 	// Disabled by default to prevent unauthenticated users from triggering downloads.
 	MirrorAPI bool `json:"mirror_api" yaml:"mirror_api"`
@@ -1258,6 +1267,8 @@ func (c *Config) LoadFromEnv() {
 	setEnvString(&c.ArtifactMaxSize, "PROXY_ARTIFACT_MAX_SIZE")
 	setEnvString(&c.MetadataRewriteCacheSize, "PROXY_METADATA_REWRITE_CACHE_SIZE")
 	setEnvString(&c.HTTPTimeout, "PROXY_HTTP_TIMEOUT")
+	setEnvString(&c.WriteTimeout, "PROXY_WRITE_TIMEOUT")
+	setEnvString(&c.ArtifactTimeout, "PROXY_ARTIFACT_TIMEOUT")
 	c.setGradleReadOnlyFromEnv()
 	setEnvString(&c.Gradle.BuildCache.MaxUploadSize, "PROXY_GRADLE_BUILD_CACHE_MAX_UPLOAD_SIZE")
 	setEnvString(&c.Gradle.BuildCache.MaxAge, "PROXY_GRADLE_BUILD_CACHE_MAX_AGE")
@@ -1357,7 +1368,7 @@ func (c *Config) Validate() error {
 		return err
 	}
 
-	if err := validateHTTPTimeout(c.HTTPTimeout); err != nil {
+	if err := c.validateRequestTimeouts(); err != nil {
 		return err
 	}
 
@@ -1481,6 +1492,8 @@ func (g *GradleBuildCacheConfig) Validate() error {
 const (
 	defaultMetadataTTL                   = 5 * time.Minute  //nolint:mnd // sensible default
 	defaultDirectServeTTL                = 15 * time.Minute //nolint:mnd // sensible default
+	defaultWriteTimeout                  = 5 * time.Minute
+	defaultArtifactTimeout               = 5 * time.Minute
 	defaultHTTPTimeout                   = 30 * time.Second //nolint:mnd // sensible default
 	defaultHitFlushInterval              = time.Second
 	defaultMetadataMaxSize               = 100 << 20
@@ -1509,16 +1522,30 @@ func (c *Config) ParseMaxSize() int64 {
 	return size
 }
 
-func validateHTTPTimeout(s string) error {
+func (c *Config) validateRequestTimeouts() error {
+	for _, timeout := range []struct{ field, value string }{
+		{"http_timeout", c.HTTPTimeout},
+		{"write_timeout", c.WriteTimeout},
+		{"artifact_timeout", c.ArtifactTimeout},
+	} {
+		if err := validateRequestTimeout(timeout.field, timeout.value); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func validateRequestTimeout(field, s string) error {
 	if s == "" || s == "0" {
 		return nil
 	}
 	d, err := time.ParseDuration(s)
 	if err != nil {
-		return fmt.Errorf("invalid http_timeout %q: %w", s, err)
+		return fmt.Errorf("invalid %s %q: %w", field, s, err)
 	}
 	if d < 0 {
-		return fmt.Errorf("invalid http_timeout %q: must be non-negative", s)
+		return fmt.Errorf("invalid %s %q: must be non-negative", field, s)
 	}
 	return nil
 }
@@ -1617,15 +1644,28 @@ func (c *Config) ParseArtifactMaxSize() int64 {
 // ParseHTTPTimeout returns the upstream HTTP client timeout.
 // Returns 30s if unset, 0 (no timeout) if explicitly set to "0".
 func (c *Config) ParseHTTPTimeout() time.Duration {
-	if c.HTTPTimeout == "" {
-		return defaultHTTPTimeout
-	}
-	if c.HTTPTimeout == "0" {
-		return 0
+	return parseRequestTimeout(c.HTTPTimeout, defaultHTTPTimeout)
+}
+
+// ParseWriteTimeout returns the response write timeout, defaulting to 5m.
+// An explicit zero disables the timeout.
+func (c *Config) ParseWriteTimeout() time.Duration {
+	return parseRequestTimeout(c.WriteTimeout, defaultWriteTimeout)
+}
+
+// ParseArtifactTimeout returns the upstream artifact download timeout for
+// serving requests, defaulting to 5m. An explicit zero disables the timeout.
+func (c *Config) ParseArtifactTimeout() time.Duration {
+	return parseRequestTimeout(c.ArtifactTimeout, defaultArtifactTimeout)
+}
+
+func parseRequestTimeout(value string, fallback time.Duration) time.Duration {
+	if value == "" {
+		return fallback
 	}
-	d, err := time.ParseDuration(c.HTTPTimeout)
+	d, err := time.ParseDuration(value)
 	if err != nil || d < 0 {
-		return defaultHTTPTimeout
+		return fallback
 	}
 	return d
 }

internal/config/request_timeouts_test.go

@@ -0,0 +1,76 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestRequestTimeouts(t *testing.T) {
+	for _, value := range []string{"", "0", "0s", "18m", "invalid", "-1s", "999999999999999999h"} {
+		t.Run(value, func(t *testing.T) {
+			cfg := Default()
+			cfg.WriteTimeout, cfg.ArtifactTimeout = value, value
+			valid := value == "" || value == "0" || value == "0s" || value == "18m"
+			want := 5 * time.Minute
+			if value == "0" || value == "0s" {
+				want = 0
+			}
+			if value == "18m" {
+				want = 18 * time.Minute
+			}
+			if got := cfg.ParseWriteTimeout(); got != want {
+				t.Errorf("write timeout = %v, want %v", got, want)
+			}
+			if got := cfg.ParseArtifactTimeout(); got != want {
+				t.Errorf("artifact timeout = %v, want %v", got, want)
+			}
+			err := cfg.Validate()
+			if (err == nil) != valid {
+				t.Fatalf("Validate() = %v, valid = %v", err, valid)
+			}
+			if !valid {
+				if !strings.Contains(err.Error(), "write_timeout") {
+					t.Fatalf("error does not identify write_timeout: %v", err)
+				}
+				cfg.WriteTimeout = ""
+				if err := cfg.Validate(); err == nil || !strings.Contains(err.Error(), "artifact_timeout") {
+					t.Fatalf("artifact validation = %v", err)
+				}
+			}
+		})
+	}
+}
+
+func TestLoadRequestTimeouts(t *testing.T) {
+	for _, format := range []struct{ extension, data string }{
+		{"yaml", "write_timeout: 18m\nartifact_timeout: 10m\nhttp_timeout: 90s\n"},
+		{"json", `{"write_timeout":"18m","artifact_timeout":"10m","http_timeout":"90s"}`},
+	} {
+		t.Run(format.extension, func(t *testing.T) {
+			path := filepath.Join(t.TempDir(), "config."+format.extension)
+			if err := os.WriteFile(path, []byte(format.data), 0600); err != nil {
+				t.Fatal(err)
+			}
+			cfg, err := Load(path)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if cfg.ParseWriteTimeout() != 18*time.Minute || cfg.ParseArtifactTimeout() != 10*time.Minute || cfg.ParseHTTPTimeout() != 90*time.Second {
+				t.Fatalf("file timeouts = %v, %v, %v", cfg.WriteTimeout, cfg.ArtifactTimeout, cfg.HTTPTimeout)
+			}
+			t.Setenv("PROXY_WRITE_TIMEOUT", "20m")
+			t.Setenv("PROXY_ARTIFACT_TIMEOUT", "0")
+			t.Setenv("PROXY_HTTP_TIMEOUT", "15s")
+			cfg.LoadFromEnv()
+			if cfg.ParseWriteTimeout() != 20*time.Minute || cfg.ParseArtifactTimeout() != 0 || cfg.ParseHTTPTimeout() != 15*time.Second {
+				t.Fatalf("environment timeouts = %v, %v, %v", cfg.WriteTimeout, cfg.ArtifactTimeout, cfg.HTTPTimeout)
+			}
+			if err := cfg.Validate(); err != nil {
+				t.Fatal(err)
+			}
+		})
+	}
+}

internal/server/request_timeouts_test.go

@@ -0,0 +1,131 @@
+package server
+
+import (
+	"context"
+	"errors"
+	"io"
+	"log/slog"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/git-pkgs/proxy/internal/config"
+)
+
+// Exercise the real serve wiring: a slow artifact must use artifact_timeout,
+// while waiting for it also consumes the independent response write deadline.
+func TestServeRequestTimeouts(t *testing.T) {
+	for _, tc := range []struct {
+		name, write, artifact string
+		wantOK                bool
+		wantConnectionError   bool
+	}{
+		{"independent from metadata timeout", "5s", "5s", true, false},
+		{"write deadline expires while waiting", "1ms", "5s", false, true},
+		{"artifact download times out", "5s", "1ms", false, false},
+		{"both timeouts disabled", "0", "0", true, false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			status, body, err := fetchWithRequestTimeouts(t, tc.write, tc.artifact)
+			if tc.wantConnectionError {
+				if err == nil {
+					t.Fatal("response succeeded after its write deadline")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatal(err)
+			}
+			if tc.wantOK {
+				if status != http.StatusOK || body != "release asset" {
+					t.Fatalf("response = %d %q", status, body)
+				}
+			} else if status < 500 {
+				t.Fatalf("artifact timeout response = %d %q, want server error", status, body)
+			}
+		})
+	}
+}
+
+func fetchWithRequestTimeouts(t *testing.T, writeTimeout, artifactTimeout string) (int, string, error) {
+	t.Helper()
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(100 * time.Millisecond)
+		_, _ = io.WriteString(w, "release asset")
+	}))
+	defer upstream.Close()
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() { _ = listener.Close() }()
+	dir := t.TempDir()
+	cfg := config.Default()
+	cfg.Listen = listener.Addr().String()
+	cfg.BaseURL = "http://" + cfg.Listen
+	cfg.Database.Path = filepath.Join(dir, "proxy.db")
+	cfg.Storage.URL = "file://" + filepath.Join(dir, "artifacts")
+	cfg.Upstream.Generic = map[string]string{"releases": upstream.URL}
+	cfg.Upstream.AllowLoopback = true
+	cfg.HTTPTimeout = "1ns"
+	cfg.WriteTimeout, cfg.ArtifactTimeout = writeTimeout, artifactTimeout
+	if err := cfg.Validate(); err != nil {
+		t.Fatal(err)
+	}
+	srv, err := New(cfg, slog.New(slog.NewTextHandler(io.Discard, nil)), BuildInfo{Version: "test"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	done := make(chan error, 1)
+	go func() { done <- srv.Start(listener) }()
+	defer func() {
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+		if err := srv.Shutdown(ctx); err != nil {
+			t.Error(err)
+		}
+		if err := <-done; !errors.Is(err, http.ErrServerClosed) {
+			t.Errorf("Start() = %v", err)
+		}
+	}()
+	client := &http.Client{Timeout: 10 * time.Second}
+	defer client.CloseIdleConnections()
+
+	resp, err := client.Get(cfg.BaseURL + "/generic/releases/owner/repo/releases/download/v1/asset.bin")
+	if err != nil {
+		return 0, "", err
+	}
+	defer func() { _ = resp.Body.Close() }()
+	body, err := io.ReadAll(resp.Body)
+	return resp.StatusCode, string(body), err
+}
+
+func TestMirrorIgnoresServeArtifactTimeout(t *testing.T) {
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(20 * time.Millisecond)
+		_, _ = io.WriteString(w, "mirrored artifact")
+	}))
+	defer upstream.Close()
+	db, store := setupEvictionTest(t)
+	defer func() { _ = store.Close() }()
+	cfg := config.Default()
+	cfg.Upstream.AllowLoopback = true
+	cfg.ArtifactTimeout = "1ns"
+	cfg.WriteTimeout = "1ns"
+	proxy, _ := NewMirrorProxy(cfg, db, store, slog.New(slog.NewTextHandler(io.Discard, nil)))
+	defer proxy.HTTPClient.CloseIdleConnections()
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	result, err := proxy.GetOrFetchArtifactFromURL(ctx, "generic", "mirror-timeout", "v1", "asset.bin", upstream.URL+"/asset.bin")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() { _ = result.Reader.Close() }()
+	body, err := io.ReadAll(result.Reader)
+	if err != nil || string(body) != "mirrored artifact" {
+		t.Fatalf("mirror result = %q, %v", body, err)
+	}
+}

internal/server/server.go

@@ -90,11 +90,10 @@ import (
 )
 
 const (
-	serverReadTimeout  = 30 * time.Second
-	serverWriteTimeout = 5 * time.Minute
-	serverIdleTimeout  = 60 * time.Second
-	dashboardTopN      = 10
-	hoursPerDay        = 24
+	serverReadTimeout = 30 * time.Second
+	serverIdleTimeout = 60 * time.Second
+	dashboardTopN     = 10
+	hoursPerDay       = 24
 
 	// Upstream transport defaults, matching what fetch.NewFetcher would use
 	// if we did not hand it our own client. Go's default transport keeps only
@@ -226,7 +225,7 @@ func (s *Server) Start(listeners ...net.Listener) error {
 }
 
 func (s *Server) serve(listener net.Listener) error {
-	metadataClient, artifactClient := s.newUpstreamClients(serverWriteTimeout)
+	metadataClient, artifactClient := s.newUpstreamClients(s.cfg.ParseArtifactTimeout())
 
 	// Create shared components with circuit breaker.
 	baseFetcher := fetch.NewFetcher(fetch.WithHTTPClient(artifactClient))
@@ -372,7 +371,7 @@ func (s *Server) serve(listener net.Listener) error {
 		Addr:         s.cfg.Listen,
 		Handler:      r,
 		ReadTimeout:  serverReadTimeout,
-		WriteTimeout: serverWriteTimeout, // Large artifacts need time
+		WriteTimeout: s.cfg.ParseWriteTimeout(), // Large artifacts need time
 		IdleTimeout:  serverIdleTimeout,
 	}