Serve JPEG XL when a request names no format (closes #222)

A /v1/image/ URL whose last segment is a size with no format, such as
800x600 or orig, is served as JPEG XL and signed as jxl, so it shares
the signature of the same URL ending in .jxl. An encrypted URL whose
token holds no format is served as JPEG XL, as encurl.DefaultFormat is
now jxl. The generator page selects JPEG XL by default, and a form
with an empty format, or none, makes a URL whose name ends in .jxl.

The image processor no longer takes an empty format as orig: both
routes give every request a format, so it refuses a request with none
instead of keeping a second default. auto still ends with JPEG.

Model: opus-5-5

README.md

@@ -46,9 +46,9 @@ everything in this list without further settings. The reverse proxy must:
   builds encrypted URLs from `Host`, and checks `Referer` against
   `referer_blocklist`;
 - set `X-Forwarded-For` to the client's address, with `trusted_proxies` set to
-  the address pixa sees the proxy's requests come from, so the login limit and
-  `image_requests_per_minute` count each client by its own address (see
-  `trusted_proxies` under Configuration);
+  the address pixa sees the proxy's requests come from, so the login limit
+  counts each client by its own address (see `trusted_proxies` under
+  Configuration);
 - wait for pixa's answer for at least `downstream_timeout` (default `60s`), the
   longest pixa takes to fetch, convert and send an image.
 
@@ -58,9 +58,8 @@ password there.
 
 pixa does the rest itself: it checks signatures and encrypted URLs, applies the
 allowlist, refuses upstream hosts with private or local addresses, limits login
-attempts, each client's image requests, upstream response size and image
-dimensions, and sends the security headers, `Strict-Transport-Security`
-included, with every response.
+attempts, upstream response size and image dimensions, and sends the security
+headers, `Strict-Transport-Security` included, with every response.
 
 The state directory (`state_dir`, `/var/lib/pixa` in the container) holds the
 database and the disk cache:
@@ -187,10 +186,10 @@ path under `/v1/` answers 200, in maintenance mode too.
   `Referer` names a host in `referer_blocklist`, checked before the signature,
   the cache and the upstream fetch; 403 when the upstream host, or a host it
   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); 502 when the upstream answered with an
-  error status, and for 5 minutes after that for the same source URL; 503 when
-  pixa is busy or in maintenance mode; 500 for any other failure.
+  address in a blocked network (see `blocked_networks`); 502 when the upstream
+  answered with an error status, and for 5 minutes after that for the same
+  source URL; 503 when pixa is busy or in maintenance mode; 500 for any other
+  failure.
 - `GET` or `HEAD` `/v1/e/<token>/<name>` — an image through an encrypted URL
   (see Encrypted URLs). Needs: nothing but the URL. Answers: 200; 304 when
   `If-None-Match` matches the image's `ETag`; 400 for a token that does not
@@ -198,7 +197,7 @@ path under `/v1/` answers 200, in maintenance mode too.
   expired; 504 when the upstream has not sent its response headers within
   `upstream_fetch_timeout`, but 500 when that time runs out while the image
   itself is still arriving; 400 for an `Accept` header that is not valid, and
-  406, 403, 429, 502, 503 and 500, as for `/v1/image/`.
+  406, 403, 502, 503 and 500, as for `/v1/image/`.
 - `GET /robots.txt` — asks every crawler to stay away (`Disallow: /`). Needs:
   nothing. Answers: 200.
 - `GET /.well-known/healthcheck.json` — JSON with `status` (`ok`), `now`,
@@ -314,14 +313,6 @@ its own `X-Forwarded-For`, whether it connects directly or through the proxy,
 because its own address is trusted too. Setting `trusted_proxies` to only the
 address pixa sees for requests that come through the proxy closes this.
 
-One client may send `image_requests_per_minute` requests per minute
-(default 600) to the image routes, `/v1/image/` and `/v1/e/` together, `GET` and
-`HEAD`, cache hits included; a request over the limit is refused with 429 and a
-`Retry-After` header. Clients are counted as for the login limit. A browser's
-CORS preflight is not counted. Behind a CDN or proxy whose addresses are not in
-`trusted_proxies`, every request counts as the same client, so such a deployment
-needs a higher limit, or `0`, which turns the limit off.
-
 ### Encrypted URLs
 
 An encrypted URL is an image URL made on pixa's own web page by someone who
@@ -491,7 +482,6 @@ tell. With no file, pixa uses the environment and the defaults.
 | `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20`                       |
 | `PIXA_UPSTREAM_CONNECTIONS`          | `upstream_connections`          | Concurrent connections to all upstream hosts together; default `64`          |
 | `PIXA_MAX_CONCURRENT_PROCESSING`     | `max_concurrent_processing`     | Images processed at once; default the number of CPUs                         |
