From 9b82c06d927b30f0a12c15080f99a4a12c8c56df Mon Sep 17 00:00:00 2001 From: Mathias Date: Fri, 2 Oct 2026 16:47:49 +0200 Subject: [PATCH] fix(web): 429 for a busy queue; a non-UUID id and a malformed or odd-byte title are 4xx or cleaned, never 500 (review F6, F7) Co-Authored-By: Claude Sonnet 5.5 --- internal/web/uploads.go | 57 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/internal/web/uploads.go b/internal/web/uploads.go index b139846..e956896 100644 --- a/internal/web/uploads.go +++ b/internal/web/uploads.go @@ -8,8 +8,10 @@ import ( "math" "net/http" "path/filepath" + "regexp" "strings" "time" + "unicode" "unicode/utf8" "github.com/a-h/templ" @@ -92,13 +94,26 @@ func (a *App) handleUploadPost(w http.ResponseWriter, r *http.Request) { break } if err != nil { + var tooBig *http.MaxBytesError + if !errors.As(err, &tooBig) { + // A malformed multipart body (a header with a control byte, a truncated + // part) is the sender's to fix: a 400, not a server error, and the error + // text is never logged: it quotes the headers, which carry the file name. + a.logger().Info("upload rejected", "reason", "malformed") + a.renderUploadsPage(w, r, userID, http.StatusBadRequest, "That upload could not be read. Please try again.") + return + } a.uploadFailure(w, r, userID, err) return } switch part.FormName() { case "title": - b, _ := io.ReadAll(io.LimitReader(part, maxTitleRunes*utf8.UTFMax)) - title = clipRunes(strings.TrimSpace(string(b)), maxTitleRunes) + b, err := io.ReadAll(io.LimitReader(part, maxTitleRunes*utf8.UTFMax)) + if err != nil { + a.renderUploadsPage(w, r, userID, http.StatusBadRequest, "That upload could not be read. Please try again.") + return + } + title = cleanTitle(string(b)) case "file": if title == "" { title = titleFromFilename(part.FileName()) @@ -126,8 +141,11 @@ func (a *App) uploadFailure(w http.ResponseWriter, r *http.Request, userID strin // Logged by reason only: the filename and title are private content. a.logger().Info("upload rejected", "reason", string(rej.Reason)) status := http.StatusUnprocessableEntity - if rej.Reason == uploads.ReasonTooLarge { + switch rej.Reason { + case uploads.ReasonTooLarge: status = http.StatusRequestEntityTooLarge + case uploads.ReasonBusy: + status = http.StatusTooManyRequests } a.renderUploadsPage(w, r, userID, status, rej.Message) case errors.As(err, &tooBig): @@ -168,7 +186,14 @@ func (a *App) loadOwnedUpload(w http.ResponseWriter, r *http.Request) (*domain.U if !ok { return nil, false } - u, err := a.Store.GetUpload(r.Context(), userID, r.PathValue("id")) + id := r.PathValue("id") + if !uuidShape.MatchString(id) { + // Not an id at all: the same 404 as an unknown or foreign one, never a database + // error that surfaces as a 500 (and no oracle about which ids exist). + http.NotFound(w, r) + return nil, false + } + u, err := a.Store.GetUpload(r.Context(), userID, id) if errors.Is(err, store.ErrUploadNotFound) { http.NotFound(w, r) return nil, false @@ -183,11 +208,29 @@ func (a *App) loadOwnedUpload(w http.ResponseWriter, r *http.Request) (*domain.U // titleFromFilename is the default title: the file's name without its extension. func titleFromFilename(name string) string { base := filepath.Base(name) - t := strings.TrimSuffix(base, filepath.Ext(base)) + t := cleanTitle(strings.TrimSuffix(base, filepath.Ext(base))) if t == "" || t == "." { return "Untitled recording" } - return clipRunes(t, maxTitleRunes) + return t +} + +// uuidShape is the form of an upload id. Anything else cannot be one, so it is a 404. +var uuidShape = regexp.MustCompile(`^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$`) + +// cleanTitle makes a title safe to store and show: invalid UTF-8 dropped, control +// characters (NUL, newlines, escapes) turned into spaces, runs of space collapsed, trimmed +// and clipped. A title is cosmetic; bad bytes in it must never fail an upload, and Postgres +// refuses NUL in text. +func cleanTitle(s string) string { + s = strings.ToValidUTF8(s, "") + s = strings.Map(func(r rune) rune { + if unicode.IsControl(r) { + return ' ' + } + return r + }, s) + return clipRunes(strings.Join(strings.Fields(s), " "), maxTitleRunes) } func clipRunes(s string, n int) string { @@ -203,6 +246,8 @@ func uploadFailureMessage(k domain.UploadErrorKind) string { switch k { case domain.UploadErrInterrupted: return "The server restarted before this recording was transcribed. Please upload it again." + case domain.UploadErrBusy: + return "Too many recordings were waiting to be transcribed when this one was uploaded. Please upload it again in a few minutes." case domain.UploadErrTooLong: return "This recording is longer than the limit once its audio is read (the file's header said otherwise). Please upload a shorter recording." case domain.UploadErrIncomplete: