From 4957e5ce255947bea04d1d9ba32cac227e775488 Mon Sep 17 00:00:00 2001
From: Jakub Zych <jakub@keios.eu>
Date: Wed, 28 Oct 2026 21:12:46 +0100
Subject: [PATCH] refactor(surf): take the uploads inline types from
 attach.IsInlineContentType (quick-261028-rqe)

- the route's private inline list and its helper are gone; the
  disposition writer asks attach.IsInlineContentType, the same
  definition attach.Store uses for the disposition it stores
- headers are unchanged (TestUploadsResponseHeaders, TestUploadsMount)
- surf README and the storage page name the shared function
---
 docs/services/storage.md |  2 +-
 modules/surf/README.md   |  2 +-
 modules/surf/uploads.go  | 39 +++++++++++++--------------------------
 3 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/docs/services/storage.md b/docs/services/storage.md
index d8256d07..4c603c94 100644
--- a/docs/services/storage.md
+++ b/docs/services/storage.md
@@ -111,7 +111,7 @@ The route serves originals and thumbnails and answers 404 for a missing file, an
 
 - Every response carries `X-Content-Type-Options: nosniff`.
 - Every response carries `Content-Security-Policy: default-src 'none'; img-src 'self'; media-src 'self'; sandbox`. A file opened as a page runs no script and gets an opaque origin, so it cannot read the application's cookies or storage. The policy does not affect an `<img>` or `<video>` that embeds the file.
