Add Improve UI error handling, add download resuming, improve cleanup

code

main.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"crypto/rand"
+	_ "embed"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -11,7 +12,9 @@ import (
 	"io/fs"
 	"log"
 	"mime"
+	"mime/multipart"
 	"net/http"
+	"net/url"
 	"os"
 	"os/signal"
 	"path/filepath"
@@ -34,8 +37,33 @@ const defaultMaxUploadSize int64 = 5 << 30 // 5GB
 // maxUploadSize.
 const maxMultipartMemory int64 = 32 << 20 // 32MB
 
-var indexHTML *template.Template
+// maxFilesPerUpload bounds how many parts a single multipart upload may
+// contain, so one request cannot exhaust inodes with tiny files.
+const maxFilesPerUpload = 100
+
+// defaultShutdownGrace is used when SHUTDOWN_GRACE is not set. Align it with
+// the pod's terminationGracePeriodSeconds when uploads are expected to be
+// large and slow.
+const defaultShutdownGrace = 15 * time.Second
+
+// defaultCleanupInterval is used when CLEANUP_INTERVAL is not set.
+const defaultCleanupInterval = 6 * time.Hour
+
+// defaultFileExpires is used when FILE_DURATION is not set.
+const defaultFileExpires = 7 * 24 * time.Hour
+
+// newUploadGrace keeps the cleanup sweep from removing an upload directory
+// that was just created but whose file has not been written yet.
+const newUploadGrace = 5 * time.Minute
+
+// Embedded so the binary does not depend on its working directory.
+//
+//go:embed index.html.tmpl
+var indexTemplate string
+
+var indexHTML = template.Must(template.New("index").Parse(indexTemplate))
 var maxUploadSize int64 = defaultMaxUploadSize
+var fileExpires = defaultFileExpires
 var version = "dev"
 
 func main() {
@@ -43,26 +71,66 @@ func main() {
 	if uploadDir == "" {
 		uploadDir = "/uploads"
 	}
-	var fileExpires time.Duration = 7 * 24 * time.Hour
+	// DRYRUN_CLEAN defaults to true: cleanup reports what it would delete but
+	// removes nothing until it is explicitly disabled.
+	dryRun := true
+	if dryRunEnv := os.Getenv("DRYRUN_CLEAN"); dryRunEnv != "" {
+		parsed, err := strconv.ParseBool(dryRunEnv)
+		if err != nil {
+			log.Fatalf("DRYRUN_CLEAN %s is invalid: %v", dryRunEnv, err)
+		}
+		dryRun = parsed
+	}
 
-	dryRun := strings.ToLower(os.Getenv("DRYRUN_CLEAN")) != "false"
 	fileDuration := os.Getenv("FILE_DURATION")
 	if fileDuration != "" {
 		expires, err := time.ParseDuration(fileDuration)
 		if err != nil {
 			log.Fatalf("FILE_DURATION %s is invalid: %v", fileDuration, err)
 		}
+		// A negative duration would make every file look expired on the very
+		// first sweep and wipe the upload directory.
+		if expires < 0 {
+			log.Fatalf("FILE_DURATION %s is invalid: must not be negative", fileDuration)
+		}
 		fileExpires = expires
 	}
 
 	if maxUploadSizeEnv := os.Getenv("MAX_UPLOAD_SIZE"); maxUploadSizeEnv != "" {
 		size, err := strconv.ParseInt(maxUploadSizeEnv, 10, 64)
-		if err != nil || size <= 0 {
+		if err != nil {
 			log.Fatalf("MAX_UPLOAD_SIZE %s is invalid: %v", maxUploadSizeEnv, err)
 		}
+		if size <= 0 {
+			log.Fatalf("MAX_UPLOAD_SIZE %s is invalid: must be positive", maxUploadSizeEnv)
+		}
 		maxUploadSize = size
 	}
 
+	shutdownGrace := defaultShutdownGrace
+	if graceEnv := os.Getenv("SHUTDOWN_GRACE"); graceEnv != "" {
+		grace, err := time.ParseDuration(graceEnv)
+		if err != nil {
+			log.Fatalf("SHUTDOWN_GRACE %s is invalid: %v", graceEnv, err)
+		}
+		if grace <= 0 {
+			log.Fatalf("SHUTDOWN_GRACE %s is invalid: must be positive", graceEnv)
+		}
+		shutdownGrace = grace
+	}
+
+	cleanupInterval := defaultCleanupInterval
+	if intervalEnv := os.Getenv("CLEANUP_INTERVAL"); intervalEnv != "" {
+		interval, err := time.ParseDuration(intervalEnv)
+		if err != nil {
+			log.Fatalf("CLEANUP_INTERVAL %s is invalid: %v", intervalEnv, err)
+		}
+		if interval <= 0 {
+			log.Fatalf("CLEANUP_INTERVAL %s is invalid: must be positive", intervalEnv)
+		}
+		cleanupInterval = interval
+	}
+
 	port := os.Getenv("PORT")
 	if port == "" {
 		port = "8080"
@@ -71,15 +139,20 @@ func main() {
 	if err := os.MkdirAll(uploadDir, 0755); err != nil {
 		log.Fatalf("Failed to create upload directory: %v", err)
 	}
-	indexHTML = template.Must(template.ParseFiles("index.html.tmpl"))
-	http.HandleFunc("/", handleRequest)
+	mux := http.NewServeMux()
+	mux.HandleFunc("/", handleRequest)
 
 	log.Printf("Starting server on port %s, upload directory: %s", port, uploadDir)
 	log.Printf("Version: %s", version)
 	log.Printf("Files expires after: %v (DRYRUN_CLEAN=%v)", fileExpires, dryRun)
 	log.Printf("Max upload size: %d bytes", maxUploadSize)
+
+	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
+	defer stop()
+
 	if fileExpires != 0 {
-		go cleanupProcedure(fileExpires, dryRun)
+		log.Printf("Cleanup interval: %v", cleanupInterval)
+		go cleanupProcedure(ctx, fileExpires, cleanupInterval, dryRun)
 	} else {
 		log.Print("Cleaning old files is disabled")
 	}
@@ -90,69 +163,65 @@ func main() {
 	// style attacks on connection/header handling.
 	srv := &http.Server{
 		Addr:              ":" + port,
+		Handler:           withLogging(mux),
 		ReadHeaderTimeout: 10 * time.Second,
 		IdleTimeout:       120 * time.Second,
 	}
 
 	serverErr := make(chan error, 1)
 	go func() {
-		if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+		if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
 			serverErr <- err
 		}
 		close(serverErr)
 	}()
 
-	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
-	defer stop()
-
 	select {
 	case err := <-serverErr:
 		log.Fatalf("Server failed: %v", err)
 	case <-ctx.Done():
 		log.Print("Shutdown signal received, shutting down gracefully...")
-		shutdownCtx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+		shutdownCtx, cancel := context.WithTimeout(context.Background(), shutdownGrace)
 		defer cancel()
+		// A large upload in flight will not finish inside the grace period.
+		// Force the remaining connections closed and still exit cleanly, so a
+		// rollout mid-transfer does not look like a crash to Kubernetes.
 		if err := srv.Shutdown(shutdownCtx); err != nil {
-			log.Fatalf("Graceful shutdown failed: %v", err)
+			log.Printf("Graceful shutdown did not complete within %v (%v), forcing close", shutdownGrace, err)
+			srv.Close()
 		}
 		log.Print("Server stopped")
 	}
 }
 
 func handleRequest(w http.ResponseWriter, r *http.Request) {
-	start := time.Now()
-
-	if r.Method == "GET" && r.URL.Path == "/" {
+	if r.Method == http.MethodGet && r.URL.Path == "/" {
 		serveFrontpage(w, r)
-		logRequest(r, 200, time.Since(start))
 		return
 	}
-	if r.Method == "GET" && r.URL.Path == "/healthz" {
+	if r.Method == http.MethodGet && r.URL.Path == "/healthz" {
 		handleHealthz(w, r)
 		return
 	}
 
-	if r.Method == "POST" && r.URL.Path == "/upload" {
-		handleMultipartUpload(w, r, start)
+	if r.Method == http.MethodPost && r.URL.Path == "/upload" {
+		handleMultipartUpload(w, r)
 		return
 	}
 
-	if r.Method == "PUT" {
-		handleUpload(w, r, start)
+	if r.Method == http.MethodPut {
+		handleUpload(w, r)
 		return
 	}
 
-	if r.Method == "GET" {
-		handleDownload(w, r, start)
+	// ServeContent handles HEAD correctly, so downloads answer both verbs.
+	if r.Method == http.MethodGet || r.Method == http.MethodHead {
+		handleDownload(w, r)
 		return
 	}
 
-	if r.Method == "HEAD" {
-		return
-	}
-
-	w.WriteHeader(http.StatusMethodNotAllowed)
-	logRequest(r, 405, time.Since(start))
+	w.Header().Set("Allow", "GET, HEAD, PUT, POST")
+	http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
 }
 
 func handleHealthz(w http.ResponseWriter, r *http.Request) {
@@ -164,20 +233,23 @@ func handleHealthz(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(http.StatusOK)
 }
 
-func handleUpload(w http.ResponseWriter, r *http.Request, start time.Time) {
-	filename := strings.TrimPrefix(r.URL.Path, "/")
-	if filename == "" {
+func handleUpload(w http.ResponseWriter, r *http.Request) {
+	rawName := strings.TrimPrefix(r.URL.Path, "/")
+	if rawName == "" {
 		http.Error(w, "Filename required", http.StatusBadRequest)
-		logRequest(r, 400, time.Since(start))
 		return
 	}
 
-	filename = filepath.Base(filename)
+	filename, ok := safeFilename(rawName)
+	if !ok {
+		http.Error(w, "Invalid filename", http.StatusBadRequest)
+		return
+	}
 
 	id, err := generateUniqueID()
 	if err != nil {
+		log.Printf("Failed to generate unique ID: %v", err)
 		http.Error(w, "Failed to generate unique ID", http.StatusInternalServerError)
-		logRequest(r, 500, time.Since(start))
 		return
 	}
 
@@ -186,13 +258,15 @@ func handleUpload(w http.ResponseWriter, r *http.Request, start time.Time) {
 	file, err := os.Create(targetPath)
 	if err != nil {
 		os.Remove(targetDir)
+		log.Printf("Failed to create %q: %v", targetPath, err)
 		http.Error(w, "Failed to create file", http.StatusInternalServerError)
-		logRequest(r, 500, time.Since(start))
 		return
 	}
 	defer file.Close()
 
-	limitedBody := http.MaxBytesReader(w, r.Body, maxUploadSize)
+	// MaxBytesReader needs the ResponseWriter the stdlib handed us so it can
+	// mark the connection for close; the logging wrapper would hide it.
+	limitedBody := http.MaxBytesReader(baseWriter(w), r.Body, maxUploadSize)
 	written, err := io.Copy(file, limitedBody)
 	if err != nil {
 		file.Close()
@@ -200,23 +274,39 @@ func handleUpload(w http.ResponseWriter, r *http.Request, start time.Time) {
 		os.Remove(targetDir)
 		if isMaxBytesError(err) {
 			http.Error(w, fmt.Sprintf("File exceeds maximum upload size of %d bytes", maxUploadSize), http.StatusRequestEntityTooLarge)
-			logRequest(r, 413, time.Since(start))
 			return
 		}
+		log.Printf("Failed to write %q: %v", targetPath, err)
 		http.Error(w, "Failed to write file", http.StatusInternalServerError)
-		logRequest(r, 500, time.Since(start))
 		return
 	}
 
-	proto := getProtocol(r)
-	host := getHost(r)
-	downloadURL := fmt.Sprintf("%s://%s/%s/%s\n", proto, host, id, filename)
+	link := downloadURL(r, id, filename)
 
+	w.Header().Set("Content-Type", "text/plain; charset=utf-8")
 	w.WriteHeader(http.StatusOK)
-	w.Write([]byte(downloadURL))
+	w.Write([]byte(link + "\n"))
 
-	log.Printf("Uploaded %s (%d bytes) -> %s", filename, written, downloadURL)
-	logRequest(r, 200, time.Since(start))
+	log.Printf("Uploaded %q (%d bytes) -> %s", filename, written, link)
+}
+
+// safeFilename reduces an untrusted name to its base component and reports
+// whether the result is safe to join onto the upload directory. Note that
+// filepath.IsLocal alone is not enough: it accepts ".", and filepath.Base
+// turns an empty name into "." as well.
+func safeFilename(name string) (string, bool) {
+	base := filepath.Base(name)
+	if base == "." || base == ".." || !filepath.IsLocal(base) {
+		return "", false
+	}
+	return base, true
+}
+
+// downloadURL builds the externally visible URL for a stored file. The
+// filename is percent-escaped so that spaces, '#' and '?' survive the round
+// trip back through handleDownload.
+func downloadURL(r *http.Request, id, filename string) string {
+	return fmt.Sprintf("%s://%s/%s/%s", getProtocol(r), getHost(r), id, url.PathEscape(filename))
 }
 
 func writeMultipartError(w http.ResponseWriter, status int, message string) {
@@ -225,94 +315,130 @@ func writeMultipartError(w http.ResponseWriter, status int, message string) {
 	json.NewEncoder(w).Encode(map[string]string{"error": message})
 }
 
-func handleMultipartUpload(w http.ResponseWriter, r *http.Request, start time.Time) {
-	r.Body = http.MaxBytesReader(w, r.Body, maxUploadSize)
+// uploadResponse is the success body for POST /upload. Failed carries the
+// names of parts that could not be stored, so a partial failure is visible to
+// the client instead of silently yielding a short urls list.
+type uploadResponse struct {
+	URLs   []string `json:"urls"`
+	Failed []string `json:"failed,omitempty"`
+}
+
+func handleMultipartUpload(w http.ResponseWriter, r *http.Request) {
+	// See handleUpload: MaxBytesReader must see the original ResponseWriter.
+	r.Body = http.MaxBytesReader(baseWriter(w), r.Body, maxUploadSize)
 
 	memoryLimit := maxUploadSize
 	if memoryLimit > maxMultipartMemory {
 		memoryLimit = maxMultipartMemory
 	}
 
+	// Registered before ParseMultipartForm: a parse failure can still leave
+	// spilled temporary files behind, and the error paths below return early.
+	defer func() {
+		if r.MultipartForm != nil {
+			r.MultipartForm.RemoveAll()
+		}
+	}()
+
 	if err := r.ParseMultipartForm(memoryLimit); err != nil {
 		if isMaxBytesError(err) {
 			writeMultipartError(w, http.StatusRequestEntityTooLarge, fmt.Sprintf("Upload exceeds maximum size of %d bytes", maxUploadSize))
-			logRequest(r, 413, time.Since(start))
 			return
 		}
 		writeMultipartError(w, http.StatusBadRequest, "Failed to parse multipart form")
-		logRequest(r, 400, time.Since(start))
 		return
 	}
-	defer r.MultipartForm.RemoveAll()
 
 	files := r.MultipartForm.File["files"]
 	if len(files) == 0 {
 		writeMultipartError(w, http.StatusBadRequest, "No files provided")
-		logRequest(r, 400, time.Since(start))
+		return
+	}
+	if len(files) > maxFilesPerUpload {
+		writeMultipartError(w, http.StatusBadRequest, fmt.Sprintf("Too many files: %d, maximum is %d", len(files), maxFilesPerUpload))
 		return
 	}
 
-	proto := getProtocol(r)
-	host := getHost(r)
-	urls := make([]string, 0, len(files))
+	resp := uploadResponse{URLs: make([]string, 0, len(files))}
 
 	for _, fileHeader := range files {
-		src, err := fileHeader.Open()
-		if err != nil {
+		filename, ok := safeFilename(fileHeader.Filename)
+		if !ok {
+			log.Printf("Rejected upload part with invalid filename %q", fileHeader.Filename)
+			resp.Failed = append(resp.Failed, fileHeader.Filename)
 			continue
 		}
 
-		id, err := generateUniqueID()
+		link, err := storeUploadedFile(r, fileHeader, filename)
 		if err != nil {
-			src.Close()
+			log.Printf("Failed to store uploaded file %q: %v", filename, err)
+			resp.Failed = append(resp.Failed, filename)
 			continue
 		}
+		resp.URLs = append(resp.URLs, link)
+	}
 
-		filename := filepath.Base(fileHeader.Filename)
-		targetDir := filepath.Join(uploadDir, id)
-		targetPath := filepath.Join(targetDir, filename)
-
-		dst, err := os.Create(targetPath)
-		if err != nil {
-			src.Close()
-			os.Remove(targetDir)
-			continue
-		}
-
-		written, err := io.Copy(dst, src)
-		dst.Close()
-		src.Close()
-
-		if err != nil {
-			os.Remove(targetPath)
-			os.Remove(targetDir)
-			continue
-		}
-
-		downloadURL := fmt.Sprintf("%s://%s/%s/%s", proto, host, id, filename)
-		urls = append(urls, downloadURL)
-		log.Printf("Uploaded %s (%d bytes) -> %s", filename, written, downloadURL)
+	// Every part failed: this is a server-side problem, not a 200.
+	if len(resp.URLs) == 0 {
+		writeMultipartError(w, http.StatusInternalServerError, "Failed to store uploaded files")
+		return
 	}
 
 	w.Header().Set("Content-Type", "application/json")
-	json.NewEncoder(w).Encode(map[string][]string{"urls": urls})
-	logRequest(r, 200, time.Since(start))
+	json.NewEncoder(w).Encode(resp)
 }
 
-func handleDownload(w http.ResponseWriter, r *http.Request, start time.Time) {
+// storeUploadedFile writes one multipart part to its own upload directory and
+// returns the download URL. Any partially written file or directory is removed
+// before returning an error.
+func storeUploadedFile(r *http.Request, fileHeader *multipart.FileHeader, filename string) (string, error) {
+	src, err := fileHeader.Open()
+	if err != nil {
+		return "", err
+	}
+	defer src.Close()
+
+	id, err := generateUniqueID()
+	if err != nil {
+		return "", err
+	}
+
+	targetDir := filepath.Join(uploadDir, id)
+	targetPath := filepath.Join(targetDir, filename)
+
+	dst, err := os.Create(targetPath)
+	if err != nil {
+		os.Remove(targetDir)
+		return "", err
+	}
+
+	written, err := io.Copy(dst, src)
+	closeErr := dst.Close()
+	if err == nil {
+		err = closeErr
+	}
+	if err != nil {
+		os.Remove(targetPath)
+		os.Remove(targetDir)
+		return "", err
+	}
+
+	link := downloadURL(r, id, filename)
+	log.Printf("Uploaded %q (%d bytes) -> %s", filename, written, link)
+	return link, nil
+}
+
+func handleDownload(w http.ResponseWriter, r *http.Request) {
 	parts := strings.Split(strings.TrimPrefix(r.URL.Path, "/"), "/")
 	if len(parts) != 2 {
 		http.NotFound(w, r)
-		logRequest(r, 404, time.Since(start))
 		return
 	}
 
-	id := parts[0]
-	filename := parts[1]
-
-	if !filepath.IsLocal(id) || !filepath.IsLocal(filename) {
+	id, idOK := safeFilename(parts[0])
+	filename, nameOK := safeFilename(parts[1])
+	if !idOK || !nameOK {
 		http.NotFound(w, r)
-		logRequest(r, 404, time.Since(start))
 		return
 	}
 
@@ -320,24 +446,37 @@ func handleDownload(w http.ResponseWriter, r *http.Request, start time.Time) {
 	file, err := os.Open(filePath)
 	if err != nil {
 		http.NotFound(w, r)
-		logRequest(r, 404, time.Since(start))
 		return
 	}
 	defer file.Close()
 
 	stat, err := file.Stat()
 	if err != nil {
+		log.Printf("Failed to stat %q: %v", filePath, err)
 		http.Error(w, "Failed to stat file", http.StatusInternalServerError)
-		logRequest(r, 500, time.Since(start))
+		return
+	}
+	// A directory would otherwise be served as a zero-length file.
+	if stat.IsDir() {
+		http.NotFound(w, r)
 		return
 	}
 
+	// Set Content-Type explicitly so ServeContent does not sniff the body.
+	// The extension lookup relies on the system MIME database, which the
+	// Dockerfile provides via the mailcap package.
 	w.Header().Set("Content-Type", detectContentType(filename))
-	w.Header().Set("Content-Disposition", mime.FormatMediaType("inline", map[string]string{"filename": filename}))
-	w.Header().Set("Content-Length", fmt.Sprintf("%d", stat.Size()))
+	// FormatMediaType returns "" for names it cannot encode; don't emit an
+	// empty header in that case.
+	if disposition := mime.FormatMediaType("inline", map[string]string{"filename": filename}); disposition != "" {
+		w.Header().Set("Content-Disposition", disposition)
+	}
+	// Download URLs are unguessable secrets, so keep shared caches out of it.
+	w.Header().Set("Cache-Control", "private")
 
-	io.Copy(w, file)
-	logRequest(r, 200, time.Since(start))
+	// ServeContent handles Range, If-Modified-Since, Content-Length and HEAD,
+	// which makes interrupted downloads resumable with `curl -C -`.
+	http.ServeContent(w, r, filename, stat.ModTime(), file)
 }
 
 func generateUniqueID() (string, error) {
@@ -437,22 +576,105 @@ func detectContentType(filename string) string {
 	return "application/octet-stream"
 }
 
-func logRequest(r *http.Request, status int, duration time.Duration) {
-	log.Printf("[%s] %s %s %d %v", time.Now().Format("2006-01-02 15:04:05"), r.Method, r.URL.Path, status, duration)
+// responseRecorder captures the status code and byte count actually written,
+// so the access log reports what happened rather than what the handler
+// intended. http.ServeContent in particular may answer 206 or 304.
+type responseRecorder struct {
+	http.ResponseWriter
+	status  int
+	written int64
+}
+
+func (rr *responseRecorder) WriteHeader(status int) {
+	if rr.status == 0 {
+		rr.status = status
+	}
+	rr.ResponseWriter.WriteHeader(status)
+}
+
+func (rr *responseRecorder) Write(b []byte) (int, error) {
+	if rr.status == 0 {
+		rr.status = http.StatusOK
+	}
+	n, err := rr.ResponseWriter.Write(b)
+	rr.written += int64(n)
+	return n, err
+}
+
+// Unwrap lets http.ResponseController and baseWriter reach the original
+// ResponseWriter.
+func (rr *responseRecorder) Unwrap() http.ResponseWriter { return rr.ResponseWriter }
+
+// baseWriter returns the ResponseWriter the net/http server created. It is
+// needed by http.MaxBytesReader, which type-asserts its argument to an
+// unexported interface to mark an oversized request's connection for close;
+// that assertion fails against a wrapper.
+func baseWriter(w http.ResponseWriter) http.ResponseWriter {
+	for {
+		u, ok := w.(interface{ Unwrap() http.ResponseWriter })
+		if !ok {
+			return w
+		}
+		w = u.Unwrap()
+	}
+}
+
+// withLogging records one access-log line per request, including for handlers
+// that never write an explicit status.
+func withLogging(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		start := time.Now()
+		rec := &responseRecorder{ResponseWriter: w}
+
+		next.ServeHTTP(rec, r)
+
+		status := rec.status
+		if status == 0 {
+			status = http.StatusOK
+		}
+		// %q on the path: it is attacker-controlled and would otherwise let a
+		// request forge log lines by embedding newlines.
+		log.Printf("%s %q %d %d %v", r.Method, r.URL.Path, status, rec.written, time.Since(start))
+	})
 }
 
 func serveFrontpage(w http.ResponseWriter, r *http.Request) {
-	proto := getProtocol(r)
-	host := getHost(r)
 	pageData := map[string]any{
-		"Proto":         proto,
-		"Host":          host,
+		"Proto":         getProtocol(r),
+		"Host":          getHost(r),
 		"MaxUploadSize": maxUploadSize,
+		"Expiry":        formatExpiry(fileExpires),
 		"Version":       version,
 	}
 
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
-	indexHTML.Execute(w, pageData)
+	if err := indexHTML.Execute(w, pageData); err != nil {
+		log.Printf("Failed to render front page: %v", err)
+	}
+}
+
+// formatExpiry renders a retention duration for the front page.
+// Duration.String() would produce "168h0m0s" for the default week.
+func formatExpiry(d time.Duration) string {
+	switch {
+	case d <= 0:
+		return "never"
+	case d%(24*time.Hour) == 0:
+		return plural(int(d/(24*time.Hour)), "day")
+	case d%time.Hour == 0:
+		return plural(int(d/time.Hour), "hour")
+	case d%time.Minute == 0:
+		return plural(int(d/time.Minute), "minute")
+	default:
+		return d.String()
+	}
+}
+
+func plural(n int, unit string) string {
+	if n == 1 {
+		return "1 " + unit
+	}
+	return fmt.Sprintf("%d %ss", n, unit)
 }
 
 // Cleanup
@@ -476,12 +698,51 @@ func isDirEmpty(name string) (bool, error) {
 	return false, err
 }
 
-func cleanupProcedure(duration time.Duration, dryRun bool) {
-	var oldFiles []string
-	var emptyDirs []string
+func cleanupProcedure(ctx context.Context, duration, interval time.Duration, dryRun bool) {
+	ticker := time.NewTicker(interval)
+	defer ticker.Stop()
 
-	deleteIfTooOld := func(path string, d fs.DirEntry, err error) error {
+	for {
+		cleanupCycle(duration, dryRun)
+
+		select {
+		case <-ctx.Done():
+			log.Print("Cleanup stopped")
+			return
+		case <-ticker.C:
+		}
+	}
+}
+
+// cleanupCycle performs one sweep: expired files first, then the upload
+// directories left empty behind them.
+func cleanupCycle(duration time.Duration, dryRun bool) {
+	oldFiles, err := collectExpiredFiles(uploadDir, duration)
+	if err != nil {
+		log.Printf("Cleanup: failed to scan %q for expired files: %v", uploadDir, err)
+	}
+
+	removedFiles := removeAll(oldFiles, "expired file", dryRun)
+
+	emptyDirs, err := collectEmptyDirs(uploadDir)
+	if err != nil {
+		log.Printf("Cleanup: failed to scan %q for empty folders: %v", uploadDir, err)
+	}
+
+	removedDirs := removeAll(emptyDirs, "empty folder", dryRun)
+
+	log.Printf("Cleanup finished: %d expired file(s) and %d empty folder(s) found, %d and %d removed (dryRun=%v)",
+		len(oldFiles), len(emptyDirs), removedFiles, removedDirs, dryRun)
+}
+
+// collectExpiredFiles returns files under root whose mtime is at least
+// duration old.
+func collectExpiredFiles(root string, duration time.Duration) ([]string, error) {
+	var oldFiles []string
+
+	err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
+			log.Printf("Cleanup: skipping %q: %v", path, err)
 			return nil
 		}
 		if d.IsDir() {
@@ -490,64 +751,76 @@ func cleanupProcedure(duration time.Duration, dryRun bool) {
 
 		info, err := d.Info()
 		if err != nil {
-			log.Printf("Error cleaning file '%s' : %v", d.Name(), err)
+			log.Printf("Cleanup: failed to stat %q: %v", path, err)
 			return nil
 		}
-		age := time.Since(info.ModTime())
-		if age >= duration {
+		if time.Since(info.ModTime()) >= duration {
 			oldFiles = append(oldFiles, path)
 		}
 		return nil
-	}
+	})
 
-	findEmptyDirs := func(path string, d fs.DirEntry, err error) error {
+	return oldFiles, err
+}
+
+// collectEmptyDirs returns empty directories under root, excluding root itself
+// and any directory created within newUploadGrace, which may belong to an
+// upload that has not written its file yet.
+func collectEmptyDirs(root string) ([]string, error) {
+	var emptyDirs []string
+
+	err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
+		// d is nil when the root itself cannot be read, so this check must
+		// come first.
+		if err != nil {
+			log.Printf("Cleanup: skipping %q: %v", path, err)
+			return nil
+		}
 		if !d.IsDir() {
 			return nil
 		}
-		if filepath.Clean(path) == filepath.Clean(uploadDir) {
+		if filepath.Clean(path) == filepath.Clean(root) {
 			return nil
 		}
+
+		info, err := d.Info()
+		if err != nil {
+			log.Printf("Cleanup: failed to stat %q: %v", path, err)
+			return nil
+		}
+		if time.Since(info.ModTime()) < newUploadGrace {
+			return nil
+		}
+
 		empty, err := isDirEmpty(path)
+		if err != nil {
+			log.Printf("Cleanup: failed to read %q: %v", path, err)
+			return nil
+		}
 		if empty {
 			emptyDirs = append(emptyDirs, path)
-		} else if err != nil {
-			log.Printf("Error cleaning directory '%s' : %v", path, err)
 		}
-
 		return nil
-	}
+	})
 
-	for {
-		oldFiles = oldFiles[:0]
-		emptyDirs = emptyDirs[:0]
+	return emptyDirs, err
+}
 
-		filepath.WalkDir(uploadDir, deleteIfTooOld)
-		for _, file := range oldFiles {
-			if !dryRun {
-				err := os.Remove(file)
-				if err != nil {
-					log.Printf("Failed to remove expired file '%s' : %v", file, err)
-				} else {
-					log.Printf("Removed expired file '%s'", file)
-				}
-			} else {
-				log.Printf("Dry run: Would remove expired file '%s'", file)
-			}
+// removeAll deletes the given paths unless dryRun is set, and returns how many
+// were actually removed.
+func removeAll(paths []string, kind string, dryRun bool) int {
+	removed := 0
+	for _, path := range paths {
+		if dryRun {
+			log.Printf("Dry run: Would remove %s %q", kind, path)
+			continue
 		}
-
-		filepath.WalkDir(uploadDir, findEmptyDirs)
-		for _, file := range emptyDirs {
-			if !dryRun {
-				err := os.Remove(file)
-				if err != nil {
-					log.Printf("Failed to remove empty folder '%s' : %v", file, err)
-				} else {
-					log.Printf("Removed empty folder '%s'", file)
-				}
-			} else {
-				log.Printf("Dry run: Would remove empty folder '%s'", file)
-			}
+		if err := os.Remove(path); err != nil {
+			log.Printf("Failed to remove %s %q: %v", kind, path, err)
+			continue
 		}
-		time.Sleep(6 * time.Hour)
+		log.Printf("Removed %s %q", kind, path)
+		removed++
 	}
+	return removed
 }