Re-vendor the canonical files from sneak/prompts at 0653579 (closes #221)

REPO_POLICIES.md, script/cibuild, script/docker, script/lint and
script/test are their copies at that commit. In every Dockerfile stage
the system package install and the Go module download now come before
ARG CACHEBUST, so check builds keep those layers until the base image,
the install line, go.mod or go.sum change. golangci-lint run also
mounts golangci-lint's cache. The test phase and the build stage
install their packages with apt-get and apk directly, as
REPO_POLICIES.md now has them do, instead of through
script/bootstrap --cgo. The test phase installs no recommended
packages and names libheif's AVIF encoder, libheif-plugin-aomenc,
which the tests use.

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
+  modify existing tests. (You do not need user approval for writing NEW tests.)
-  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.)
 
 - 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
+  `Content-Type` that is not one of the source formats (below), or with an image
-  whose first bytes are not those of the format its `Content-Type` names, or
+  whose first bytes libvips reads as another format; 502 when the upstream
-  with an image whose first bytes libvips reads as another format; 502 when the
+  answered with an error status, and for 5 minutes after that for an image of
-  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
   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.
@@ -357,22 +355,11 @@ either side may be; so is a size with `fit=outside`, which scales the image to
 cover the size and so past it on one side, when that side would come out
 above 8192.
 
-An AVIF image of more than 16,777,216 pixels, the number 4096x4096 and 8192x2048
-have, is refused with 400 before anything is decoded, as its encode could take
-more than 40 seconds (see below). That includes `orig` of an AVIF source, and
-`auto` when it chooses AVIF, so a client whose `Accept` names `image/avif` but
-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
+with 415, and so is an image whose first bytes libvips reads as another format,
-`Content-Type` names, such as a JPEG sent as `image/png`, text sent as an image,
+such as an SVG whose first bytes pass the check for AVIF.
-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 the format `auto`, pixa chooses the format for each request from its
 `Accept` header, in this order:
@@ -417,18 +404,14 @@ transcode that stops this way is answered 503 with the error
 ended. govips gives pixa no way to stop libvips once an encode 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, and for AVIF the limit of 16,777,216 pixels, bound how long that takes.
+output bounds how long that takes. With one libvips thread, as pixad runs it,
-With one libvips thread, as pixad runs it, encoding 8192x8192 random pixels
+encoding 8192x8192 random pixels takes up to about 1 second as JPEG, 15 as PNG,
-takes up to about 1 second as JPEG, 15 as PNG and 16 as GIF, the slowest of 8 or
+16 as GIF, 63 as WebP, 110 as AVIF and 96 as JPEG XL, the slowest of 8 or 16
-16 bits per sample, with or without an alpha channel. The WebP and JPEG XL
+bits per sample, with or without an alpha channel. WebP, AVIF and JPEG XL are
-encoders run below libvips' default effort, and AVIF at the lowest effort govips
+that slow only with an alpha channel: 8-bit RGB takes about 44, 53 and 28
-can set, so that no encode takes more than 40 seconds of the default
+seconds. Decoding such a source adds up to about 11 seconds. A WebP, AVIF or
-`downstream_timeout` of 60: at any quality and 8 or 16 bits per sample,
+JPEG XL encode can therefore hold its slot past the default `downstream_timeout`
-8192x8192 random pixels with an alpha channel take up to about 23 seconds as
+of 60 seconds.
-WebP and 27 as JPEG XL, and 4096x4096, 8192x2048 or 2048x8192 up to about 34 as
-AVIF, which at quality 85 would take about 96 at 8192x8192. Without an alpha
-channel they take up to about 16, 13 and 22 seconds. Decoding such a source adds
-up to about 11 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
@@ -480,13 +463,13 @@ nor change what it asks for.
    sent with an empty format, or none, also makes a JPEG XL URL. Width and
    height both empty or `0` keep the original size; if only one of them is empty
    or `0`, that side is scaled to keep the image's proportions. A URL whose
-   scaled side would come out above 8192 answers 400, and so does an AVIF URL of
+   scaled side would come out above 8192 answers 400, and so does a source URL
-   more than 16,777,216 pixels or a source URL whose path holds a `..` segment.
+   whose path holds a `..` segment. So does a source URL that the image URL
-   So does a source URL that the image URL would fetch written another way, and
+   would fetch written another way, and the page then shows the URL pixa would
-   the page then shows the URL pixa would fetch: one with a user name and
+   fetch: one with a user name and password, which pixa does not send, or one
-   password, which pixa does not send, or one whose path holds `%2F`, fetched as
+   whose path holds `%2F`, fetched as `/`, or `(`, fetched as `%28`, as pixa
-   `/`, or `(`, fetched as `%28`, as pixa keeps the source path decoded and
+   keeps the source path decoded and fetches it percent-encoded again (see
-   fetches it percent-encoded again (see Routes).
+   Routes).
 3. The page shows the URL, `https://<host>/v1/e/<token>/img.<format>`, and when
    it expires. `<host>` is the host the page was opened on, and the URL starts
    with `http` instead while `debug` is on. The name after the token is ignored