-- A file whose type is not a raster image (`image/jpeg`, `image/png`, `image/gif`, `image/webp` or `image/avif`) is sent with `Content-Disposition: attachment`, so the browser downloads it instead of showing it. That includes HTML, SVG, PDF and plain text.
+- A file whose type is not a raster image (`image/jpeg`, `image/png`, `image/gif`, `image/webp` or `image/avif`) is sent with `Content-Disposition: attachment`, so the browser downloads it instead of showing it. That includes HTML, SVG, PDF and plain text. The list is `attach.IsInlineContentType`, the same one `attach.Store` uses for the disposition it stores with the file (see the [deploy note](#deploy-note)).
 
 With `serve` on, `public_path_prefix` must be an absolute URL path such as `/storage/uploads`: a leading slash, no scheme or host, no query, no empty, `.` or `..` segment and no `{` or `}`. Start-up fails with an error naming the key otherwise, for example for the WinterCMS disk path `storage/app/uploads/public` or a CDN URL. With `serve` off the key only builds file URLs, so a CDN origin is fine there.
 
diff --git a/modules/surf/README.md b/modules/surf/README.md
index e46eab6e..e45c0c25 100644
--- a/modules/surf/README.md
+++ b/modules/surf/README.md
@@ -36,7 +36,7 @@ Use cases:
 - Path-scoped CORS configured with the same keys as Laravel's `config/cors.php` (`surf.CORSConfig`), including preflight handling. `surf.LoadCORSConfig` refuses credentials with a `*` origin and origin patterns that do not compile; an allowed origin is echoed exactly, with `Vary: Origin` on every response whose answer depends on the request origin. Every `OPTIONS` request on a CORS path is answered with 204 before routing, with the headers Laravel's `HandleCors` sends: `Cache-Control: no-cache, private` always and, on a preflight (an `Origin` and an `Access-Control-Request-Method`), the requested method (upper-cased) and headers echoed in `Access-Control-Allow-Methods` and `Access-Control-Allow-Headers` when `*` allows any, `Vary` on the request headers and PHP's default `Content-Type: text/html; charset=UTF-8`.
 - Optional `surf.LocaleResolver`: a compiled plugin publishes the interface through backpack; surf looks it up without importing the plugin. When present, it may strip an enabled locale prefix before routing and writes a validated code with `towel.WithLocale`. When absent, hosts keep the raw `Accept-Language` header and `surf.LocaleFromPrincipal` overlay.
 - `surf.LocaleFromPrincipal` switches the request locale to the signed-in user's preferred locale.
-- Opt-in uploads route: with `storage.uploads.serve` true, `surf.BuildRouter` mounts `GET <storage.uploads.public_path_prefix>/{path...}` (owned by `summercms.surf`) on `attach.StaticHandlerPublic`, so a file is served only when its `system_files` row is public. Every response carries `X-Content-Type-Options: nosniff` and a sandboxing `Content-Security-Policy`, and a file whose type is not a raster image (JPEG, PNG, GIF, WebP, AVIF) is sent with `Content-Disposition: attachment`, so a stored HTML or SVG file never renders on the app origin. The bucket and the database are looked up per request, so `route:list` lists the route without opening either; until `serve` publishes them it answers 404. The route owns the whole prefix: `surf.BuildRouter` fails when a plugin route sits at or under it, and when the prefix is not an absolute URL path (no leading slash, a scheme or host, a query, an empty, `.` or `..` segment, or `{`/`}`).
+- Opt-in uploads route: with `storage.uploads.serve` true, `surf.BuildRouter` mounts `GET <storage.uploads.public_path_prefix>/{path...}` (owned by `summercms.surf`) on `attach.StaticHandlerPublic`, so a file is served only when its `system_files` row is public. Every response carries `X-Content-Type-Options: nosniff` and a sandboxing `Content-Security-Policy`, and a file whose type is not a raster image (JPEG, PNG, GIF, WebP, AVIF; the shared definition is `attach.IsInlineContentType`, the same one `attach.Store` uses for the disposition it stores with each file) is sent with `Content-Disposition: attachment`, so a stored HTML or SVG file never renders on the app origin. The bucket and the database are looked up per request, so `route:list` lists the route without opening either; until `serve` publishes them it answers 404. The route owns the whole prefix: `surf.BuildRouter` fails when a plugin route sits at or under it, and when the prefix is not an absolute URL path (no leading slash, a scheme or host, a query, an empty, `.` or `..` segment, or `{`/`}`).
 - Opt-in API documentation page: with `http.api_docs.enabled` true, `surf.BuildRouter` mounts `GET /api-docs` (a Swagger UI page) and `GET /api-docs.json` (owned by `summercms.apidocs`, public, no middleware). The spec comes from the one plugin implementing `surf.APIDocsProvider`, as YAML or JSON, converted once at boot to compact JSON in source key order; zero or several providers, or a spec that does not convert, fail boot. The page loads swagger-ui-dist `surf.SwaggerUIVersion` from jsDelivr with Subresource Integrity hashes on all three assets and a `Content-Security-Policy` whose only script source is a per-request nonce; every script tag, the two jsDelivr scripts included, carries it, so no other jsDelivr script can run.
 - Request log, on by default: every request writes one Info record `http request` from the [diary](../diary/README.md) component `http` after it is served, with `method`, `pattern` (the matched route, such as `GET /posts/{id}`), `status`, `duration_ms` and `request_id` (the `X-Request-Id` header, trimmed and cut to 128 bytes, left out when empty). A request whose client went away adds `client_gone=true`; its 5xx is logged as 499 (client closed request) only when the client had already gone when the status was written and the handler did not panic, so a 5xx written before the cancellation and a recovered panic keep their real status. An unmatched request (a 404 or 405 from the mux, or a CORS preflight answered before routing) has an empty `pattern`. The URL path, the query string, headers and the client address are never logged, for an unmatched request too: a token route requested with another method answers 405 before any route handler runs. Records go to the app's published `*slog.Logger`, else the `http` component logger, whose channels all redact through [sunscreen](../sunscreen/README.md). Silence it with `log.components.http: warn` or turn it off with `http.access_log: false`.
 - Startup summary and shutdown report for `serve`: once it listens, a block on a terminal (version, environment, URL and admin URL, database host and name, pending migrations, plugins, queue, scheduler, mail, cache, realtime and search drivers; `-v` adds one line per plugin) and, off a terminal, the same facts as a `startup summary` record and a `listening on <addr>` record of component `serve`; after SIGINT or SIGTERM, a report of requests served, drained and aborted, worker jobs, uptime and drain errors. Never a DSN user or password.
diff --git a/modules/surf/uploads.go b/modules/surf/uploads.go
index 80ef382a..b190482a 100644
--- a/modules/surf/uploads.go
+++ b/modules/surf/uploads.go
@@ -2,7 +2,6 @@ package surf
 
 import (
 	"fmt"
-	"mime"
 	"net/http"
 	"strings"
 
@@ -27,26 +26,20 @@ const defaultUploadsPrefix = "/storage/uploads"
 // embeds the file.
 const uploadsContentSecurityPolicy = "default-src 'none'; img-src 'self'; media-src 'self'; sandbox"
 
-// uploadsInlineTypes are the content types the uploads route lets a browser
-// show inline: raster images. Every other successful response, SVG and PDF
-// included, is served with Content-Disposition: attachment.
-var uploadsInlineTypes = map[string]bool{
-	"image/avif": true,
-	"image/gif":  true,
-	"image/jpeg": true,
-	"image/png":  true,
-	"image/webp": true,
-}
-
 // mountUploads registers GET {storage.uploads.public_path_prefix}/{path...}
 // when storage.uploads.serve is true. The handler delegates to
 // attach.StaticHandlerPublic, so missing rows and is_public=false rows 404.
 // Because the files are served on the app origin, every response carries
 // X-Content-Type-Options: nosniff and uploadsContentSecurityPolicy, and a
-// file whose stored type is not an inline raster image (uploadsInlineTypes)
-// is served as an attachment: attach.Store records the type sniffed from
-// the bytes, so an uploaded text file that starts with <html> is text/html. The bucket and the database are looked up per request:
-// boot and route:list build the router before serve publishes either.
+// file whose stored type is not an inline raster image
+// (attach.IsInlineContentType) is served as an attachment: attach.Store
+// records the type sniffed from the bytes, so an uploaded text file that
+// starts with <html> is text/html. attach.Store also stores the attachment
+// disposition with the blob, but the route still decides from the type on
+// every response: attach.StaticHandlerPublic sends only the stored type, and
+// blobs written earlier or outside attach.Store carry no disposition. The
+// bucket and the database are looked up per request: boot and route:list
+// build the router before serve publishes either.
 // The route owns the whole prefix: a plugin route at or under it fails
 // BuildRouter, as a route under the admin prefix does. With the key false
 // or absent nothing is registered and nothing is checked.
@@ -125,9 +118,9 @@ func uploadsHandler(app *backpack.App, pages *errorPages, prefix string) http.Ha
 
 // serveUploads runs next with the uploads response headers: nosniff and the
 // sandboxing CSP always, and Content-Disposition: attachment on a successful
-// response whose Content-Type is not in uploadsInlineTypes. The disposition
-// is decided when the status is written (or, for a HEAD that writes
-// nothing, when next returns), after next has set the Content-Type.
+// response whose Content-Type attach.IsInlineContentType rejects. The
+// disposition is decided when the status is written (or, for a HEAD that
+// writes nothing, when next returns), after next has set the Content-Type.
 func serveUploads(w http.ResponseWriter, req *http.Request, next http.Handler) {
 	w.Header().Set("X-Content-Type-Options", "nosniff")
 	w.Header().Set("Content-Security-Policy", uploadsContentSecurityPolicy)
@@ -148,7 +141,7 @@ func (w *uploadsDispositionWriter) decide(status int) {
 		return
 	}
 	w.decided = true
-	if status >= 200 && status < 300 && !uploadsInline(w.Header().Get("Content-Type")) {
+	if status >= 200 && status < 300 && !attach.IsInlineContentType(w.Header().Get("Content-Type")) {
 		w.Header().Set("Content-Disposition", "attachment")
 	}
 }
@@ -164,9 +157,3 @@ func (w *uploadsDispositionWriter) Write(p []byte) (int, error) {
 }
 
 func (w *uploadsDispositionWriter) Unwrap() http.ResponseWriter { return w.ResponseWriter }
-
-// uploadsInline reports whether contentType is an inline raster image.
-func uploadsInline(contentType string) bool {
-	mediaType, _, err := mime.ParseMediaType(contentType)
-	return err == nil && uploadsInlineTypes[strings.ToLower(mediaType)]
-}