Share one fetch and transcode among concurrent misses (closes #65)

Requests that missed the same variant at once each fetched and
transcoded it. They now share one call through
golang.org/x/sync/singleflight, keyed on the variant cache key. The
first request processes the variant with a context that ignores its
cancellation but keeps its deadline; the others wait for its result,
holding no connection or processing slot, and return as soon as their
own context ends. The processing request waits even then, up to its
deadline. Each request counts one miss; the processing counts its fetch
and transcode once. A panic while processing is reported through the
request's Sentry hub, when there is one, and becomes an error instead
of stopping pixad.

Model: opus-5-5

README.md

@@ -111,8 +111,9 @@ When several requests for the same image, size, format, quality and fit miss
 the cache at once, they share one upstream fetch (or one read of the cached
 source) and one transcode: the first request does the work, and the others wait
 for its image or its error, holding no upstream connection or processing slot
-of their own. A waiting request stops waiting when its own client goes away;
-the work goes on for the others.
+of their own. A waiting request stops waiting when its own client goes away.
+The work goes on for the others even if the first request's client goes away,
+until that request's `downstream_timeout` ends.
 
 The login form (`POST /`) is limited to 5 attempts per minute per client
 address, counting an IPv6 client by its /64; an attempt over the limit is
@@ -245,7 +246,7 @@ variables set by the file's `env:` section are checked the same way.
 | `PIXA_UPSTREAM_FETCH_TIMEOUT`        | `upstream_fetch_timeout`        | Time allowed for one fetch from an upstream host; default `30s`              |
 | `PIXA_UPSTREAM_MAX_RESPONSE_SIZE`    | `upstream_max_response_size`    | Largest upstream response accepted, in bytes; default 50 MiB                 |
 | `PIXA_DOWNSTREAM_TIMEOUT`            | `downstream_timeout`            | Time allowed for answering one client request; default `60s`                 |
-| `PIXA_ACCESS_CONTROL_ALLOW_ORIGIN`   | `access_control_allow_origin`   | CORS origin allowed to read responses: `*` or one origin; default `*`        |
+| `PIXA_ACCESS_CONTROL_ALLOW_ORIGIN`   | `access_control_allow_origin`   | CORS origin allowed to read image responses: `*` or one origin; default `*`  |
 | `PIXA_METRICS_USERNAME`              | `metrics.username`              | Username for `/metrics`, which is served only when both are set              |
 | `PIXA_METRICS_PASSWORD`              | `metrics.password`              | Password for `/metrics`; set together with the username                      |
 | `PIXA_SENTRY_DSN`                    | `sentry_dsn`                    | Sentry DSN for error reporting; empty disables it                            |
@@ -254,8 +255,9 @@ variables set by the file's `env:` section are checked the same way.
 
 Key settings in more detail:
 
-- `access_control_allow_origin` — the origin a browser lets read pixa's
-  responses, sent as the CORS `Access-Control-Allow-Origin` header: `*`, the
+- `access_control_allow_origin` — the origin a browser lets read the responses
+  of the image routes, `/v1/image/` and `/v1/e/`, sent as the CORS
+  `Access-Control-Allow-Origin` header; no other route sends it. `*`, the
   default, is any site; otherwise one `http` or `https` origin such as
   `https://example.com`, whose host is a lowercase host name (letters,
   digits, hyphens and dots, with a letter in its last part) or an IP address

TODO.md

@@ -32,14 +32,21 @@ P2: security: referer blacklist
 - 2026-09-29 share concurrent misses (closes #65): requests that miss the same
   variant at once (the same cache key, so quality and fit included) share one
   upstream fetch or cached source read and one transcode through
-  `golang.org/x/sync/singleflight`; the first request's processing runs with a
-  context that does not end with its own, and the others wait for its image or
+  `golang.org/x/sync/singleflight`; the first request's processing ignores its
+  cancellation but keeps its deadline, and the others wait for its image or
   error holding no upstream connection or processing slot, and stop waiting
   when their own context ends; the request doing the processing waits for it
-  even then, as before; a request whose context has already ended starts
-  nothing; each request counts one miss, and the processing counts its fetch
-  and transcode once; a panic while processing becomes an error for every
-  waiting request instead of stopping pixad; documented in `README.md`.
+  even then, up to its deadline; a request whose context has already ended
+  starts nothing; each request counts one miss, and the processing counts its
+  fetch and transcode once; a panic while processing is reported to Sentry when
+  `sentry_dsn` is set and becomes an error for every waiting request instead of
+  stopping pixad; documented in `README.md`.
+- 2026-09-29 only the image routes send CORS headers (closes #98): the CORS
+  middleware, with the `access_control_allow_origin` origin, moved from the
+  router root onto a `/v1` subrouter holding `/v1/image/` and `/v1/e/`, where it
+  still answers a preflight `OPTIONS` request; the login and URL generator
+  pages, `/metrics` and the other routes send no `Access-Control-Allow-Origin`;
+  documented in `README.md` and `config.example.yml`.
 - 2026-10-02 a plain `docker build .` stamps the tag or short commit, not
   `dev` (closes #166): `.dockerignore` lets `.git` into the build context,
   without `.git/config`; with no `VERSION` build argument the `Dockerfile`

config.example.yml

@@ -103,8 +103,9 @@ upstream_max_response_size: 52428800
 # longer than upstream_fetch_timeout plus 20 seconds.
 downstream_timeout: 60s
 
-# The origin a browser lets read pixa's responses, sent as the CORS
-# Access-Control-Allow-Origin header: "*" (the default) is any site;
+# The origin a browser lets read the responses of the image routes,
+# /v1/image/ and /v1/e/, sent as the CORS Access-Control-Allow-Origin
+# header; no other route sends it. "*" (the default) is any site;
 # otherwise one http or https origin such as https://example.com, whose
 # host is a lowercase host name (letters, digits, hyphens and dots, with a
 # letter in its last part) or an IP address (IPv6 in brackets, in its

internal/imgcache/concurrent_misses_internal_test.go

@@ -13,6 +13,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/getsentry/sentry-go"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/magic"
 )
@@ -408,6 +409,32 @@ func TestService_Get_EndedRequestFetchesNothing(t *testing.T) {
 	}
 }
 
+// TestService_Get_ReturnsByItsDeadline gives a request a deadline and holds
+// its fetch until the fetch's context ends, as the fetcher does while it waits
+// for a free connection to the host. The request must return by its deadline
+// with the deadline's error.
+func TestService_Get_ReturnsByItsDeadline(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures, _ := setupHeldFetchService(t)
+
+	const timeout = 200 * time.Millisecond
+
+	ctx, cancel := context.WithTimeout(t.Context(), timeout)
+	defer cancel()
+
+	results := startGet(ctx, svc, photoVariant(fixtures, 85, FitCover))
+
+	select {
+	case got := <-results:
+		if !errors.Is(got.err, context.DeadlineExceeded) {
+			t.Errorf("Get() error = %v, want %v", got.err, context.DeadlineExceeded)
+		}
+	case <-time.After(timeout + time.Second):
+		t.Fatal("request did not return by its deadline")
+	}
+}
+
 // panickingFetcher panics on every fetch.
 type panickingFetcher struct{}
 
@@ -442,3 +469,41 @@ func TestService_Get_PanicBecomesAnError(t *testing.T) {
 		t.Errorf("Get() error = %v, want one naming the panic", err)
 	}
 }
+
+// TestService_Get_PanicIsReportedToSentry checks that a panic while a variant
+// is being made is reported through the Sentry hub on the request's context,
+// where the Sentry middleware puts one when sentry_dsn is set.
+func TestService_Get_PanicIsReportedToSentry(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := SetupTestService(t)
+	svc.fetcher = panickingFetcher{}
+
+	transport := &sentry.MockTransport{}
+
+	client, err := sentry.NewClient(sentry.ClientOptions{
+		Dsn:       "https://abc123@sentry.example.com/42",
+		Transport: transport,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ctx := sentry.SetHubOnContext(t.Context(),
+		sentry.NewHub(client, sentry.NewScope()))
+
+	func() {
+		defer func() {
+			if recovered := recover(); recovered != nil {
+				t.Fatalf("Get() panicked: %v", recovered)
+			}
+		}()
+
+		_, _ = svc.Get(ctx, photoVariant(fixtures, 85, FitCover))
+	}()
+
+	events := transport.Events()
+	if len(events) != 1 || events[0].Message != "upstream fetcher panicked" {
+		t.Errorf("Sentry events = %+v, want one for the panic", events)
+	}
+}

internal/imgcache/service.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/dustin/go-humanize"
+	"github.com/getsentry/sentry-go"
 	"golang.org/x/sync/singleflight"
 	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
@@ -250,10 +251,9 @@ var errPanicked = errors.New("panic while processing image")
 // processOrWait returns the variant req asks for. The first of the requests
 // that miss a variant at the same time processes it, and singleflight hands
 // its result, or its error, to the others: they fetch nothing, read no source
-// and take no upstream connection or processing slot. The processing runs with
-// a context that does not end with the first request's, so the others are
-// still served if that client goes away; the fetch timeout and the waits for a
-// connection and a processing slot still bound it.
+// and take no upstream connection or processing slot. The processing ignores
+// the first request's cancellation, so the others are still served if that
+// client goes away, but keeps its deadline.
 func (s *Service) processOrWait(
 	ctx context.Context, req *ImageRequest,
 ) (*ImageResponse, error) {
@@ -273,7 +273,9 @@ func (s *Service) processOrWait(
 			close(processing)
 
 			// singleflight would raise a panic again in a goroutine of its
-			// own, where no handler recovers it, and stop pixad
+			// own, where no handler recovers it, and stop pixad. It is
+			// reported through the Sentry hub that the Sentry middleware puts
+			// on the request's context when sentry_dsn is set.
 			defer func() {
 				recovered := recover()
 				if recovered != nil {
@@ -281,12 +283,24 @@ func (s *Service) processOrWait(
 						"host", req.SourceHost, "path", req.SourcePath,
 						"panic", recovered, "stack", string(debug.Stack()))
 
+					if hub := sentry.GetHubFromContext(ctx); hub != nil {
+						hub.RecoverWithContext(ctx, recovered)
+					}
+
 					err = fmt.Errorf("%w: %v", errPanicked, recovered)
 				}
 			}()
 
-			return s.processFromSourceOrFetch(
-				context.WithoutCancel(ctx), req, cacheKey)
+			processingCtx := context.WithoutCancel(ctx)
+
+			if deadline, ok := ctx.Deadline(); ok {
+				var cancel context.CancelFunc
+
+				processingCtx, cancel = context.WithDeadline(processingCtx, deadline)
+				defer cancel()
+			}
+
+			return s.processFromSourceOrFetch(processingCtx, req, cacheKey)
 		})
 
 	var result singleflight.Result

internal/server/cors_internal_test.go

@@ -0,0 +1,64 @@
+package server
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// TestCORSOnlyOnImageRoutes verifies that the image routes answer with the
+// configured access_control_allow_origin, a preflight request included, and
+// that the login and URL generator pages send no Access-Control-Allow-Origin,
+// so no other site can read them. /metrics is left out: its middleware
+// registers with the process-wide Prometheus registry, which only one test
+// in this package can do.
+func TestCORSOnlyOnImageRoutes(t *testing.T) {
+	t.Parallel()
+
+	const appOrigin = "https://app.example.com"
+
+	s := newTestServer(t)
+	s.config.AccessControlAllowOrigin = appOrigin
+	s.SetupRoutes()
+
+	requests := []struct {
+		method string
+		path   string
+		want   string
+	}{
+		{http.MethodGet, unsignedImagePath, appOrigin},
+		{http.MethodHead, unsignedImagePath, appOrigin},
+		{http.MethodOptions, unsignedImagePath, appOrigin},
+		{http.MethodGet, encryptedImagePath, appOrigin},
+		{http.MethodGet, "/", ""},
+		{http.MethodOptions, "/", ""},
+		{http.MethodPost, "/generate", ""},
+		{http.MethodGet, "/logout", ""},
+	}
+
+	for _, tc := range requests {
+		t.Run(tc.method+" "+tc.path, func(t *testing.T) {
+			t.Parallel()
+
+			req := httptest.NewRequestWithContext(
+				t.Context(), tc.method, tc.path, nil)
+			req.Header.Set("Origin", appOrigin)
+
+			// An OPTIONS request naming the method it asks about is the
+			// preflight a browser sends before some cross-origin requests.
+			if tc.method == http.MethodOptions {
+				req.Header.Set("Access-Control-Request-Method", http.MethodGet)
+			}
+
+			rec := httptest.NewRecorder()
+			s.ServeHTTP(rec, req)
+			t.Logf("status %d", rec.Code)
+
+			got := rec.Header().Get("Access-Control-Allow-Origin")
+			if got != tc.want {
+				t.Errorf("Access-Control-Allow-Origin = %q, want %q",
+					got, tc.want)
+			}
+		})
+	}
+}

internal/server/maintenance_internal_test.go

@@ -13,6 +13,10 @@ import (
 // unsignedImagePath is an image URL that carries no signature.
 const unsignedImagePath = "/v1/image/cdn.example.com/cat.jpg/100x100.jpeg"
 
+// encryptedImagePath is an encrypted image URL whose token cannot be
+// decrypted.
+const encryptedImagePath = "/v1/e/token/cat.jpg"
+
 // TestMaintenanceModeRefusesImageRequests verifies that while maintenance
 // mode is on, both image routes answer 503 Service Unavailable with a
 // Retry-After header and the JSON error body the image handlers send.
@@ -28,7 +32,7 @@ func TestMaintenanceModeRefusesImageRequests(t *testing.T) {
 	}{
 		{http.MethodGet, unsignedImagePath},
 		{http.MethodHead, unsignedImagePath},
-		{http.MethodGet, "/v1/e/token/cat.jpg"},
+		{http.MethodGet, encryptedImagePath},
 	}
 
 	for _, tc := range requests {
@@ -95,7 +99,7 @@ func TestImageRequestsServedWithoutMaintenanceMode(t *testing.T) {
 	}{
 		{http.MethodGet, unsignedImagePath, http.StatusUnauthorized},
 		{http.MethodHead, unsignedImagePath, http.StatusUnauthorized},
-		{http.MethodGet, "/v1/e/token/cat.jpg", http.StatusBadRequest},
+		{http.MethodGet, encryptedImagePath, http.StatusBadRequest},
 	}
 
 	for _, tc := range requests {

internal/server/routes.go

@@ -38,7 +38,6 @@ func (s *Server) SetupRoutes() {
 		s.router.Use(s.mw.Metrics())
 	}
 
-	s.router.Use(s.mw.CORS())
 	s.router.Use(middleware.Timeout(s.config.DownstreamTimeout))
 
 	if s.sentryEnabled {
@@ -74,22 +73,31 @@ func (s *Server) SetupRoutes() {
 
 	s.router.Get("/logout", s.h.HandleLogout())
 
-	// Image routes, refused while maintenance mode is on. Only these: the
-	// image's Docker HEALTHCHECK requests the health check, a 503 there
-	// would make the container unhealthy, and upaas marks a deploy failed
-	// when its container is unhealthy.
-	s.router.Group(func(r chi.Router) {
-		r.Use(s.refuseDuringMaintenance)
+	// Image routes, the only ones that send CORS headers, as pages on other
+	// sites read them. They are a subrouter rather than a group: a group's
+	// middleware runs only for a request that matches one of its routes,
+	// and a browser's preflight OPTIONS request matches none, so the CORS
+	// middleware could not answer it.
+	s.router.Route("/v1", func(r chi.Router) {
+		r.Use(s.mw.CORS())
 
-		// Main image proxy route
-		// /v1/image/<host>/<path>/<width>x<height>.<format>
-		r.Get("/v1/image/*", s.h.HandleImage())
-		r.Head("/v1/image/*", s.h.HandleImage())
+		// Refused while maintenance mode is on. Only these: the image's
+		// Docker HEALTHCHECK requests the health check, a 503 there would
+		// make the container unhealthy, and upaas marks a deploy failed
+		// when its container is unhealthy.
+		r.Group(func(r chi.Router) {
+			r.Use(s.refuseDuringMaintenance)
 
-		// Encrypted image URL route
-		// The trailing filename (e.g., /img.jpg) is ignored but helps
-		// browsers with content type
-		r.Get("/v1/e/{token}/*", s.h.HandleImageEnc())
+			// Main image proxy route
+			// /v1/image/<host>/<path>/<width>x<height>.<format>
+			r.Get("/image/*", s.h.HandleImage())
+			r.Head("/image/*", s.h.HandleImage())
+
+			// Encrypted image URL route
+			// The trailing filename (e.g., /img.jpg) is ignored but helps
+			// browsers with content type
+			r.Get("/e/{token}/*", s.h.HandleImageEnc())
+		})
 	})
 
 	// Metrics endpoint with auth