@@ -730,22 +713,16 @@ Key settings in more detail:
   omitted key defaults to the RFC 1918 private ranges (`10.0.0.0/8`,
   `172.16.0.0/12`, `192.168.0.0/16`), since pixa is deployed behind a proxy on a
   private network; an explicitly empty list (`[]`) trusts no one, and an
-  explicit list replaces the default. An invalid CIDR aborts startup. An IPv6
+  explicit list replaces the default. An invalid CIDR aborts startup. Set this
-  address in the IPv4-mapped form (`::ffff:a.b.c.d`) is matched only as the IPv4
+  to the address pixa sees for requests that come through your proxy, such as
-  address it holds, so an IPv6 range such as `::/0` trusts neither it nor a
+  `172.17.0.1/32`, when the defaults do not cover it, or to trust nothing else
-  plain IPv4 address. An entry written in that form is read as the IPv4 range it
+  (see the login limit and `image_requests_per_minute` under Routes). For a
-  covers, so `::ffff:10.0.0.0/104` trusts what `10.0.0.0/8` trusts; such an
+  proxy on the Docker host that connects to pixa over `127.0.0.1`, that address
-  entry shorter than `/96` covers more than IPv4 addresses and aborts startup.
+  is the gateway of the container's Docker network (`172.17.0.1` on the default
-  Set this to the address pixa sees for requests that come through your proxy,
+  bridge), not the proxy's own address; a proxy that connects through another of
-  such as `172.17.0.1/32`, when the defaults do not cover it, or to trust
+  the host's addresses is seen with that address. To be sure which address it
-  nothing else (see the login limit and `image_requests_per_minute` under
+  is, set this to `[]` (or `PIXA_TRUSTED_PROXIES` to empty), send a request
-  Routes). For a proxy on the Docker host that connects to pixa over
+  through the proxy, and read `remoteIP` in pixa's request log line for it
-  `127.0.0.1`, that address is the gateway of the container's Docker network
-  (`172.17.0.1` on the default bridge), not the proxy's own address; a proxy
-  that connects through another of the host's addresses is seen with that
-  address. To be sure which address it is, set this to `[]` (or
-  `PIXA_TRUSTED_PROXIES` to empty), send a request through the proxy, and read
-  `remoteIP` in pixa's request log line for it
 - `upstream_fetch_timeout` — time allowed for one fetch from an upstream host,
   as a duration such as `30s` (the default) or `2m`
 - `upstream_max_response_size` — largest upstream response accepted, in bytes;
@@ -863,16 +840,15 @@ cache is kept from build to build: the steps that compile mount it, so a package
 whose inputs are unchanged is not compiled again, and both `go test` runs pass
 `-count=1` so that no stored test result is replayed. The lint step also mounts
 golangci-lint's cache, which reuses a package's findings only when its files,
-those of the packages it imports, `go.mod`, the linters and their settings in
+those of the packages it imports, `go.mod`, `.golangci.yml` and the
-`.golangci.yml`, and the golangci-lint version are all unchanged. The `test`
+golangci-lint version are all unchanged. The `test` phase runs on the Debian Go
-phase runs on the Debian Go image, with Debian's libvips and libheif; the image
+image, with Debian's libvips and libheif; the image pixa ships is built and runs
-pixa ships is built and runs on Alpine. `script/check`, `script/cibuild`,
+on Alpine. `script/check`, `script/cibuild`, `script/docker`, `script/lint`,
-`script/docker`, `script/lint`, `script/test`, `script/setup` and
+`script/test`, `script/setup` and `script/install-precommit` are the standard
-`script/install-precommit` are the standard copies from `sneak/prompts`, kept
+copies from `sneak/prompts`, kept identical to them. `script/fmt` and
-identical to them. `script/fmt` and `script/fmt-check` are the standard copies
+`script/fmt-check` are the standard copies with pixa's `gofmt` step kept before
-with pixa's `gofmt` step kept before prettier. prettier formats the markdown
+prettier. prettier formats the markdown only, not the HTML templates, as it
-only, not the HTML templates, as it cannot parse a Go template action inside a
+cannot parse a Go template action inside a tag.
-tag.
 
 ## Load Test
 

TODO.md

@@ -30,38 +30,10 @@ 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
-  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
-  seconds such an image was never delivered, yet its encode held a processing
-  slot to the end. WebP is now saved at effort 2 and JPEG XL at effort 4, which
-  take up to about 23 and 27 seconds at any quality. AVIF was already at the
-  lowest effort govips can set, so an AVIF output of more than 16,777,216 pixels
-  is refused with 400 before anything is decoded; 4096x4096, 8192x2048 or
-  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
   keeps golangci-lint's cache.
