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()