-| `PIXA_IMAGE_REQUESTS_PER_MINUTE`     | `image_requests_per_minute`     | Image requests per client per minute; `0` turns it off; default `600`        |
 | `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`                 |
@@ -541,13 +531,13 @@ Key settings in more detail:
   explicit list replaces the default. An invalid CIDR aborts startup. Set this
   to the address pixa sees for requests that come through your proxy, such as
   `172.17.0.1/32`, when the defaults do not cover it, or to trust nothing else
-  (see the login limit and `image_requests_per_minute` under Routes). For a
-  proxy on the Docker host that connects to pixa over `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
+  (see the login limit under Routes). For a proxy on the Docker host that
+  connects to pixa over `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;
@@ -580,11 +570,6 @@ Key settings in more detail:
   container's CPU limit. A request that finds all of them in use waits up to 10
   seconds for one to free up; if none does, and `downstream_timeout` has not
   ended first, it is answered 503 the same way
-- `image_requests_per_minute` — how many requests one client may send to the
-  image routes per minute (see the limit on them under Routes); default `600`.
-  `0` turns the limit off, and a negative or non-integer value aborts startup.
-  Behind a CDN or proxy whose addresses are not in `trusted_proxies`, every
-  request counts as the same client, so raise the limit there, or set `0`
 - `maintenance_mode` — while `true`, the image routes (`/v1/image/` and
   `/v1/e/`) answer every request for an image with 503, a `Retry-After` header
   and a JSON error body. The health check (`/.well-known/healthcheck.json`)

TODO.md

@@ -26,33 +26,10 @@ resolved. The disk cache is now size-bounded with LRU eviction
 
 # Next Step
 
-P2: security: per-origin rate limiting
+P2: security: per-IP rate limiting on the image routes
 
 # Completed Steps
 