-- 2026-10-19 a `trusted_proxies` entry in the IPv4-mapped form trusted nothing;
-  it is now read as the IPv4 range it covers by the same code that reads
-  `blocked_networks`, and such an entry shorter than `/96` aborts startup
-  (closes #303).
 - 2026-10-18 the next reconciliation pass waits at least ten times as long as
   the last one took, so passes on a large cache no longer run back to back
   (closes #250).

internal/config/config.go

@@ -272,7 +272,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 		return nil, err
 	}
 
-	blockedNetworks, err := parseCIDRList(sc, keyBlockedNetworks)
+	blockedNetworks, err := parseBlockedNetworks(sc)
 	if err != nil {
 		return nil, err
 	}
@@ -1249,13 +1249,6 @@ func defaultTrustedProxies() []netip.Prefix {
 // wrong type, an empty entry, a non-string entry, or an unparseable CIDR
 // aborts startup naming the key and the offending value; the default
 // (an empty list) applies only to an omitted key.
-//
-// An entry written in the IPv4-mapped form (::ffff:a.b.c.d) is kept as the
-// IPv4 range it covers: ::ffff:10.0.0.0/104 is kept as 10.0.0.0/8. Both
-// blocked_networks and trusted_proxies are matched against an IPv4-mapped
-// address as the IPv4 address it holds, so the entry as written would match
-// nothing. Such an entry shorter than /96 covers more than the IPv4
-// addresses and aborts startup.
 func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) {
 	raw, ok := lookupValue(sc, key)
 	if !ok {
@@ -1271,10 +1264,6 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) {
 		return nil, err
 	}
 
-	// An IPv4-mapped address is the 96 bits of ::ffff:0:0/96 followed by
-	// the 32 bits of the IPv4 address it holds.
-	const mappedPrefixBits = 96
-
 	prefixes := make([]netip.Prefix, 0, len(entries))
 
 	for _, entry := range entries {
@@ -1284,22 +1273,45 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) {
 				settingName(key), entry, errNotAValidCIDR)
 		}
 
-		if prefix.Addr().Is4In6() && prefix.Bits() < mappedPrefixBits {
-			return nil, fmt.Errorf("%s: value %q is %w",
-				settingName(key), entry, errMappedTooShort)
-		}
-
-		if prefix.Addr().Is4In6() {
-			prefix = netip.PrefixFrom(prefix.Addr().Unmap(),
-				prefix.Bits()-mappedPrefixBits)
-		}
-
 		prefixes = append(prefixes, prefix)
 	}
 
 	return prefixes, nil
 }
 
+// parseBlockedNetworks parses blocked_networks with parseCIDRList, and keeps
+// an entry written in the IPv4-mapped form (::ffff:a.b.c.d) as the IPv4
+// range it covers: ::ffff:10.0.0.0/104 is kept as 10.0.0.0/8. The fetcher
+// matches an IPv4-mapped address as the IPv4 address it holds, so the entry
+// as written would match nothing. Such an entry shorter than /96 covers more
+// than the IPv4 addresses and aborts startup.
+func parseBlockedNetworks(sc *smartconfig.Config) ([]netip.Prefix, error) {
+	prefixes, err := parseCIDRList(sc, keyBlockedNetworks)
+	if err != nil {
+		return nil, err
+	}
+
+	// An IPv4-mapped address is the 96 bits of ::ffff:0:0/96 followed by
+	// the 32 bits of the IPv4 address it holds.
+	const mappedPrefixBits = 96
+
+	for i, prefix := range prefixes {
+		if !prefix.Addr().Is4In6() {
+			continue
+		}
+
+		if prefix.Bits() < mappedPrefixBits {
+			return nil, fmt.Errorf("%s: value %q is %w",
+				settingName(keyBlockedNetworks), prefix, errMappedTooShort)
+		}
+
+		prefixes[i] = netip.PrefixFrom(prefix.Addr().Unmap(),
+			prefix.Bits()-mappedPrefixBits)
+	}
+
+	return prefixes, nil
+}
+
 // parseHostList parses the value of the named config key into host patterns,
 // or returns nil if the key is omitted. It accepts a YAML list of strings or a
 // comma-separated string. An explicitly null value, a wrong type, an empty

internal/config/trusted_proxies_internal_test.go

@@ -83,41 +83,3 @@ func TestTrustedProxiesInvalidAbortsStartup(t *testing.T) {
 		},
 	})
 }
