Test that Process stops once its deadline has passed

Process looks at its context only while it waits for a processing
slot, so a request whose deadline has passed, before Process starts or
while it reads its input, still has its image decoded and encoded while
it holds the slot. The new test fails until Process returns
context.DeadlineExceeded in both cases and gives the slot back
(#243).

Model: opus-5-5

README.md

@@ -313,23 +313,9 @@ 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 even if the first request's client goes away. The shared
+goes on for the others even if the first request's client goes away, until that
-fetch sends the first request's ID upstream, and the lines logged for the fetch
+request's `downstream_timeout` ends. The shared fetch sends the first request's
-and the transcode carry that ID.
+ID upstream, and the lines logged for the fetch and the transcode carry that ID.
-
-A transcode whose request's `downstream_timeout` has ended (for a shared one,
-the first request's) stops when it gets its processing slot or before it begins
-encoding, and gives the slot back. libvips cannot stop an encode once it has
-begun, so an encode keeps its slot until it finishes, which can be after
-`downstream_timeout`. The limit of 8192 on each side of the source and of the
-output bounds how long that takes. With one libvips thread, as pixad runs it,
-encoding 8192x8192 random pixels takes up to about 1 second as JPEG, 15 as PNG,
-16 as GIF, 63 as WebP, 110 as AVIF and 96 as JPEG XL, the slowest of 8 or 16
-bits per sample, with or without an alpha channel. WebP, AVIF and JPEG XL are
-that slow only with an alpha channel: 8-bit RGB takes about 44, 53 and 28
-seconds. Decoding such a source adds up to about 11 seconds. A WebP, AVIF or
-JPEG XL encode can therefore hold its slot past the default `downstream_timeout`
-of 60 seconds.
 
 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

TODO.md

@@ -30,21 +30,6 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
-- 2026-10-15 a transcode stops once its request's `downstream_timeout` has
-  ended, unless it has begun encoding (closes #243): the image processor looked
-  at its context only while it waited for a processing slot, so an image was
-  decoded and encoded, holding the slot, however long after its request's
-  deadline that took. `Process` now returns the context's error, and gives the
-  slot back, when the context has ended by the time it has the slot or before it
-  encodes. libvips cannot stop an encode once it has begun, so that encode keeps
-  its slot until it finishes; the limit of 8192 on each side of the source and
-  of the output bounds how long that takes. With one libvips thread, 8192x8192
-  random pixels take up to about 1 second to encode as JPEG, 15 as PNG, 16 as
-  GIF, 63 as WebP, 110 as AVIF and 96 as JPEG XL, the last three with an alpha
-  channel, plus up to about 11 seconds to decode the source; `README.md` gives
-  these times and no longer says the work stops when `downstream_timeout` ends.
-  The settings of the WebP, AVIF and JPEG XL encoders, which can hold a slot
-  past the default `downstream_timeout` of 60 seconds, are unchanged.
 - 2026-10-15 the example config allowlists no host (closes #249): it allowlisted
   `github.com` and `user-images.githubusercontent.com`, where anyone can put an
   image, so a deployment set up as Getting Started says served those images to
@@ -53,15 +38,6 @@ P2: security: per-origin rate limiting
   commented entry under `example.com`. Getting Started and "Running under upaas"
   in `README.md` say that every image on an allowlisted host is served without a
   signature, so an allowlisted host must be one the operator controls.
-- 2026-10-15 a chunked upstream body of exactly `upstream_max_response_size`
-  bytes is accepted (closes #248): once the limit was used up, the fetcher's
-  next read refused the body without looking for its end. Go's HTTP client
-  returns the end of a body sent with a `Content-Length` together with its last
-  bytes, but the end of a chunked body can come in a later read, so such a body
-  was refused after it had been downloaded whole, and the image routes
-  answered 500. Once the limit is used up, the fetcher now reads one more byte:
-  if the body has ended it is accepted, and if there is more it is refused with
-  `ErrResponseTooLarge`, as before.
 - 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side
   (closes #257): it scales the image to cover the requested size, and the side
   scaled past that size was held to no maximum, so `1x8192?fit=outside` of a
@@ -508,11 +484,11 @@ P2: security: per-origin rate limiting
   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, until it ends, which can be after its deadline; a request whose context
+  then, up to its deadline; a request whose context has already ended starts
-  has already ended starts nothing; each request counts one miss, and the
+  nothing; each request counts one miss, and the processing counts its fetch and
-  processing counts its fetch and transcode once; a panic while processing is
+  transcode once; a panic while processing is reported to Sentry when
-  reported to Sentry when `sentry_dsn` is set and becomes an error for every
+  `sentry_dsn` is set and becomes an error for every waiting request instead of
-  waiting request instead of stopping pixad; documented in `README.md`.
+  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

internal/httpfetcher/fetch_internal_test.go

@@ -9,7 +9,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"slices"
-	"strconv"
 	"strings"
 	"sync"
 	"testing"
@@ -298,139 +297,6 @@ func TestFetchMaxResponseSizeEnforced(t *testing.T) {
 	}
 }
 
-// fetchWithMaxResponseSize serves handler as the fake upstream and fetches
-// its /image route with MaxResponseSize set to maxResponseSize.
-func fetchWithMaxResponseSize(
-	t *testing.T,
-	maxResponseSize int64,
-	handler http.HandlerFunc,
-) *FetchResult {
-	t.Helper()
-
-	srv := httptest.NewServer(handler)
-	t.Cleanup(srv.Close)
-
-	cfg := DefaultConfig()
-	cfg.MaxResponseSize = maxResponseSize
-
-	f, _ := newServerFetcher(t, srv, cfg)
-
-	res := fetchImage(t, f, "/image")
-	t.Cleanup(func() { _ = res.Content.Close() })
-
-	return res
-}
-
-// TestFetchContentLengthBodyAtMaxResponseSize checks that a body sent with a
-// Content-Length is read whole at exactly MaxResponseSize bytes and refused
-// at one byte more.
-func TestFetchContentLengthBodyAtMaxResponseSize(t *testing.T) {
-	t.Parallel()
-
-	const maxResponseSize = 8
-
-	for _, tc := range []struct {
-		size    int
-		wantErr error
-	}{
-		{maxResponseSize, nil},
-		{maxResponseSize + 1, ErrResponseTooLarge},
-	} {
-		t.Run(strconv.Itoa(tc.size)+" bytes", func(t *testing.T) {
-			t.Parallel()
-
-			body := strings.Repeat("x", tc.size)
-
-			res := fetchWithMaxResponseSize(t, maxResponseSize,
-				func(w http.ResponseWriter, _ *http.Request) {
-					w.Header().Set("Content-Type", contentTypeJPEG)
-					w.Header().Set("Content-Length", strconv.Itoa(tc.size))
-					_, _ = io.WriteString(w, body)
-				})
-
-			if res.ContentLength != int64(tc.size) {
-				t.Fatalf("ContentLength = %d, want %d", res.ContentLength, tc.size)
-			}
-
-			data, err := io.ReadAll(res.Content)
-			t.Logf("body of %d bytes: read %d, error %v", tc.size, len(data), err)
-
-			if !errors.Is(err, tc.wantErr) {
-				t.Fatalf("read error = %v, want %v", err, tc.wantErr)
-			}
-
-			if err == nil && string(data) != body {
-				t.Errorf("read %q, want %q", data, body)
-			}
-		})
-	}
-}
-
-// TestFetchChunkedBodyAtMaxResponseSize checks that a chunked body is read
-// whole at exactly MaxResponseSize bytes and refused at one byte more. The
-// upstream sends the end of the body only once the test has read
-// MaxResponseSize bytes, so the read after them comes before the end of the
-// body has arrived, as it can from a real upstream.
-func TestFetchChunkedBodyAtMaxResponseSize(t *testing.T) {
-	t.Parallel()
-
-	const maxResponseSize = 8
-
-	for _, tc := range []struct {
-		size    int
-		wantErr error
-	}{
-		{maxResponseSize, nil},
-		{maxResponseSize + 1, ErrResponseTooLarge},
-	} {
-		t.Run(strconv.Itoa(tc.size)+" bytes", func(t *testing.T) {
-			t.Parallel()
-
-			body := strings.Repeat("x", tc.size)
-			endBody := make(chan struct{})
-
-			res := fetchWithMaxResponseSize(t, maxResponseSize,
-				func(w http.ResponseWriter, r *http.Request) {
-					w.Header().Set("Content-Type", contentTypeJPEG)
-					_, _ = io.WriteString(w, body)
-					// With no Content-Length, flushing sends the body
-					// chunked; its end is sent when the handler returns.
-					_ = http.NewResponseController(w).Flush()
-
-					select {
-					case <-endBody:
-					case <-r.Context().Done():
-					}
-				})
-
-			if res.ContentLength != -1 {
-				t.Fatalf("ContentLength = %d, want -1 (chunked)", res.ContentLength)
-			}
-
-			head := make([]byte, maxResponseSize)
-
-			_, err := io.ReadFull(res.Content, head)
-			if err != nil {
-				t.Fatalf("read the first %d bytes: %v", maxResponseSize, err)
-			}
-
-			close(endBody)
-
-			rest, err := io.ReadAll(res.Content)
-			data := string(head) + string(rest)
-			t.Logf("body of %d bytes: read %d, error %v", tc.size, len(data), err)
-
-			if !errors.Is(err, tc.wantErr) {
-				t.Fatalf("read error = %v, want %v", err, tc.wantErr)
-			}
-
-			if err == nil && data != body {
-				t.Errorf("read %q, want %q", data, body)
-			}
-		})
-	}
-}
-
 func TestFetchSemaphoreReleasedOnError(t *testing.T) {
 	t.Parallel()
 

internal/httpfetcher/httpfetcher.go

@@ -660,33 +660,14 @@ func dialSSRFSafe(
 	return nil, fmt.Errorf("%w to %s", errConnectFailed, host)
 }
 
-// limitedReader wraps a reader and refuses a body longer than remaining
+// limitedReader wraps a reader and limits the number of bytes read.
-// bytes with ErrResponseTooLarge. remaining goes below 0 once the body is
-// found to be longer.
 type limitedReader struct {
 	reader    io.Reader
 	remaining int64
 }
 
-// Read reads at most the remaining bytes. Once they are read, it reads one
-// more byte to tell the end of the body from more data: at the end it
-// returns the wrapped reader's error, io.EOF; with more data it returns
-// ErrResponseTooLarge, as does every later Read.
 func (r *limitedReader) Read(p []byte) (int, error) {
-	if r.remaining < 0 {
+	if r.remaining <= 0 {
-		return 0, ErrResponseTooLarge
-	}
-
-	if r.remaining == 0 {
-		var extra [1]byte
-
-		n, err := r.reader.Read(extra[:])
-		r.remaining -= int64(n)
-
-		if n == 0 {
-			return 0, err
-		}
-
 		return 0, ErrResponseTooLarge
 	}
 

internal/imageprocessor/imageprocessor.go

@@ -208,9 +208,7 @@ func New(params Params) *ImageProcessor {
 // Process transforms an image according to the request. When
 // MaxConcurrentProcessing images are already being processed, it waits up
 // to ProcessingWaitTimeout for one to finish, then fails with
-// ErrTooManyImages. When ctx has ended once Process has a slot, or before it
+// ErrTooManyImages.
-// encodes, it returns ctx's error and gives the slot back. libvips cannot stop
-// an encode once it has begun, so an encode keeps its slot until it finishes.
 func (p *ImageProcessor) Process(
 	ctx context.Context,
 	input io.Reader,
@@ -292,7 +290,7 @@ func (p *ImageProcessor) Process(
 	}
 
 	// Encode to target format
-	output, err := p.encode(ctx, img, outputFormat, req.Quality)
+	output, err := p.encode(img, outputFormat, req.Quality)
 	if err != nil {
 		return nil, fmt.Errorf("failed to encode: %w", err)
 	}
@@ -419,28 +417,25 @@ func (p *ImageProcessor) WaitForProcessing(ctx context.Context) int {
 
 // acquireSlot takes a slot in processingSemaphore, waiting at most
 // processingWaitTimeout for one to free up, and returns the func that gives
-// it back. When ctx has ended by the time it has a slot, it gives the slot
+// it back. A free slot is taken even when ctx has ended; only the wait for
-// back and returns ctx's error.
+// one stops when ctx ends, as the rest of Process does not check ctx.
 func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) {
+	release := func() { <-p.processingSemaphore }
+
 	select {
 	case p.processingSemaphore <- struct{}{}:
+		return release, nil
+	default:
+	}
+
+	select {
+	case p.processingSemaphore <- struct{}{}:
+		return release, nil
 	case <-time.After(p.processingWaitTimeout):
 		return nil, ErrTooManyImages
 	case <-ctx.Done():
 		return nil, ctx.Err()
 	}
-
-	release := func() { <-p.processingSemaphore }
-
-	// select takes the slot even when ctx has ended at the same moment
-	err := ctx.Err()
-	if err != nil {
-		release()
-
-		return nil, err
-	}
-
-	return release, nil
 }
 
 // detectFormat returns the format string for a govips image type.
@@ -532,17 +527,10 @@ func (p *ImageProcessor) resize(
 
 const defaultQuality = 85
 
-// encode encodes an image to the specified format. libvips cannot stop an
+// encode encodes an image to the specified format.
-// encode once it has begun, so when ctx has ended encode returns ctx's error
-// without beginning one.
 func (p *ImageProcessor) encode(
-	ctx context.Context, img *vips.ImageRef, format Format, quality int,
+	img *vips.ImageRef, format Format, quality int,
 ) ([]byte, error) {
-	err := ctx.Err()
-	if err != nil {
-		return nil, err
-	}
-
 	if quality <= 0 {
 		quality = defaultQuality
 	}
@@ -552,7 +540,7 @@ func (p *ImageProcessor) encode(
 	// libvips' built-in profile; govips' own sRGB path variable is set on
 	// first use but read without a lock, so concurrent requests race on it.
 	if img.HasICCProfile() {
-		err = img.TransformICCProfileWithFallback("srgb", "srgb")
+		err := img.TransformICCProfileWithFallback("srgb", "srgb")
 		if err != nil {
 			return nil, fmt.Errorf("failed to convert to sRGB: %w", err)
 		}