Keep WebP, AVIF and JPEG XL encodes at the limits under 40 seconds (closes #272)

WebP is saved at effort 2 and JPEG XL at effort 4, and an AVIF output of
more than 16,777,216 pixels is refused with 400 before anything is
decoded, as AVIF is already at the lowest effort govips can set. With
one libvips thread, at any quality, 8192x8192 random pixels with an
alpha channel take up to about 23 seconds as WebP and 27 as JPEG XL, and
4096x4096, 8192x2048 or 2048x8192 up to about 34 as AVIF. Resizing with
fit=cover already decodes the input, so the output's dimensions are now
worked out before resizing, in outputDimensions. Reading the input moves
into readInput to keep Process within the linter's limits.

Model: opus-5-5

AGENTS.md

@@ -14,11 +14,7 @@ These rules MUST be followed at all times, it is very important.
   without asking first. In almost all cases, the code should be changed, NOT the
   tests. If you think the test needs to be changed, make your case for that and
   ask for permission to proceed, then stop. You need explicit user approval to
-  modify existing tests. A bug fix may change an existing test without asking
-  when that test pins exactly the behavior the filed bug calls wrong, and the
-  change moves it to the fixed behavior and nothing else; any other change to an
-  existing test still needs explicit user approval. (You do not need user
-  approval for writing NEW tests.)
+  modify existing tests. (You do not need user approval for writing NEW tests.)
 
 - When linting, assume the linter config is CORRECT, and that each item output
   by the linter is something that legitimately needs fixing in the code.

README.md

@@ -247,11 +247,9 @@ path under `/v1/` answers 200, in maintenance mode too.
   redirects to, is `localhost`, ends in `.localhost` or `.local`, or has an
   address in a blocked network (see `blocked_networks`); 429 over
   `image_requests_per_minute` (below); 415 when the upstream answers with a
-  `Content-Type` that is not one of the source formats (below), with a body
-  whose first bytes are not those of the format its `Content-Type` names, or
-  with an image whose first bytes libvips reads as another format; 502 when the
-  upstream answered with an error status, and for 5 minutes after that, or after
-  a body whose first bytes are not those of its `Content-Type`, for an image of
+  `Content-Type` that is not one of the source formats (below), or with an image
+  whose first bytes libvips reads as another format; 502 when the upstream
+  answered with an error status, and for 5 minutes after that for an image of
   the same source URL that pixa can neither serve from its cache nor make from
   the source image still in its cache; 503 when pixa is busy or in maintenance
   mode; 500 for any other failure.
@@ -366,13 +364,8 @@ not `image/jxl` is refused such a size.
 The source image may be JPEG, PNG, GIF, WebP, AVIF or JPEG XL. The upstream's
 `Content-Type` must name its format, and the image's first bytes must match it.
 An upstream answering with any other `Content-Type`, SVG included, is refused
-with 415. So is a body whose first bytes are not those of the format its
-`Content-Type` names, such as a JPEG sent as `image/png`, text sent as an image,
-or an empty body. pixa then does not fetch that source URL again for 5 minutes,
-as after an upstream error status, and meanwhile answers 502 for an image that
-would need that fetch. An image whose first bytes libvips reads as another
-format, such as an SVG whose first bytes pass the check for AVIF, is refused
-with 415 too.
+with 415, and so is an image whose first bytes libvips reads as another format,
+such as an SVG whose first bytes pass the check for AVIF.
 
 With the format `auto`, pixa chooses the format for each request from its
 `Accept` header, in this order:

TODO.md

@@ -30,7 +30,7 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
-- 2026-10-20 a WebP, AVIF or JPEG XL encode at the limits takes at most about 34
+- 2026-10-15 a WebP, AVIF or JPEG XL encode at the limits takes at most about 34
   seconds (closes #272): with one libvips thread, 8192x8192 random pixels with
   an alpha channel took about 61 seconds to encode as WebP, 81 as JPEG XL and,
   at quality 85, 96 as AVIF, so with the default `downstream_timeout` of 60
@@ -42,18 +42,6 @@ P2: security: per-origin rate limiting
   2048x8192 takes up to about 34 seconds at any quality. Resizing with
   `fit=cover` already decodes the input, so the output's dimensions are worked
   out before resizing. `README.md` gives the limit and the new times.
-- 2026-10-20 a source whose first bytes do not match its `Content-Type` answers
-  415 (closes #253): a body whose first bytes are not those of the format its
-  `Content-Type` names, such as a JPEG sent as `image/png`, text, or a body
-  under 12 bytes, answered 500 on both image routes and was logged as an error
-  of pixa's, and each request for it fetched the whole body again. Both routes
-  now answer 415, and the image service logs it at warning level as a refused
-  source. It also gets a negative cache entry, as an upstream error status does,
-  so the source URL is not fetched again for 5 minutes, and meanwhile an image
-  that would need that fetch answers 502.
-- 2026-10-20 `AGENTS.md`: a bug fix may change an existing test without asking
-  when that test pins exactly the behavior the filed bug calls wrong, and the
-  change moves it to the fixed behavior and nothing else (closes #280).
 - 2026-10-18 re-vendored from `sneak/prompts` commit `0653579` (closes #221):
   each `Dockerfile` stage installs its system packages and downloads the Go
   modules above `ARG CACHEBUST`, so check builds keep them, and the lint step

internal/handlers/first_bytes_internal_test.go

@@ -1,180 +0,0 @@
-package handlers
-
-import (
-	"bytes"
-	"encoding/json"
-	"image/color"
-	"log/slog"
-	"net/http"
-	"slices"
-	"testing"
-	"testing/fstest"
-	"time"
-
-	"github.com/go-chi/chi/v5"
-	"sneak.berlin/go/pixa/internal/encurl"
-	"sneak.berlin/go/pixa/internal/httpfetcher"
-	"sneak.berlin/go/pixa/internal/imgcache"
-)
-
-// mismatchedSourcePath is the path of the source that
-// TestFirstBytesNotMatchingContentType serves on allowlistedHost. The mock
-// fetcher names a file's Content-Type from its extension, so the source is
-// sent as image/png.
-const mismatchedSourcePath = "/images/source.png"
-
-// newRoutesFetchingThrough returns both image routes, and the Handlers behind
-// them, whose image service fetches through fetcher and has allowlistedHost on
-// its allowlist. The handlers and the image service log to log.
-func newRoutesFetchingThrough(
-	t *testing.T, fetcher httpfetcher.Fetcher, log *slog.Logger,
-) (*Handlers, http.Handler) {
-	t.Helper()
-
-	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
-		StateDir:    t.TempDir(),
-		NegativeTTL: 5 * time.Minute,
-	})
-	if err != nil {
-		t.Fatalf("imgcache.NewCache() error = %v", err)
-	}
-
-	svc, err := imgcache.NewService(&imgcache.ServiceConfig{
-		Cache:      cache,
-		Fetcher:    fetcher,
-		SigningKey: testSigningKey,
-		Allowlist:  []string{allowlistedHost},
-		Logger:     log,
-	})
-	if err != nil {
-		t.Fatalf("imgcache.NewService() error = %v", err)
-	}
-
-	encGen, err := encurl.NewGenerator(testSigningKey)
-	if err != nil {
-		t.Fatalf("encurl.NewGenerator() error = %v", err)
-	}
-
-	h := &Handlers{log: log, imgSvc: svc, encGen: encGen}
-
-	r := chi.NewRouter()
-	r.Get("/v1/image/*", h.HandleImage())
-	r.Get("/v1/e/{token}/*", h.HandleImageEnc())
-
-	return h, r
-}
-
-// mismatchedSourcePlainURL returns the /v1/image/ URL of mismatchedSourcePath
-// on allowlistedHost at 50x50 as a JPEG.
-func mismatchedSourcePlainURL(*testing.T, *Handlers) string {
-	return "/v1/image/" + allowlistedHost + mismatchedSourcePath + "/50x50.jpeg"
-}
-
-// mismatchedSourceEncURL returns an encrypted /v1/e/ URL, made with h's
-// generator, of mismatchedSourcePath on allowlistedHost at 50x50 as a JPEG.
-func mismatchedSourceEncURL(t *testing.T, h *Handlers) string {
-	t.Helper()
-
-	token, err := h.encGen.Generate(&encurl.Payload{
-		SourceHost: allowlistedHost,
-		SourcePath: mismatchedSourcePath,
-		Width:      50,
-		Height:     50,
-		Format:     imgcache.FormatJPEG,
-	})
-	if err != nil {
-		t.Fatalf("Generate() error = %v", err)
-	}
-
-	return "/v1/e/" + token + "/img.jpg"
-}
-
-// loggedLevels reads the JSON log lines in logs and returns the messages
-// logged at each level.
-func loggedLevels(t *testing.T, logs *bytes.Buffer) map[string][]string {
-	t.Helper()
-
-	t.Logf("logged:\n%s", logs.String())
-
-	messages := make(map[string][]string)
-
-	dec := json.NewDecoder(logs)
-	for dec.More() {
-		var line struct {
-			Level string `json:"level"`
-			Msg   string `json:"msg"`
-		}
-
-		err := dec.Decode(&line)
-		if err != nil {
-			t.Fatalf("decoding log line: %v", err)
-		}
-
-		messages[line.Level] = append(messages[line.Level], line.Msg)
-	}
-
-	return messages
-}
-
-// TestFirstBytesNotMatchingContentType checks that both image routes answer
-// 415 for an upstream body whose first bytes are not those of the format its
-// Content-Type names, and log it as a refused source, not as an error of
-// pixa's. The next request for that source must answer 502 without fetching
-// it again, as after an upstream error status.
-func TestFirstBytesNotMatchingContentType(t *testing.T) {
-	t.Parallel()
-
-	bodies := map[string][]byte{
-		"JPEG sent as image/png":          generateTestJPEG(t, 10, 10, color.Black),
-		"text sent as image/png":          []byte("This is just text, not a PNG"),
-		"body too short to name a format": []byte("short"),
-	}
-
-	// The source's URL on each image route.
-	sourceURLs := map[string]func(*testing.T, *Handlers) string{
-		"plain URL":     mismatchedSourcePlainURL,
-		"encrypted URL": mismatchedSourceEncURL,
-	}
-
-	for bodyName, body := range bodies {
-		for urlName, sourceURL := range sourceURLs {
-			t.Run(urlName+", "+bodyName, func(t *testing.T) {
-				t.Parallel()
-
-				fetcher := &countingFetcher{
-					Fetcher: httpfetcher.NewMock(fstest.MapFS{
-						allowlistedHost + mismatchedSourcePath: &fstest.MapFile{
-							Data: body,
-						},
-					}),
-				}
-
-				var logs bytes.Buffer
-
-				h, routes := newRoutesFetchingThrough(t, fetcher,
-					slog.New(slog.NewJSONHandler(&logs, nil)))
-				target := sourceURL(t, h)
-
-				checkErrorBody(t, sendGet(t, routes, target),
-					http.StatusUnsupportedMediaType, "unsupported source format")
-
-				levels := loggedLevels(t, &logs)
-				if len(levels[slog.LevelError.String()]) != 0 {
-					t.Errorf("logged at error level: %q",
-						levels[slog.LevelError.String()])
-				}
-
-				if !slices.Contains(levels[slog.LevelWarn.String()], "source refused") {
-					t.Error(`no "source refused" line logged at warning level`)
-				}
-
-				checkErrorBody(t, sendGet(t, routes, target),
-					http.StatusBadGateway, "upstream error")
-
-				if n := fetcher.fetches.Load(); n != 1 {
-					t.Errorf("upstream fetched %d times, want 1", n)
-				}
-			})
-		}
-	}
-}

internal/handlers/image.go

@@ -14,7 +14,6 @@ import (
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imgcache"
-	"sneak.berlin/go/pixa/internal/magic"
 )
 
 // HandleImage handles the main image proxy route:
@@ -221,18 +220,6 @@ func (s *Handlers) HandleTooManyRequests() http.HandlerFunc {
 func (s *Handlers) respondImageError(
 	w http.ResponseWriter, req *imgcache.ImageRequest, err error,
 ) {
-	// An upstream body whose first bytes are not those of the format its
-	// Content-Type names is the upstream's error, not pixa's: the image
-	// service has logged it as a refused source.
-	if errors.Is(err, magic.ErrMagicByteMismatch) ||
-		errors.Is(err, magic.ErrUnknownFormat) ||
-		errors.Is(err, magic.ErrNotEnoughData) {
-		s.respondError(w, "unsupported source format",
-			http.StatusUnsupportedMediaType)
-
-		return
-	}
-
 	s.log.Error("failed to get image",
 		"host", req.SourceHost,
 		"path", req.SourcePath,

internal/handlers/imageenc.go

@@ -15,7 +15,6 @@ import (
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imgcache"
-	"sneak.berlin/go/pixa/internal/magic"
 )
 
 // HandleImageEnc handles requests to /v1/e/{token}/* for encrypted
@@ -164,10 +163,7 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
 		s.respondError(w, "server busy, try again later",
 			http.StatusServiceUnavailable)
 	case errors.Is(err, httpfetcher.ErrInvalidContentType),
-		errors.Is(err, imageprocessor.ErrUnsupportedInputFormat),
-		errors.Is(err, magic.ErrMagicByteMismatch),
-		errors.Is(err, magic.ErrUnknownFormat),
-		errors.Is(err, magic.ErrNotEnoughData):
+		errors.Is(err, imageprocessor.ErrUnsupportedInputFormat):
 		s.respondError(w, "unsupported source format",
 			http.StatusUnsupportedMediaType)
 	case errors.Is(err, imageprocessor.ErrOutputTooLarge):

internal/imageprocessor/avif_too_large_internal_test.go

@@ -2,64 +2,44 @@ package imageprocessor
 
 import (
 	"bytes"
-	"encoding/binary"
 	"errors"
-	"os"
 	"testing"
 )
 
 // TestImageProcessor_RefusesAVIFAboveMaxPixels verifies that an AVIF output
 // of more than MaxAVIFOutputPixels is refused with ErrAVIFOutputTooLarge
-// before anything is decoded, whatever size and fit give it, the source's own
-// size included, and that an AVIF output of at most that many pixels, or one
-// in another format, is not. One input is a 64x32 PNG cut off in its pixel
-// data. The other is testdata/red.avif, a 4x4 AVIF, with the size in its
-// header changed to 4097x4096. libvips reads each input's size from its
-// header but fails when it decodes its pixel data, so a request that gets
-// that failure was not refused, and a refusal came before any decoding.
+// before anything is decoded, whatever size and fit give it, and that an AVIF
+// output of at most that many pixels, or one in another format, is not. The
+// input is a 64x32 PNG cut off in its pixel data: libvips reads its size from
+// its header but fails when it decodes it, so a request that gets that failure
+// was not refused, and a refusal came before any decoding.
 func TestImageProcessor_RefusesAVIFAboveMaxPixels(t *testing.T) {
 	t.Parallel()
 
 	whole := createTestPNG(t, 64, 32)
 	cutOff := whole[:len(whole)/2]
 
-	largeAVIF, err := os.ReadFile("testdata/red.avif")
-	if err != nil {
-		t.Fatalf("failed to read test AVIF: %v", err)
-	}
-
-	// The ispe box gives the width and then the height, 4 bytes each, after
-	// the box's type and its 4 bytes of version and flags
-	ispe := bytes.Index(largeAVIF, []byte("ispe"))
-	binary.BigEndian.PutUint32(largeAVIF[ispe+8:], 4097)
-	binary.BigEndian.PutUint32(largeAVIF[ispe+12:], 4096)
-
 	tests := []struct {
 		name          string
-		input         []byte
 		format        Format
 		width, height int
 		fit           FitMode
 		refused       bool
 	}{
-		{"AVIF at the limit", cutOff, FormatAVIF, 4096, 4096, FitCover, false},
-		{"AVIF above the limit", cutOff, FormatAVIF, 4096, 4097, FitCover, true},
-		{"WebP above the AVIF limit", cutOff, FormatWebP, 4096, 4097, FitCover, false},
+		{"AVIF at the limit", FormatAVIF, 4096, 4096, FitCover, false},
+		{"AVIF above the limit", FormatAVIF, 4096, 4097, FitCover, true},
+		{"WebP above the AVIF limit", FormatWebP, 4096, 4097, FitCover, false},
 		// FitInside does not enlarge, so the output stays 64x32
-		{"AVIF inside a larger size", cutOff, FormatAVIF, 8192, 8192, FitInside, false},
+		{"AVIF inside a larger size", FormatAVIF, 8192, 8192, FitInside, false},
 		// FitOutside scales 64x32 by 128 to cover 8192x1: 8192x4096
-		{"AVIF outside a smaller size", cutOff, FormatAVIF, 8192, 1, FitOutside, true},
-		// With no size, the output keeps the source's 4097x4096
-		{"AVIF of the source's size", largeAVIF, FormatAVIF, 0, 0, FitCover, true},
-		{"orig of an AVIF source", largeAVIF, FormatOriginal, 0, 0, FitCover, true},
-		{"JPEG of the source's size", largeAVIF, FormatJPEG, 0, 0, FitCover, false},
+		{"AVIF outside a smaller size", FormatAVIF, 8192, 1, FitOutside, true},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			_, err := New(Params{}).Process(t.Context(), bytes.NewReader(tt.input),
+			_, err := New(Params{}).Process(t.Context(), bytes.NewReader(cutOff),
 				&Request{
 					Size:    Size{Width: tt.width, Height: tt.height},
 					Format:  tt.format,
@@ -77,7 +57,7 @@ func TestImageProcessor_RefusesAVIFAboveMaxPixels(t *testing.T) {
 
 			if err == nil || errors.Is(err, ErrAVIFOutputTooLarge) {
 				t.Errorf("Process() error = %v, want the failure to decode "+
-					"the input", err)
+					"the cut-off input", err)
 			}
 		})
 	}

internal/imgcache/concurrent_misses_internal_test.go

@@ -331,7 +331,8 @@ func TestService_Get_FirstRequestLeavingKeepsTheWork(t *testing.T) {
 // TestService_Get_ConcurrentMissesShareAFailure has several requests for an
 // image that cannot be served arrive while its fetch is held: the one fetch
 // answers all of them with its error. The request after them is answered from
-// the negative cache, where the failure is kept.
+// the negative cache when the failure is kept there, and fetches again when it
+// is not.
 func TestService_Get_ConcurrentMissesShareAFailure(t *testing.T) {
 	t.Parallel()
 
@@ -345,8 +346,8 @@ func TestService_Get_ConcurrentMissesShareAFailure(t *testing.T) {
 		{"upstream answers 404, kept in the negative cache",
 			"/images/missing.jpg", httpfetcher.ErrUpstreamError,
 			ErrNegativeCached, 1},
-		{"source fails the magic byte check, kept in the negative cache",
-			"/images/text.png", magic.ErrUnknownFormat, ErrNegativeCached, 1},
+		{"source fails the magic byte check, not kept",
+			"/images/text.png", magic.ErrUnknownFormat, magic.ErrUnknownFormat, 2},
 	}
 
 	for _, tc := range tests {

internal/imgcache/service.go

@@ -458,7 +458,13 @@ func (s *Service) fetchAndProcess(
 	if err != nil {
 		// Store negative cache for certain errors
 		if isNegativeCacheable(err) {
-			s.storeNegative(ctx, req, extractStatusCode(err), err)
+			statusCode := extractStatusCode(err)
+
+			storeErr := s.cache.StoreNegative(ctx, req, statusCode, err.Error())
+			if storeErr != nil {
+				s.log.Warn("failed to store negative cache entry",
+					"host", req.SourceHost, "path", req.SourcePath, "error", storeErr)
+			}
 		}
 
 		return nil, fmt.Errorf("upstream fetch failed: %w", err)
@@ -504,16 +510,9 @@ func (s *Service) fetchAndProcess(
 		"cipher", fetchResult.TLSCipherSuite,
 	)
 
-	// Validate magic bytes match content type. A body that does not is the
-	// upstream's error, so it gets a negative cache entry, as an upstream
-	// error status does.
+	// Validate magic bytes match content type
 	err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType)
 	if err != nil {
-		s.log.Warn("source refused", "request_id", middleware.GetReqID(ctx),
-			"host", req.SourceHost, "path", req.SourcePath,
-			"content_type", fetchResult.ContentType, "error", err)
-		s.storeNegative(ctx, req, httpStatusUnsupportedMediaType, err)
-
 		return nil, fmt.Errorf("content validation failed: %w", err)
 	}
 
@@ -529,19 +528,6 @@ func (s *Service) fetchAndProcess(
 	)
 }
 
-// storeNegative stores a negative cache entry for req's source, whose fetch
-// failed with err, so the source is not fetched again until the entry
-// expires. A failure to store the entry is only logged.
-func (s *Service) storeNegative(
-	ctx context.Context, req *ImageRequest, statusCode int, err error,
-) {
-	storeErr := s.cache.StoreNegative(ctx, req, statusCode, err.Error())
-	if storeErr != nil {
-		s.log.Warn("failed to store negative cache entry",
-			"host", req.SourceHost, "path", req.SourcePath, "error", storeErr)
-	}
-}
-
 // processedVariant is a variant as processAndStore made it. Each request that
 // shared its processing serves it through a reader of its own.
 type processedVariant struct {
@@ -656,9 +642,8 @@ func signatureRequest(req *ImageRequest) *signature.Request {
 
 // HTTP status codes for error responses.
 const (
-	httpStatusBadGateway           = 502
-	httpStatusInternalError        = 500
-	httpStatusUnsupportedMediaType = 415
+	httpStatusBadGateway    = 502
+	httpStatusInternalError = 500
 )
 
 // isNegativeCacheable returns true if the error should be cached.