-
-// TestTrustedProxiesMappedEntryIsIPv4Range checks that an entry written in
-// the IPv4-mapped form (::ffff:a.b.c.d) is kept as the IPv4 range it covers,
-// since the client address is matched as the IPv4 address it holds. /96, the
-// shortest such entry, covers every IPv4 address.
-func TestTrustedProxiesMappedEntryIsIPv4Range(t *testing.T) {
-	t.Parallel()
-
-	c, err := configFromYAML(t, signingKeyLine+
-		`trusted_proxies: ["::ffff:10.0.0.0/104", "::ffff:0:0/96"]`+"\n")
-	if err != nil {
-		t.Fatalf("IPv4-mapped trusted_proxies entries should load: %v", err)
-	}
-
-	got := make([]string, len(c.TrustedProxies))
-	for i, p := range c.TrustedProxies {
-		got[i] = p.String()
-	}
-
-	if joined := strings.Join(got, ","); joined != "10.0.0.0/8,0.0.0.0/0" {
-		t.Errorf("TrustedProxies = %v, want 10.0.0.0/8 and 0.0.0.0/0", got)
-	}
-}
-
-// TestTrustedProxiesMappedEntryShorterThan96AbortsStartup checks that an
-// entry in the IPv4-mapped form shorter than /96, which covers more than the
-// IPv4 addresses, aborts startup with an error naming the key and the value.
-func TestTrustedProxiesMappedEntryShorterThan96AbortsStartup(t *testing.T) {
-	t.Parallel()
-
-	runAbortCases(t, []abortCase{
-		{
-			name:              "mapped-95",
-			yaml:              signingKeyLine + `trusted_proxies: ["::ffff:0:0/95"]` + "\n",
-			wantErrSubstrings: []string{keyTrustedProxies, "::ffff:0:0/95"},
-		},
-	})
-}

internal/handlers/avif_too_large_internal_test.go

@@ -1,49 +0,0 @@
-package handlers
-
-import (
-	"fmt"
-	"log/slog"
-	"net/http"
-	"net/http/httptest"
-	"strings"
-	"testing"
-
-	"sneak.berlin/go/pixa/internal/imageprocessor"
-	"sneak.berlin/go/pixa/internal/imgcache"
-)
-
-// TestAVIFOutputTooLargeAnswers400 checks that respondImageError and
-// handleImageError, which answer the errors of the two image routes, answer
-// 400 naming the AVIF pixel limit when the image processor refuses an AVIF
-// output above it, wrapped as the service wraps it. The request's source only
-// goes into a log line, so it is left empty.
-func TestAVIFOutputTooLargeAnswers400(t *testing.T) {
-	t.Parallel()
-
-	h := &Handlers{log: slog.New(slog.DiscardHandler)}
-	err := fmt.Errorf("image processing failed: %w: 4097x4096",
-		imageprocessor.ErrAVIFOutputTooLarge)
-	wantBody := fmt.Sprintf("above %d pixels", imageprocessor.MaxAVIFOutputPixels)
-
-	plain := httptest.NewRecorder()
-	h.respondImageError(plain, &imgcache.ImageRequest{}, err)
-
-	encrypted := httptest.NewRecorder()
-	h.handleImageError(encrypted, err)
-
-	for name, rec := range map[string]*httptest.ResponseRecorder{
-		"respondImageError": plain, "handleImageError": encrypted,
-	} {
-		t.Logf("%s: status = %d, body = %q", name, rec.Code, rec.Body.String())
-
-		if rec.Code != http.StatusBadRequest {
-			t.Errorf("%s: status = %d, want %d",
-				name, rec.Code, http.StatusBadRequest)
-		}
-
-		if !strings.Contains(rec.Body.String(), wantBody) {
-			t.Errorf("%s: body = %q, want it to say %q",
-				name, rec.Body.String(), wantBody)
-		}
-	}
-}

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,
@@ -277,14 +264,6 @@ func (s *Handlers) respondImageError(
 		return
 	}
 
-	if errors.Is(err, imageprocessor.ErrAVIFOutputTooLarge) {
-		s.respondError(w, fmt.Sprintf("size too large for AVIF: the output "+
-			"would be above %d pixels", imageprocessor.MaxAVIFOutputPixels),
-			http.StatusBadRequest)
-
-		return
-	}
-
 	s.respondError(w, "internal error", http.StatusInternalServerError)
 }
 

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,20 +163,13 @@ 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, imageprocessor.ErrUnsupportedInputFormat):
-		errors.Is(err, magic.ErrMagicByteMismatch),
-		errors.Is(err, magic.ErrUnknownFormat),
-		errors.Is(err, magic.ErrNotEnoughData):
 		s.respondError(w, "unsupported source format",
 			http.StatusUnsupportedMediaType)
 	case errors.Is(err, imageprocessor.ErrOutputTooLarge):
 		s.respondError(w, fmt.Sprintf("size too large for the source: the side "+
 			"computed from its proportions is above %d",
 			imageprocessor.MaxOutputDimension), http.StatusBadRequest)