-- 2026-10-12 per-client limit on the image routes (closes #236): the new setting
-  `image_requests_per_minute` (`PIXA_IMAGE_REQUESTS_PER_MINUTE`), default 600,
-  is how many `GET` and `HEAD` requests one client may send to `/v1/image/` and
-  `/v1/e/` together per minute, cache hits included; the next is answered 429
-  with `Retry-After`. It uses the login limit's `RateLimit` middleware, so
-  clients are told apart the same way, and runs after the CORS middleware, so a
-  preflight is not counted. `0` turns the limit off; a negative or non-integer
-  value aborts startup.
-- 2026-10-08 every output format is saved with settings pixa sets on purpose
-  (closes #232): each format has its own govips export, as JPEG XL does, in
-  place of govips' generic `Export`, which sent libvips a zero for some settings
-  it was not given. PNG gets libvips' default compression, 6, where it had none,
-  and WebP libvips' default effort, 4, where it had 0. GIF was already at
-  libvips' default effort, 7, and JPEG output is unchanged. AVIF is saved at
-  effort 1, the lowest govips can set, where it had libvips' default, 4. With
-  one libvips thread, an 8192x8192 image of random pixels, the worst case, takes
-  about 51 seconds as AVIF at effort 1 and 43 as WebP at effort 4, against the
-  default `downstream_timeout` of 60 seconds. On an image of milder noise, which
-  AVIF at effort 1 saves in about 12 seconds, effort 4 takes minutes. AVIF is
-  also saved with 8 bits per sample from a 16-bit source, which libvips would
-  save with 12: a 16-bit 8192x8192 image of milder noise takes about 54 seconds
-  at effort 1 with 12 bits, nearly all of the default `downstream_timeout`, and
-  about 12 with 8.
 - 2026-10-08 JPEG XL is the default output (closes #222): a `/v1/image/` URL
   whose last segment is a size with no format, such as `800x600` or `orig`, is
   served as JPEG XL and signed as `jxl`, so it has the signature of the same URL
@@ -744,6 +721,8 @@ P2: security: per-origin rate limiting
 
 # Future Steps
 
+- P2: security
+    - per-origin rate limiting
 - P2: HTTP response handling
     - Last-Modified headers
 - P2: configuration

configs/config.example.yml

@@ -115,14 +115,6 @@ upstream_connections: 64
 # frees up it is answered 503, unless downstream_timeout has ended first.
 # max_concurrent_processing: 4
 
-# How many requests to the image routes (/v1/image/ and /v1/e/) one client
-# may send per minute; a request over the limit is answered 429 with a
-# Retry-After header. 0 turns the limit off. Clients are told apart as for
-# the login limit, so behind a CDN or proxy whose addresses are not in
-# trusted_proxies every request counts as the same client: raise the limit
-# there, or set 0. (default: 600)
-image_requests_per_minute: 600
-
 # Time allowed for one fetch from an upstream host (default: 30s)
 upstream_fetch_timeout: 30s
 

internal/config/config.go

@@ -34,7 +34,6 @@ const (
 	DefaultUpstreamFetchTimeout       = 30 * time.Second
 	DefaultUpstreamMaxResponseSize    = 50 << 20 // 50 MiB
 	DefaultDownstreamTimeout          = 60 * time.Second
-	DefaultImageRequestsPerMinute     = 600
 )
 
 // Configuration key names.
@@ -62,7 +61,6 @@ const (
 	keyUpstreamFetchTimeout       = "upstream_fetch_timeout"
 	keyUpstreamMaxResponseSize    = "upstream_max_response_size"
 	keyDownstreamTimeout          = "downstream_timeout"
-	keyImageRequestsPerMinute     = "image_requests_per_minute"
 )
 
 // placeholderSigningKey is the dummy signing_key shipped in
@@ -160,11 +158,6 @@ type Config struct {
 	// of the per-request timeout middleware.
 	DownstreamTimeout time.Duration
 
-	// ImageRequestsPerMinute is how many requests one client may make to
-	// the image routes per minute; the next is refused with 429 Too Many
-	// Requests. 0 turns the limit off.
-	ImageRequestsPerMinute int
-
 	// BlockedNetworks are operator-supplied CIDR ranges to refuse in
 	// addition to the built-in SSRF blocklist. Enforced by the upstream
 	// fetcher's dialer; the built-in ranges always apply.
@@ -309,8 +302,6 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 			keyAccessControlAllowOrigin, DefaultAccessControlAllowOrigin),
 		DownstreamTimeout: loader.durationVal(
 			keyDownstreamTimeout, DefaultDownstreamTimeout),
-		ImageRequestsPerMinute: loader.intVal(
-			keyImageRequestsPerMinute, DefaultImageRequestsPerMinute),
 		CacheMaxBytes:    loader.int64Val(keyCacheMaxBytes, 0),
 		BlockedNetworks:  blockedNetworks,
 		TrustedProxies:   trustedProxies,
@@ -320,7 +311,9 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 	// The default for an omitted cache_max_bytes is worked out when
 	// the cache opens; here we only record whether the operator set
 	// the key explicitly.
-	_, c.CacheMaxBytesExplicit = lookupValue(sc, keyCacheMaxBytes)
+	if _, present := lookupValue(sc, keyCacheMaxBytes); present {
+		c.CacheMaxBytesExplicit = true
+	}
 
 	// Build DBURL from StateDir if not explicitly set. The derived URL
 	// is a default: it applies only when db_url is omitted, never to an
@@ -433,7 +426,7 @@ func isKnownConfigKey(key string) bool {
 		keyMaxConcurrentProcessing, keyCacheMaxBytes, keyBlockedNetworks,
 		keyTrustedProxies, keyAccessControlAllowOrigin, keyUpstreamFetchTimeout,
 		keyUpstreamMaxResponseSize, keyDownstreamTimeout, keyRefererBlocklist,
-		keyImageRequestsPerMinute, "env":
+		"env":
 		return true
 	}
 
@@ -468,7 +461,6 @@ func envVarNames() map[string]string {
 		keyUpstreamFetchTimeout:       "PIXA_UPSTREAM_FETCH_TIMEOUT",
 		keyUpstreamMaxResponseSize:    "PIXA_UPSTREAM_MAX_RESPONSE_SIZE",
 		keyDownstreamTimeout:          "PIXA_DOWNSTREAM_TIMEOUT",
-		keyImageRequestsPerMinute:     "PIXA_IMAGE_REQUESTS_PER_MINUTE",
 	}
 }
 
@@ -621,14 +613,6 @@ func (c *Config) validate() error {
 			settingName(keyCacheMaxBytes), c.CacheMaxBytes, errMustNotBeNegative)
 	}
 
-	// Zero is valid (it turns the limit off); only negative values are
-	// rejected.
-	if c.ImageRequestsPerMinute < 0 {
-		return fmt.Errorf("%s: value %d %w",
-			settingName(keyImageRequestsPerMinute), c.ImageRequestsPerMinute,
-			errMustNotBeNegative)
-	}
-
 	err = c.validateUpstreamMaxResponseSize()
 	if err != nil {
 		return err
@@ -641,9 +625,12 @@ func (c *Config) validate() error {
 		}
 	}
 
-	err = c.validateSentryDSN()
-	if err != nil {
-		return err
+	if c.SentryDSN != "" {
+		parsed, err := url.Parse(c.SentryDSN)
+		if err != nil || parsed.Scheme == "" || parsed.Host == "" {
+			return fmt.Errorf("%s: value %q is %w",
+				settingName(keySentryDSN), c.SentryDSN, errNotAValidURL)
+		}
 	}
 
 	if (c.MetricsUsername == "") != (c.MetricsPassword == "") {
@@ -655,22 +642,6 @@ func (c *Config) validate() error {
 	return c.validateAccessControlAllowOrigin()
 }
 
-// validateSentryDSN checks that sentry_dsn, when set, is a URL with a scheme
-// and a host.
-func (c *Config) validateSentryDSN() error {
-	if c.SentryDSN == "" {
-		return nil
-	}
-
-	parsed, err := url.Parse(c.SentryDSN)
-	if err != nil || parsed.Scheme == "" || parsed.Host == "" {
-		return fmt.Errorf("%s: value %q is %w",
-			settingName(keySentryDSN), c.SentryDSN, errNotAValidURL)
-	}
-
-	return nil
-}
-
 // validateUpstreamMaxResponseSize checks that upstream_max_response_size
 // is from 1 byte to 1 GiB. An upstream response is read whole into
 // memory, and the image processor reads one byte past this limit, which

internal/config/env_internal_test.go

@@ -70,7 +70,6 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
 	t.Setenv("PIXA_UPSTREAM_CONNECTIONS_PER_HOST", "5")
 	t.Setenv("PIXA_UPSTREAM_CONNECTIONS", "10")
 	t.Setenv("PIXA_MAX_CONCURRENT_PROCESSING", "3")
-	t.Setenv("PIXA_IMAGE_REQUESTS_PER_MINUTE", "30")
 	t.Setenv("PIXA_CACHE_MAX_BYTES", "1024")
 	t.Setenv("PIXA_BLOCKED_NETWORKS", "203.0.113.0/24")
 	t.Setenv("PIXA_TRUSTED_PROXIES", "192.0.2.0/24")
@@ -100,7 +99,6 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
 		UpstreamConnectionsPerHost: 5,
 		UpstreamConnections:        10,
 		MaxConcurrentProcessing:    3,
-		ImageRequestsPerMinute:     30,
 		CacheMaxBytes:              1024,
 		CacheMaxBytesExplicit:      true,
 		BlockedNetworks:            []netip.Prefix{netip.MustParsePrefix("203.0.113.0/24")},

internal/config/image_requests_per_minute_internal_test.go

@@ -1,107 +0,0 @@
-package config
-
-import "testing"
-
-// testImageRequestsPerMinuteVar is the variable that sets
-// image_requests_per_minute.
-const testImageRequestsPerMinuteVar = "PIXA_IMAGE_REQUESTS_PER_MINUTE"
-
-// TestImageRequestsPerMinuteValues checks that an omitted
-// image_requests_per_minute is 600, and that 0, which turns the limit off,
-// and any larger whole number are used as given.
-func TestImageRequestsPerMinuteValues(t *testing.T) {
-	t.Parallel()
-
-	c, err := configFromYAML(t, signingKeyLine)
-	if err != nil {
-		t.Fatalf("minimal config should be valid, got error: %v", err)
-	}
-
-	if c.ImageRequestsPerMinute != 600 {
-		t.Errorf("ImageRequestsPerMinute = %d, want 600", c.ImageRequestsPerMinute)
-	}
-
-	explicit := []struct {
-		value string
-		want  int
-	}{
-		{"0", 0},
-		{"30", 30},
-	}
-
-	for _, tc := range explicit {
-		c, err := configFromYAML(t,
-			signingKeyLine+"image_requests_per_minute: "+tc.value+"\n")
-		if err != nil {
-			t.Fatalf("image_requests_per_minute: %s should load, got error: %v",
-				tc.value, err)
-		}
-
-		if c.ImageRequestsPerMinute != tc.want {
-			t.Errorf("ImageRequestsPerMinute = %d, want %d",
-				c.ImageRequestsPerMinute, tc.want)
-		}
-	}
-}
-
-// TestInvalidImageRequestsPerMinuteAbortsStartup checks that a value that is
-// negative, not a whole number, or null aborts startup naming the key and
-// the value, and the variable too where the value could have come from it.
-func TestInvalidImageRequestsPerMinuteAbortsStartup(t *testing.T) {
-	t.Parallel()
-
-	key := keyImageRequestsPerMinute
-
-	runAbortCases(t, []abortCase{
-		{
-			name: "negative",
-			yaml: signingKeyLine + key + ": -1\n",
-			wantErrSubstrings: []string{
-				key, testImageRequestsPerMinuteVar, "value -1",
-			},
-		},
-		{
-			name: "not a number",
-			yaml: signingKeyLine + key + ": plenty\n",
-			wantErrSubstrings: []string{
-				key, testImageRequestsPerMinuteVar, "plenty",
-			},
-		},
-		{
-			name:              "fractional",
-			yaml:              signingKeyLine + key + ": 2.5\n",
-			wantErrSubstrings: []string{key, "2.5"},
-		},
-		{
-			name:              "explicit null",
-			yaml:              signingKeyLine + key + ": null\n",
-			wantErrSubstrings: []string{key, nullValueText},
-		},
-	})
-}
-
-// TestImageRequestsPerMinuteFromEnvironment checks that the variable sets the
-// limit over the config file, and that an invalid value in it aborts startup
-// naming the variable and the value.
-func TestImageRequestsPerMinuteFromEnvironment(t *testing.T) {
-	t.Setenv(testImageRequestsPerMinuteVar, "0")
-
-	c, err := configFromYAML(t, signingKeyLine+"image_requests_per_minute: 30\n")
-	if err != nil {
-		t.Fatalf("limit from the environment should load: %v", err)
-	}
-
-	if c.ImageRequestsPerMinute != 0 {
-		t.Errorf("ImageRequestsPerMinute = %d, want 0 from the environment",
-			c.ImageRequestsPerMinute)
-	}
-
-	for _, value := range []string{"-5", "ten"} {
-		t.Run(value, func(t *testing.T) {
-			t.Setenv(testImageRequestsPerMinuteVar, value)
-
-			_, err := configFromYAML(t, signingKeyLine)
-			wantStartupError(t, err, testImageRequestsPerMinuteVar, value)
-		})
-	}
-}

internal/imageprocessor/encoder_settings_internal_test.go

@@ -1,145 +0,0 @@
-package imageprocessor
-
-import (
-	"bytes"
-	"image"
-	"image/color"
-	"image/png"
-	"testing"
-
-	"github.com/davidbyttow/govips/v2/vips"
-)
-
-// processedSize runs input through Process and returns the output's size in
-// bytes.
-func processedSize(t *testing.T, input []byte, req *Request) int64 {
-	t.Helper()
-
-	result, err := New(Params{}).Process(t.Context(), bytes.NewReader(input), req)
-	if err != nil {
-		t.Fatalf("Process() error = %v", err)
-	}
-
-	_ = result.Content.Close()
-
-	return result.ContentLength
-}
-
-// encodePNG encodes img as PNG with Go's encoder.
-func encodePNG(t *testing.T, img image.Image) []byte {
-	t.Helper()
-
-	var buf bytes.Buffer
-
-	err := png.Encode(&buf, img)
-	if err != nil {
-		t.Fatalf("failed to encode test PNG: %v", err)
-	}
-
-	return buf.Bytes()
-}
-
-// decode decodes input with vips, as Process does.
-func decode(t *testing.T, input []byte) *vips.ImageRef {
-	t.Helper()
-
-	img, err := vips.NewImageFromBuffer(input)
-	if err != nil {
-		t.Fatalf("failed to decode input: %v", err)
-	}
-
-	t.Cleanup(img.Close)
-
-	return img
-}
-
-// TestImageProcessor_PNGIsCompressed verifies that a PNG output is
-// compressed: a flat 200x150 image, 90,000 bytes of raw pixels, comes out at
-// a small fraction of that.
-func TestImageProcessor_PNGIsCompressed(t *testing.T) {
-	t.Parallel()
-
-	const width, height = 200, 150
-
-	flat := image.NewRGBA(image.Rect(0, 0, width, height))
-	for y := range height {
-		for x := range width {
-			flat.Set(x, y, color.RGBA{R: 40, G: 120, B: 200, A: 255})
-		}
-	}
-
-	size := processedSize(t, encodePNG(t, flat), &Request{Format: FormatPNG})
-
-	const rawBytes = width * height * 3
-	if size > rawBytes/10 {
-		t.Errorf("PNG output is %d bytes, want under a tenth of its %d raw",
-			size, rawBytes)
-	}
-}
-
-// TestImageProcessor_WebPAtDefaultEffort verifies that WebP is saved at
-// libvips' default effort, 4: the output is the size of the same image saved
-// at that effort.
-func TestImageProcessor_WebPAtDefaultEffort(t *testing.T) {
-	t.Parallel()
-
-	input := createTestJPEG(t, 200, 150)
-
-	size := processedSize(t, input, &Request{Format: FormatWebP, Quality: 85})
-
-	want, _, err := decode(t, input).ExportWebp(&vips.WebpExportParams{
-		StripMetadata:   true,
-		Quality:         85,
-		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 4",
-			size, len(want))
-	}
-}
-
-// TestImageProcessor_AVIFAtEffort1 verifies that AVIF is saved at effort 1
-// with 8 bits per sample: the output is the size of the same image saved
-// with those settings. The source is 16-bit, which libvips saves with 12
-// bits when it is not given a bit depth, and 640x480: on a small image,
-// such as 64x48, efforts 1 and 2 give the same output.
-func TestImageProcessor_AVIFAtEffort1(t *testing.T) {
-	t.Parallel()
-
-	const width, height = 640, 480
-
-	source := image.NewRGBA64(image.Rect(0, 0, width, height))
-	for y := range height {
-		for x := range width {
-			source.Set(x, y, color.RGBA64{
-				R: uint16((x * 65535 / width) & 0xffff),
-				G: uint16((y * 65535 / height) & 0xffff),
-				B: 32768,
-				A: 65535,
-			})
-		}
-	}
-
-	input := encodePNG(t, source)
-
-	size := processedSize(t, input, &Request{Format: FormatAVIF, Quality: 85})
-
-	want, _, err := decode(t, input).ExportAvif(&vips.AvifExportParams{
-		StripMetadata: true,
-		Quality:       85,
-		Effort:        1,
-		Bitdepth:      8,
-	})
-	if err != nil {
-		t.Fatalf("ExportAvif() error = %v", err)
-	}
-
-	if size != int64(len(want)) {
-		t.Errorf("AVIF output is %d bytes, want %d, the size at effort 1 "+
-			"and 8 bits", size, len(want))
-	}
-}

internal/imageprocessor/imageprocessor.go

@@ -504,21 +504,36 @@ func (p *ImageProcessor) encode(
 		}
 	}
 
+	var params vips.ExportParams
+
 	switch format {
 	case FormatJPEG:
-		return exportJPEG(img, quality)
+		params = vips.ExportParams{
+			Format:  vips.ImageTypeJPEG,
+			Quality: quality,
+		}
 
 	case FormatPNG:
-		return exportPNG(img)
+		params = vips.ExportParams{
+			Format: vips.ImageTypePNG,
+		}
 
 	case FormatGIF:
-		return exportGIF(img)
+		params = vips.ExportParams{
+			Format: vips.ImageTypeGIF,
+		}
 
 	case FormatWebP:
-		return exportWebP(img, quality)
+		params = vips.ExportParams{
+			Format:  vips.ImageTypeWEBP,
+			Quality: quality,
+		}
 
 	case FormatAVIF:
-		return exportAVIF(img, quality)
+		params = vips.ExportParams{
+			Format:  vips.ImageTypeAVIF,
+			Quality: quality,
+		}
 
 	case FormatJXL:
 		return exportJXL(img, quality)
@@ -529,90 +544,17 @@ func (p *ImageProcessor) encode(
 	default:
 		return nil, fmt.Errorf("%w: %s", ErrUnsupportedOutputFormat, format)
 	}
-}
 
-// govips sends libvips Go's zero value for some settings an export leaves
-// out, such as no compression at all for PNG, so each export below sets
-// every setting whose zero value is not what pixa wants. Stripping metadata
-// drops EXIF, XMP, IPTC and the ICC profile.
+	// Drop EXIF, XMP, IPTC and the ICC profile. govips ignores this for
+	// GIF, which carries none of them.
+	params.StripMetadata = true
 
-// exportJPEG encodes img as JPEG at quality, without metadata. The settings
-// it leaves out are at libvips' defaults.
-func exportJPEG(img *vips.ImageRef, quality int) ([]byte, error) {
-	output, _, err := img.ExportJpeg(&vips.JpegExportParams{
-		StripMetadata: true,
-		Quality:       quality,
-	})
+	output, _, err := img.Export(&params)
+	if err != nil {
+		return nil, err
+	}
 
-	return output, err
-}
-
-// pngCompression is libvips' default PNG compression, from 0 (none) to 9.
-const pngCompression = 6
-
-// exportPNG encodes img as PNG at libvips' default compression and row
-// filter, without metadata.
-func exportPNG(img *vips.ImageRef) ([]byte, error) {
-	output, _, err := img.ExportPng(&vips.PngExportParams{
-		StripMetadata: true,
-		Compression:   pngCompression,
-		Filter:        vips.PngFilterNone,
-	})
-
-	return output, err
-}
-
-// gifEffort is libvips' default GIF effort, from 1 to 10.
-const gifEffort = 7
-
-// exportGIF encodes img as GIF at libvips' default effort. govips cannot
-// have libvips strip metadata from GIF, which carries none.
-func exportGIF(img *vips.ImageRef) ([]byte, error) {
-	output, _, err := img.ExportGIF(&vips.GifExportParams{Effort: gifEffort})
-
-	return output, err
-}
-
-// webpEffort is libvips' default WebP effort, from 0 (fastest) to 6.
-const webpEffort = 4
-
-// exportWebP encodes img as lossy WebP at quality and libvips' default
-// effort, without metadata.
-func exportWebP(img *vips.ImageRef, quality int) ([]byte, error) {
-	output, _, err := img.ExportWebp(&vips.WebpExportParams{
-		StripMetadata:   true,
-		Quality:         quality,
-		ReductionEffort: webpEffort,
-	})
-
-	return output, err
-}
-
-// 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 51
-// seconds to save an 8192x8192 image of random pixels, the worst case,
-// against the default downstream_timeout of 60 seconds. On an image of
-// milder noise, which 1 saves in about 12 seconds, 2 takes nearly a minute
-// and libvips' default, 4, takes minutes.
-const avifEffort = 1
-
-// avifBitdepth is the AVIF bit depth, 8 bits per sample for every image.
-// libvips would save a 16-bit image with 12, but at avifEffort that takes
-// about 54 seconds for a 16-bit 8192x8192 image of milder noise, nearly all
-// of the default downstream_timeout, and about 12 seconds with 8.
-const avifBitdepth = 8
-
-// exportAVIF encodes img as lossy AVIF at quality, avifEffort and
-// avifBitdepth, without metadata.
-func exportAVIF(img *vips.ImageRef, quality int) ([]byte, error) {
-	output, _, err := img.ExportAvif(&vips.AvifExportParams{
-		StripMetadata: true,
-		Quality:       quality,
-		Effort:        avifEffort,
-		Bitdepth:      avifBitdepth,
-	})
-
-	return output, err
+	return output, nil
 }
 
 // jxlResolution is the resolution every JPEG XL image is saved with, in

internal/imgcache/urlparser.go

@@ -75,8 +75,7 @@ func ParseImagePath(path string) (*ParsedURL, error) {
 	return parseImageComponents(path)
 }
 
-// ParseImageURL parses a full URL path like /v1/image/<host>/<path>/<size>.<format>,
-// or /v1/image/<host>/<path>/<size> for JPEG XL
+// ParseImageURL parses a full URL path like /v1/image/<host>/<path>/<size>.<format>
 // Use ParseImagePath instead when working with chi's wildcard capture.
 func ParseImageURL(urlPath string) (*ParsedURL, error) {
 	// Remove the /v1/image/ prefix
@@ -93,8 +92,7 @@ func ParseImageURL(urlPath string) (*ParsedURL, error) {
 	return parseImageComponents(remainder)
 }
 
-// parseImageComponents parses <host>/<path>/<size>.<format>, or
-// <host>/<path>/<size> for JPEG XL.
+// parseImageComponents parses <host>/<path>/<size>.<format> structure.
 func parseImageComponents(remainder string) (*ParsedURL, error) {
 	// Check for path traversal before any other processing
 	err := checkPathTraversal(remainder)
@@ -102,7 +100,7 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
 		return nil, err
 	}
 
-	// Find the last path segment, which holds "size" or "size.format"
+	// Find the last path segment which contains size.format
 	lastSlash := strings.LastIndex(remainder, "/")
 	if lastSlash == -1 {
 		return nil, ErrMissingSize
@@ -230,11 +228,11 @@ func parseSizeFormat(s string) (Size, ImageFormat, error) {
 	)
 
 	if matches[4] == "orig" {
-		// "orig" or "orig.format" pattern
+		// "orig.format" pattern
 		size = Size{Width: 0, Height: 0}
 		formatStr = matches[5]
 	} else {
-		// "WxH" or "WxH.format" pattern
+		// "WxH.format" pattern
 		width, err := strconv.Atoi(matches[1])
 		if err != nil {
 			return Size{}, "", ErrInvalidSize

internal/server/image_rate_limit_internal_test.go

@@ -1,152 +0,0 @@
-package server
-
-import (
-	"net/http"
-	"net/http/httptest"
-	"strconv"
-	"testing"
-
-	"sneak.berlin/go/pixa/internal/config"
-)
-
-// requestImageFrom sends method path to s from remoteAddr and returns the
-// response. The image handlers refuse an unsigned image URL with 401 and a
-// token they cannot decrypt with 400, so either status shows the limit let
-// the request through.
-func requestImageFrom(
-	t *testing.T, s *Server, method, path, remoteAddr string,
-) *httptest.ResponseRecorder {
-	t.Helper()
-
-	req := httptest.NewRequestWithContext(t.Context(), method, path, nil)
-	req.RemoteAddr = remoteAddr
-
-	rec := httptest.NewRecorder()
-	s.ServeHTTP(rec, req)
-
-	return rec
-}
-
-// TestImageRequestLimitRefusesRequestOverLimit verifies that GET and HEAD on
-// both image routes count toward one limit per client, and that the request
-// after image_requests_per_minute of them is refused with 429 and a
-// Retry-After header.
-func TestImageRequestLimitRefusesRequestOverLimit(t *testing.T) {
-	t.Parallel()
-
-	requests := []struct {
-		method string
-		path   string
-		want   int
-	}{
-		{http.MethodGet, unsignedImagePath, http.StatusUnauthorized},
-		{http.MethodHead, unsignedImagePath, http.StatusUnauthorized},
-		{http.MethodGet, encryptedImagePath, http.StatusBadRequest},
-	}
-
-	s := newTestServer(t)
-	s.config.ImageRequestsPerMinute = len(requests)
-	s.SetupRoutes()
-
-	for _, tc := range requests {
-		rec := requestImageFrom(t, s, tc.method, tc.path, firstClient)
-		if rec.Code != tc.want {
-			t.Fatalf("%s %s within the limit status = %d, want %d",
-				tc.method, tc.path, rec.Code, tc.want)
-		}
-	}
-
-	rec := requestImageFrom(t, s, http.MethodHead, encryptedImagePath, firstClient)
-	if rec.Code != http.StatusTooManyRequests {
-		t.Fatalf("request over the limit status = %d, want %d",
-			rec.Code, http.StatusTooManyRequests)
-	}
-
-	retryAfter := rec.Header().Get("Retry-After")
-
-	seconds, err := strconv.Atoi(retryAfter)
-	if err != nil || seconds <= 0 {
-		t.Errorf("Retry-After = %q, want a positive number of seconds",
-			retryAfter)
-	}
-}
-
-// TestImageRequestLimitCountsClientsSeparately verifies that one client going
-// over the limit does not limit another.
-func TestImageRequestLimitCountsClientsSeparately(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-	s.config.ImageRequestsPerMinute = 1
-	s.SetupRoutes()
-
-	rec := requestImageFrom(t, s, http.MethodGet, unsignedImagePath, firstClient)
-	if rec.Code != http.StatusUnauthorized {
-		t.Fatalf("first client's first request status = %d, want %d",
-			rec.Code, http.StatusUnauthorized)
-	}
-
-	rec = requestImageFrom(t, s, http.MethodGet, unsignedImagePath, firstClient)
-	if rec.Code != http.StatusTooManyRequests {
-		t.Fatalf("first client's second request status = %d, want %d",
-			rec.Code, http.StatusTooManyRequests)
-	}
-
-	rec = requestImageFrom(t, s, http.MethodGet, unsignedImagePath, secondClient)
-	if rec.Code != http.StatusUnauthorized {
-		t.Errorf("second client's first request status = %d, want %d",
-			rec.Code, http.StatusUnauthorized)
-	}
-}
-
-// TestImageRequestLimitZeroLetsEveryRequestThrough verifies that with
-// image_requests_per_minute set to 0 no image request is refused with 429,
-// not even past the default limit.
-func TestImageRequestLimitZeroLetsEveryRequestThrough(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-	s.config.ImageRequestsPerMinute = 0
-	s.SetupRoutes()
-
-	for request := range config.DefaultImageRequestsPerMinute + 1 {
-		rec := requestImageFrom(t, s, http.MethodGet, unsignedImagePath, firstClient)
-		if rec.Code != http.StatusUnauthorized {
-			t.Fatalf("request %d status = %d, want %d",
-				request+1, rec.Code, http.StatusUnauthorized)
-		}
-	}
-}
-
-// TestImageRequestLimitDoesNotCountPreflight verifies that a browser's CORS
-// preflight request to an image route is answered without being counted, so
-// a client that has sent several still gets its image request through.
-func TestImageRequestLimitDoesNotCountPreflight(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-	s.config.AccessControlAllowOrigin = config.DefaultAccessControlAllowOrigin
-	s.config.ImageRequestsPerMinute = 1
-	s.SetupRoutes()
-
-	for range 3 {
-		req := httptest.NewRequestWithContext(
-			t.Context(), http.MethodOptions, unsignedImagePath, nil)
-		req.RemoteAddr = firstClient
-		req.Header.Set("Origin", "https://app.example.com")
-		req.Header.Set("Access-Control-Request-Method", http.MethodGet)
-
-		rec := httptest.NewRecorder()
-		s.ServeHTTP(rec, req)
-
-		if rec.Code != http.StatusOK {
-			t.Fatalf("preflight status = %d, want %d", rec.Code, http.StatusOK)
-		}
-	}
-
-	rec := requestImageFrom(t, s, http.MethodGet, unsignedImagePath, firstClient)
-	if rec.Code != http.StatusUnauthorized {
-		t.Errorf("image request after preflights status = %d, want %d",
-			rec.Code, http.StatusUnauthorized)
-	}
-}

internal/server/routes.go

@@ -88,13 +88,6 @@ func (s *Server) SetupRoutes() {
 		r.Group(func(r chi.Router) {
 			r.Use(s.refuseDuringMaintenance)
 
-			// Limited per client, as the login POST is; 0 turns the limit
-			// off. The CORS middleware answers a preflight before it gets
-			// here, so a preflight is not counted.
-			if s.config.ImageRequestsPerMinute > 0 {
-				r.Use(s.mw.RateLimit(s.config.ImageRequestsPerMinute, time.Minute))
-			}
-
 			// Main image proxy route
 			// /v1/image/<host>/<path>/<width>x<height>.<format>, or with no
 			// format /v1/image/<host>/<path>/<width>x<height>