From 1168565ca3369c2396a1d19e1de4bcaef63de4eb Mon Sep 17 00:00:00 2001 From: ssavutu Date: Fri, 31 Jul 2026 18:05:16 -0400 Subject: [PATCH] feat(media): raise the upload cap to 90 MiB and stop buffering uploads in RAM The migrated WordPress corpus contains unresized camera originals up to ~77 MiB (largest: 2025/07/BZ9A5771.jpg), so the 25 MiB cap rejected files the newsroom demonstrably produces. Raise the default to 90 MiB, chosen to sit under Cloudflare's 100 MB request-body limit on the tunnel fronting Delta -- past that a body clears Nginx and the backend but dies at the edge with an error the CMS never sees. ParseMultipartForm was being handed the size limit as its argument, but that argument is the in-memory buffer threshold, not the cap (MaxBytesReader has always been what enforces the cap). At 90 MiB that would let a single upload pin its full size in RAM, plus the ~10 MiB of slack Go adds on top. Give it a fixed 8 MiB instead and let the rest spill to temp files; the write path already streams and only reads image headers for dimensions, so a large upload now costs container disk rather than memory. Nginx's client_max_body_size moves to 91m to stay just above the backend, and the body/proxy timeouts go to 300s so a 90 MiB upload over a slow uplink is not killed mid-flight. MEDIA_MAX_UPLOAD_BYTES is also plumbed through Compose and the env template -- it was read by the backend but never passed in, so the cap could previously only be changed by editing Go. Deploying this needs both halves: reinstall the Nginx site and reload, and recreate the backend slots. The smaller of the two limits silently wins. Co-Authored-By: Claude Opus 5 --- deploy/README.md | 17 +++++++- deploy/cms.env.example | 1 + deploy/compose.cms.yml | 4 ++ deploy/nginx/triangle-cms.conf | 13 ++++-- server/internal/handlers/media.go | 18 +++++++-- server/internal/handlers/media_test.go | 56 ++++++++++++++++++++++++++ 6 files changed, 101 insertions(+), 8 deletions(-) diff --git a/deploy/README.md b/deploy/README.md index e3f6676..adb85d6 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -114,12 +114,23 @@ Nginx will not start without `active-upstreams.conf`, since the site `include`s it unconditionally. A passing `nginx -t` *before* the site is enabled only validates the stock config and proves nothing. -The site sets `client_max_body_size 26m` on `/v1/` to sit just above the -backend's `MEDIA_MAX_UPLOAD_BYTES` (25 MiB default). Nginx's stock limit is 1m, +The site sets `client_max_body_size 91m` on `/v1/` to sit just above the +backend's `MEDIA_MAX_UPLOAD_BYTES` (90 MiB default). Nginx's stock limit is 1m, which rejects an ordinary phone photo with a 413 before the CMS ever sees it, so a deploy that skips re-copying this file leaves media uploads broken while the app looks correctly configured. Raise both together, never just one. +The 90 MiB figure is sized to the migrated corpus, which contains unresized +camera originals up to ~77 MiB (largest: `2025/07/BZ9A5771.jpg`). The hard +ceiling above it is Cloudflare's **100 MB** request-body limit on the tunnel +fronting Delta; a body that passes Nginx and the backend but exceeds that dies +at the edge with an error the CMS never sees, so do not raise the pair past +~95 MiB without moving media uploads off the tunnel. + +The backend streams uploads to a temp file rather than buffering them in RAM +(only the first 8 MiB stays in memory), so a large upload costs container disk, +not memory. + ### Media serving `location /wp-content/` reads the migrated WordPress corpus straight off CephFS. @@ -218,6 +229,8 @@ The file must contain the exact immutable image tag for the active deployment: `/mnt/cephfs/media` unless the bind-mount target changes. - `MEDIA_BASE_URL` - public origin that serves `/wp-content/`, used to build media URLs returned by the upload endpoint. Empty yields relative URLs. +- `MEDIA_MAX_UPLOAD_BYTES` - per-file upload cap in bytes. Empty uses the + 90 MiB default. Must stay at or below Nginx's `client_max_body_size`. Keep `CMS_AUTO_PROMOTE_ALL_ADMINS=false` and `CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP=false` in production. Rebuild taxonomy diff --git a/deploy/cms.env.example b/deploy/cms.env.example index bb8e654..eabf8e6 100644 --- a/deploy/cms.env.example +++ b/deploy/cms.env.example @@ -19,3 +19,4 @@ AKISMET_BLOG_URL= MEDIA_HOST_PATH= MEDIA_ROOT= MEDIA_BASE_URL= +MEDIA_MAX_UPLOAD_BYTES= diff --git a/deploy/compose.cms.yml b/deploy/compose.cms.yml index a079034..b8e470e 100644 --- a/deploy/compose.cms.yml +++ b/deploy/compose.cms.yml @@ -35,6 +35,10 @@ x-backend-base: &backend-base # assets under MEDIA_ROOT; MEDIA_BASE_URL is the public host that serves them. MEDIA_ROOT: ${MEDIA_ROOT:-/mnt/cephfs/media} MEDIA_BASE_URL: ${MEDIA_BASE_URL:-} + # Per-file upload cap. Must stay at or below Nginx's client_max_body_size + # (91m) and below Cloudflare's 100 MB tunnel limit; raise all of them + # together or the smallest one silently wins. + MEDIA_MAX_UPLOAD_BYTES: ${MEDIA_MAX_UPLOAD_BYTES:-94371840} volumes: # CephFS media tree (host). rw so the upload endpoint can store new files; # host Nginx serves the same tree read-only (see deploy/nginx/triangle-cms.conf). diff --git a/deploy/nginx/triangle-cms.conf b/deploy/nginx/triangle-cms.conf index 67f4a4a..4bd4bcd 100644 --- a/deploy/nginx/triangle-cms.conf +++ b/deploy/nginx/triangle-cms.conf @@ -39,12 +39,19 @@ server { } location /v1/ { - # POST /v1/media accepts up to MEDIA_MAX_UPLOAD_BYTES (25 MiB by - # default). Nginx's own default is 1m, so without this it rejects any + # POST /v1/media accepts up to MEDIA_MAX_UPLOAD_BYTES (90 MiB by + # default -- the legacy corpus has full-res camera originals near 77 + # MiB). Nginx's own default is 1m, so without this it rejects any # ordinary phone photo with a 413 before the request reaches the CMS, # and the app-side limit never gets a say. Keep this at or above the # backend's limit so the backend is the one that decides. - client_max_body_size 26m; + client_max_body_size 91m; + # A 90 MiB body over a slow uplink takes minutes; the stock 60s applies + # per read, but raise the ceilings so a stalled-but-alive upload is not + # killed mid-flight. + client_body_timeout 300s; + proxy_read_timeout 300s; + proxy_send_timeout 300s; proxy_pass $triangle_cms_backend; } diff --git a/server/internal/handlers/media.go b/server/internal/handlers/media.go index 5326cb4..615c0eb 100644 --- a/server/internal/handlers/media.go +++ b/server/internal/handlers/media.go @@ -33,8 +33,18 @@ import ( const ( mediaFormField = "file" - // defaultMaxUploadBytes caps a single upload when MEDIA_MAX_UPLOAD_BYTES is unset. - defaultMaxUploadBytes int64 = 25 << 20 // 25 MiB + // defaultMaxUploadBytes caps a single upload when MEDIA_MAX_UPLOAD_BYTES is + // unset. The migrated WordPress corpus contains full-resolution camera + // originals up to ~77 MiB, so anything smaller than this rejects files the + // newsroom demonstrably produces. The ceiling is Cloudflare's 100 MB request + // limit on the tunnel in front of Delta -- stay below it, since a body that + // clears the backend but not the tunnel fails with an opaque edge error. + defaultMaxUploadBytes int64 = 90 << 20 // 90 MiB + // multipartMemoryBytes is how much of a multipart body ParseMultipartForm may + // hold in RAM before spilling the rest to temp files. It is deliberately + // decoupled from the size limit: passing the limit itself would let a single + // upload pin its full size in memory (and Go adds ~10 MiB of slack on top). + multipartMemoryBytes int64 = 8 << 20 // 8 MiB // uploadsSubdir is the WordPress-compatible prefix (under MEDIA_ROOT) that all // uploads live under, so new files line up with the rsynced legacy corpus and // the Nginx `location /wp-content/` root that serves them. @@ -111,7 +121,9 @@ func PostMedia(conn *sql.DB) http.Handler { // Cap the request body. A little slack over maxBytes covers multipart // framing so a file exactly at the limit still succeeds. r.Body = http.MaxBytesReader(w, r.Body, maxBytes+4096) - if err := r.ParseMultipartForm(maxBytes); err != nil { + // MaxBytesReader, not this argument, is what enforces the limit; this only + // decides where the bytes land on the way in (see multipartMemoryBytes). + if err := r.ParseMultipartForm(multipartMemoryBytes); err != nil { var tooLarge *http.MaxBytesError if errors.As(err, &tooLarge) { writeError(w, http.StatusRequestEntityTooLarge, diff --git a/server/internal/handlers/media_test.go b/server/internal/handlers/media_test.go index 266885f..ac99ab5 100644 --- a/server/internal/handlers/media_test.go +++ b/server/internal/handlers/media_test.go @@ -12,6 +12,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strconv" "strings" "testing" @@ -69,6 +70,61 @@ func TestPostMedia_NotConfigured(t *testing.T) { } } +func TestPostMedia_RejectsOversizeUpload(t *testing.T) { + t.Setenv("MEDIA_ROOT", t.TempDir()) + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "1024") + rec := httptest.NewRecorder() + // Comfortably past the limit plus the multipart-framing slack, so this is + // the size check firing and not a boundary-arithmetic accident. + PostMedia(nil).ServeHTTP(rec, uploadRequest(t, "big.png", bytes.Repeat([]byte("a"), 64<<10))) + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want 413; body = %s", rec.Code, rec.Body.String()) + } +} + +// The size limit is enforced by MaxBytesReader, not by ParseMultipartForm's +// argument -- that one only decides how much stays in RAM before spilling to a +// temp file. A body well past multipartMemoryBytes but under the limit must +// therefore parse normally; reaching the 415 content-type check proves it did. +func TestPostMedia_AcceptsBodyLargerThanMultipartMemory(t *testing.T) { + t.Setenv("MEDIA_ROOT", t.TempDir()) + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", strconv.FormatInt(multipartMemoryBytes*4, 10)) + rec := httptest.NewRecorder() + PostMedia(nil).ServeHTTP(rec, uploadRequest(t, "notes.txt", bytes.Repeat([]byte("a"), int(multipartMemoryBytes)+1<<20))) + if rec.Code != http.StatusUnsupportedMediaType { + t.Fatalf("status = %d, want 415 (413 means the size cap fired early); body = %s", rec.Code, rec.Body.String()) + } +} + +func TestMaxUploadBytes(t *testing.T) { + // The migrated WordPress corpus holds camera originals near 77 MiB, so a + // default below that rejects files the newsroom already has. + if defaultMaxUploadBytes < 80<<20 { + t.Fatalf("defaultMaxUploadBytes = %d, too small for the legacy corpus", defaultMaxUploadBytes) + } + + t.Run("default when unset", func(t *testing.T) { + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "") + if got := maxUploadBytes(); got != defaultMaxUploadBytes { + t.Fatalf("maxUploadBytes = %d, want %d", got, defaultMaxUploadBytes) + } + }) + + t.Run("env override", func(t *testing.T) { + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "4096") + if got := maxUploadBytes(); got != 4096 { + t.Fatalf("maxUploadBytes = %d, want 4096", got) + } + }) + + t.Run("falls back on garbage", func(t *testing.T) { + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "90MiB") + if got := maxUploadBytes(); got != defaultMaxUploadBytes { + t.Fatalf("maxUploadBytes = %d, want the default %d", got, defaultMaxUploadBytes) + } + }) +} + func TestPostMediaIndex_NotConfigured(t *testing.T) { t.Setenv("MEDIA_ROOT", "") rec := httptest.NewRecorder()