-	case errors.Is(err, imageprocessor.ErrAVIFOutputTooLarge):
-		s.respondError(w, fmt.Sprintf("size too large for AVIF: the output "+
-			"would be above %d pixels", imageprocessor.MaxAVIFOutputPixels),
-			http.StatusBadRequest)
 	default:
 		s.log.Error("image request failed", "error", err)
 		s.respondError(w, "internal error", http.StatusInternalServerError)

internal/imageprocessor/avif_too_large_internal_test.go

@@ -1,84 +0,0 @@
-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.
-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},
-		// FitInside does not enlarge, so the output stays 64x32
-		{"AVIF inside a larger size", cutOff, 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},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			_, err := New(Params{}).Process(t.Context(), bytes.NewReader(tt.input),
-				&Request{
-					Size:    Size{Width: tt.width, Height: tt.height},
-					Format:  tt.format,
-					FitMode: tt.fit,
-				})
-			t.Logf("Process() error = %v", err)
-
-			if tt.refused {
-				if !errors.Is(err, ErrAVIFOutputTooLarge) {
-					t.Errorf("Process() error = %v, want %v", err, ErrAVIFOutputTooLarge)
-				}
-
-				return
-			}
-
-			if err == nil || errors.Is(err, ErrAVIFOutputTooLarge) {
-				t.Errorf("Process() error = %v, want the failure to decode "+
-					"the input", err)
-			}
-		})
-	}
-}

internal/imageprocessor/encoder_settings_internal_test.go

@@ -77,10 +77,10 @@ func TestImageProcessor_PNGIsCompressed(t *testing.T) {
 	}
 }
 
-// TestImageProcessor_WebPAtEffort2 verifies that WebP is saved at effort 2,
+// TestImageProcessor_WebPAtDefaultEffort verifies that WebP is saved at
-// not libvips' default of 4: the output is the size of the same image saved
+// libvips' default effort, 4: the output is the size of the same image saved
-// at effort 2.
+// at that effort.
-func TestImageProcessor_WebPAtEffort2(t *testing.T) {
+func TestImageProcessor_WebPAtDefaultEffort(t *testing.T) {
 	t.Parallel()
 
 	input := createTestJPEG(t, 200, 150)
@@ -90,14 +90,14 @@ func TestImageProcessor_WebPAtEffort2(t *testing.T) {
 	want, _, err := decode(t, input).ExportWebp(&vips.WebpExportParams{
 		StripMetadata:   true,
 		Quality:         85,
-		ReductionEffort: 2,
+		ReductionEffort: 4,
 	})
 	if err != nil {
 		t.Fatalf("ExportWebp() error = %v", err)
 	}
 
 	if size != int64(len(want)) {
-		t.Errorf("WebP output is %d bytes, want %d, the size at effort 2",
+		t.Errorf("WebP output is %d bytes, want %d, the size at effort 4",
 			size, len(want))
 	}
 }
@@ -143,29 +143,3 @@ func TestImageProcessor_AVIFAtEffort1(t *testing.T) {
 			"and 8 bits", size, len(want))
 	}
 }
-
-// TestImageProcessor_JPEGXLAtEffort4 verifies that JPEG XL is saved at
-// effort 4, not libvips' default of 7: the output is the size of the same
-// image saved at effort 4, with the distance exportJXL gives quality 85.
-func TestImageProcessor_JPEGXLAtEffort4(t *testing.T) {
-	t.Parallel()
-
-	input := createTestJPEG(t, 200, 150)
-
-	size := processedSize(t, input, &Request{Format: FormatJXL, Quality: 85})
-
-	params := vips.NewJxlExportParams()
-	params.Quality = 85
-	params.Distance = jxlDistance(85)
-	params.Effort = 4
-
-	want, _, err := decode(t, input).ExportJxl(params)
-	if err != nil {
-		t.Fatalf("ExportJxl() error = %v", err)
-	}
-
-	if size != int64(len(want)) {
-		t.Errorf("JPEG XL output is %d bytes, want %d, the size at effort 4",
-			size, len(want))
-	}
-}

internal/imageprocessor/imageprocessor.go

@@ -142,19 +142,6 @@ const MaxOutputDimension = 8192
 // requested size.
 var ErrOutputTooLarge = errors.New("output image dimensions exceed maximum")
 
-// MaxAVIFOutputPixels is the most pixels an AVIF output may have, so that an
-// encode takes at most 40 seconds of the default downstream_timeout of 60
-// seconds; AVIF is already at the lowest effort govips can set (avifEffort).
-// With one thread, as pixad runs libvips, saving 4096x4096, 8192x2048 or
-// 2048x8192 random pixels with an alpha channel takes up to about 34
-// seconds, the slowest at any quality and at 8 or 16 bits per sample, and
-// 5120x4096 at 8 bits already takes about 36.
-const MaxAVIFOutputPixels = 4096 * 4096
-
-// ErrAVIFOutputTooLarge is returned, before anything is decoded, when an AVIF
-// output would have more than MaxAVIFOutputPixels pixels.
-var ErrAVIFOutputTooLarge = errors.New("AVIF output has too many pixels")
-
 // ErrUnsupportedOutputFormat is returned when the requested output format is
 // not supported.
 var ErrUnsupportedOutputFormat = errors.New("unsupported output format")
@@ -223,8 +210,7 @@ func New(params Params) *ImageProcessor {
 // ErrTooManyImages. When ctx has ended once Process has a slot, or before it
 // encodes, it returns ctx's error and gives the slot back. govips gives pixa
 // no way to stop libvips once an encode has begun, so an encode keeps its
-// slot until it finishes. An AVIF output of more than MaxAVIFOutputPixels is
+// slot until it finishes.
-// refused with ErrAVIFOutputTooLarge before anything is decoded.
 func (p *ImageProcessor) Process(
 	ctx context.Context,
 	input io.Reader,
@@ -237,9 +223,18 @@ func (p *ImageProcessor) Process(
 
 	defer release()
 
-	data, err := p.readInput(input)
+	// Read input with a size limit to prevent unbounded memory consumption.
+	// We read at most maxInputBytes+1 so we can detect if the input exceeds
+	// the limit without consuming additional memory.
+	limited := io.LimitReader(input, p.maxInputBytes+1)
+
+	data, err := io.ReadAll(limited)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to read input: %w", err)
+	}
+
+	if int64(len(data)) > p.maxInputBytes {
+		return nil, ErrInputDataTooLarge
 	}
 
 	// govips picks the libvips loader from the input's first bytes with
@@ -282,28 +277,20 @@ func (p *ImageProcessor) Process(
 		return nil, err
 	}
 
+	// Resize if needed
+	if targetWidth != origWidth || targetHeight != origHeight {
+		err := p.resize(img, targetWidth, targetHeight, req.FitMode)
+		if err != nil {
+			return nil, fmt.Errorf("failed to resize: %w", err)
+		}
+	}
+
 	// orig is the source's own format; encode refuses an empty format
 	outputFormat := req.Format
 	if outputFormat == FormatOriginal {
 		outputFormat = sourceFormat
 	}
 
-	// Determine output dimensions; too large ones are refused before
-	// resizing, which with FitCover already decodes the input
-	outputWidth, outputHeight, err := outputDimensions(origWidth, origHeight,
-		targetWidth, targetHeight, req.FitMode, outputFormat)
-	if err != nil {
-		return nil, err
-	}
-
-	// Resize if needed
-	if targetWidth != origWidth || targetHeight != origHeight {
-		err := p.resize(img, outputWidth, outputHeight, req.FitMode)
-		if err != nil {
-			return nil, fmt.Errorf("failed to resize: %w", err)
-		}
-	}
-
 	// Encode to target format
 	output, err := p.encode(ctx, img, outputFormat, req.Quality)
 	if err != nil {
@@ -458,23 +445,6 @@ func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) {
 	return release, nil
 }
 
-// readInput reads input with a size limit to prevent unbounded memory
-// consumption, refusing more than maxInputBytes with ErrInputDataTooLarge. It
-// reads at most maxInputBytes+1 so it can detect if the input exceeds the
-// limit without consuming additional memory.
-func (p *ImageProcessor) readInput(input io.Reader) ([]byte, error) {
-	data, err := io.ReadAll(io.LimitReader(input, p.maxInputBytes+1))
-	if err != nil {
-		return nil, fmt.Errorf("failed to read input: %w", err)
-	}
-
-	if int64(len(data)) > p.maxInputBytes {
-		return nil, ErrInputDataTooLarge
-	}
-
-	return data, nil
-}
-
 // detectFormat returns the format string for a govips image type.
 func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string {
 	switch imageType {
@@ -499,67 +469,9 @@ func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string {
 	}
 }
 
-// outputDimensions returns the dimensions resize gives an image of imgWidth x
+// resize resizes the image according to the fit mode. With FitOutside one
-// imgHeight for the target dimensions and fit mode, without resizing it, and
+// side can come out larger than requested; when it would be above
-// refuses an output too large for format. With FitOutside one side can come
+// MaxOutputDimension, resize returns ErrOutputTooLarge without resizing.
-// out larger than the target; when it would be above MaxOutputDimension,
-// outputDimensions returns ErrOutputTooLarge. An AVIF output of more than
-// MaxAVIFOutputPixels is ErrAVIFOutputTooLarge.
-func outputDimensions(
-	imgWidth, imgHeight, width, height int, fit FitMode, format Format,
-) (int, int, error) {
-	scaleW := float64(width) / float64(imgWidth)
-	scaleH := float64(height) / float64(imgHeight)
-	outputWidth, outputHeight := width, height
-
-	switch fit {
-	case FitCover, FitFill, "":
-		// Cover crops, and fill stretches, to exactly the target dimensions
-
-	case FitContain:
-		// Fit within the target dimensions, maintaining aspect ratio. Scaled
-		// by the smaller scale, the short side of a thin image can round
-		// down to 0, which libvips refuses as a width, so each side is at
-		// least 1.
-		scale := min(scaleW, scaleH)
-		outputWidth = max(1, int(float64(imgWidth)*scale))
-		outputHeight = max(1, int(float64(imgHeight)*scale))
-
-	case FitInside:
-		// Same as contain, but only shrink
-		scale := min(scaleW, scaleH, 1)
-		outputWidth = max(1, int(float64(imgWidth)*scale))
-		outputHeight = max(1, int(float64(imgHeight)*scale))
-
-	case FitOutside:
-		// The smallest dimension fits, the other may exceed the target. A
-		// side scaled to 1 can come out just under 1 in floating point, as
-		// 49 times 1/49 does, and round down to 0, which libvips refuses as
-		// a width, so each side is at least 1.
-		scale := max(scaleW, scaleH)
-		outputWidth = max(1, int(float64(imgWidth)*scale))
-		outputHeight = max(1, int(float64(imgHeight)*scale))
-
-	default:
-		return 0, 0, fmt.Errorf("%w: %s", ErrInvalidFitMode, fit)
-	}
-
-	// The side FitOutside scales past the target is held to the maximum too
-	if max(outputWidth, outputHeight) > MaxOutputDimension {
-		return 0, 0, fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge,
-			outputWidth, outputHeight, MaxOutputDimension)
-	}
-
-	if format == FormatAVIF && outputWidth*outputHeight > MaxAVIFOutputPixels {
-		return 0, 0, fmt.Errorf("%w: %dx%d is above %d pixels",
-			ErrAVIFOutputTooLarge, outputWidth, outputHeight, MaxAVIFOutputPixels)
-	}
-
-	return outputWidth, outputHeight, nil
-}
-
-// resize resizes the image to width x height, the dimensions
-// outputDimensions gives for the fit mode.
 func (p *ImageProcessor) resize(
 	img *vips.ImageRef, width, height int, fit FitMode,
 ) error {
@@ -568,20 +480,58 @@ func (p *ImageProcessor) resize(
 		// Crop the centre and resize to fill exact dimensions (default)
 		return cropCentreAndResize(img, width, height)
 
+	case FitContain:
+		// Resize to fit within dimensions, maintaining aspect ratio. Scaled
+		// by the smaller scale, the short side of a thin image can round
+		// down to 0, which libvips refuses as a width, so each side is at
+		// least 1.
+		imgW, imgH := img.Width(), img.Height()
+		scaleW := float64(width) / float64(imgW)
+		scaleH := float64(height) / float64(imgH)
+		scale := min(scaleW, scaleH)
+		newW := max(1, int(float64(imgW)*scale))
+		newH := max(1, int(float64(imgH)*scale))
+
+		return img.Thumbnail(newW, newH, vips.InterestingNone)
+
 	case FitFill:
 		// Resize to exact dimensions (may distort)
 		return img.ThumbnailWithSize(width, height, vips.InterestingNone, vips.SizeForce)
 
 	case FitInside:
-		// outputDimensions keeps the dimensions of an image that already fits
+		// Same as contain, but only shrink
-		if img.Width() == width && img.Height() == height {
+		if img.Width() <= width && img.Height() <= height {
-			return nil
+			return nil // Already fits
 		}
 
-		return img.Thumbnail(width, height, vips.InterestingNone)
+		imgW, imgH := img.Width(), img.Height()
+		scaleW := float64(width) / float64(imgW)
+		scaleH := float64(height) / float64(imgH)
+		scale := min(scaleW, scaleH)
+		newW := max(1, int(float64(imgW)*scale))
+		newH := max(1, int(float64(imgH)*scale))
 
-	case FitContain, FitOutside:
+		return img.Thumbnail(newW, newH, vips.InterestingNone)
-		return img.Thumbnail(width, height, vips.InterestingNone)
+
+	case FitOutside:
+		// Resize so smallest dimension fits, may exceed target on other
+		// dimension. A side scaled to 1 can come out just under 1 in
+		// floating point, as 49 times 1/49 does, and round down to 0, which
+		// libvips refuses as a width, so each side is at least 1.
+		imgW, imgH := img.Width(), img.Height()
+		scaleW := float64(width) / float64(imgW)
+		scaleH := float64(height) / float64(imgH)
+		scale := max(scaleW, scaleH)
+		newW := max(1, int(float64(imgW)*scale))
+		newH := max(1, int(float64(imgH)*scale))
+
+		// The dimension that exceeds the target is held to the maximum too
+		if newW > MaxOutputDimension || newH > MaxOutputDimension {
+			return fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge,
+				newW, newH, MaxOutputDimension)
+		}
+
+		return img.Thumbnail(newW, newH, vips.InterestingNone)
 
 	default:
 		return fmt.Errorf("%w: %s", ErrInvalidFitMode, fit)
@@ -717,16 +667,11 @@ func exportGIF(img *vips.ImageRef) ([]byte, error) {
 	return output, err
 }
 
-// webpEffort is the WebP effort, from 0 (fastest) to 6. It is below libvips'
+// webpEffort is libvips' default WebP effort, from 0 (fastest) to 6.
-// default of 4 so that an encode takes at most 40 seconds of the default
+const webpEffort = 4
-// downstream_timeout of 60 seconds. With one thread, as pixad runs libvips,
-// saving an 8192x8192 image of random pixels with an alpha channel takes up
-// to about 23 seconds at 2, and up to about 16 without one, at any quality;
-// with an alpha channel, 3 takes about 50 seconds and 4 about 61.
-const webpEffort = 2
 
-// exportWebP encodes img as lossy WebP at quality and webpEffort, without
+// exportWebP encodes img as lossy WebP at quality and libvips' default
-// metadata.
+// effort, without metadata.
 func exportWebP(img *vips.ImageRef, quality int) ([]byte, error) {
 	output, _, err := img.ExportWebp(&vips.WebpExportParams{
 		StripMetadata:   true,
@@ -738,13 +683,12 @@ func exportWebP(img *vips.ImageRef, quality int) ([]byte, error) {
 }
 
 // avifEffort is the AVIF effort, from 0 (fastest) to 9; 1 is the lowest
-// govips can set. With one thread, as pixad runs libvips, 1 takes about 52
+// govips can set. With one thread, as pixad runs libvips, 1 takes about 51
-// seconds to save an 8192x8192 RGB image of random pixels at quality 85,
+// seconds to save an 8192x8192 RGB image of random pixels, and about 110
-// and about 96 with an alpha channel, against the default
+// with an alpha channel, the worst case, against the default
-// downstream_timeout of 60 seconds, so an AVIF output is held to
+// downstream_timeout of 60 seconds. On an image of milder noise, which 1
-// MaxAVIFOutputPixels. On an image of milder noise, which 1 saves in about
+// saves in about 12 seconds, 2 takes nearly a minute and libvips' default,
-// 12 seconds, 2 takes nearly a minute and libvips' default, 4, takes
+// 4, takes minutes.
-// minutes.
 const avifEffort = 1
 
 // avifBitdepth is the AVIF bit depth, 8 bits per sample for every image.
@@ -771,18 +715,10 @@ func exportAVIF(img *vips.ImageRef, quality int) ([]byte, error) {
 // JPEG that names none.
 const jxlResolution = 72 / 25.4
 
-// jxlEffort is the JPEG XL effort, from 1 (fastest) to 9. It is below
+// exportJXL encodes img as JPEG XL at quality, with libvips' default effort
-// libvips' default of 7 so that an encode takes at most 40 seconds of the
+// and without metadata. govips sends libvips a distance, the JPEG XL
-// default downstream_timeout of 60 seconds. With one thread, as pixad runs
+// encoder's own measure of quality, along with the quality, and libvips then
-// libvips, saving an 8192x8192 image of random pixels with an alpha channel
+// uses the distance alone, so the quality is also given as a distance.
-// takes up to about 27 seconds at 4, and up to about 13 without one, at any
-// quality; with an alpha channel, 5 takes about 43 seconds and 7 about 81.
-const jxlEffort = 4
-
-// exportJXL encodes img as JPEG XL at quality and jxlEffort, without
-// metadata. govips sends libvips a distance, the JPEG XL encoder's own
-// measure of quality, along with the quality, and libvips then uses the
-// distance alone, so the quality is also given as a distance.
 func exportJXL(img *vips.ImageRef, quality int) ([]byte, error) {
 	// libvips converts a CMYK image to sRGB before it saves WebP, AVIF or
 	// PNG, but cannot save one as JPEG XL. encode has already converted
@@ -821,7 +757,6 @@ func exportJXL(img *vips.ImageRef, quality int) ([]byte, error) {
 	params := vips.NewJxlExportParams()
 	params.Quality = quality
 	params.Distance = jxlDistance(quality)
-	params.Effort = jxlEffort
 
 	output, _, err := toSave.ExportJxl(params)
 	if err != nil {

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",
+		{"source fails the magic byte check, not kept",
-			"/images/text.png", magic.ErrUnknownFormat, ErrNegativeCached, 1},
+			"/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
+	// Validate magic bytes match content type
-	// upstream's error, so it gets a negative cache entry, as an upstream
-	// error status does.
 	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 {
@@ -658,7 +644,6 @@ func signatureRequest(req *ImageRequest) *signature.Request {
 const (
 	httpStatusBadGateway    = 502
 	httpStatusInternalError = 500
-	httpStatusUnsupportedMediaType = 415
 )
 
 // isNegativeCacheable returns true if the error should be cached.