From c5f318033d0a19e15d63248d0f3d23b2c2216f82 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 13 Oct 2026 01:33:28 +0000 Subject: [PATCH 1/5] Test that an SVG source and orig of an unnamed input are refused Failing tests, ahead of the fix: an upstream answering image/svg+xml is answered 415 on the image route, also for orig; both image routes answer 415 for an unaccepted upstream content type and for orig of an input the processor cannot name; and the processor refuses orig of a TIFF instead of saving it as JPEG. Model: opus-5-5 --- .../unsupported_source_internal_test.go | 54 +++++++++++++++++++ .../orig_format_internal_test.go | 39 ++++++++++++++ internal/server/svg_internal_test.go | 41 ++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 internal/handlers/unsupported_source_internal_test.go create mode 100644 internal/imageprocessor/orig_format_internal_test.go create mode 100644 internal/server/svg_internal_test.go diff --git a/internal/handlers/unsupported_source_internal_test.go b/internal/handlers/unsupported_source_internal_test.go new file mode 100644 index 0000000..a10e37e --- /dev/null +++ b/internal/handlers/unsupported_source_internal_test.go @@ -0,0 +1,54 @@ +package handlers + +import ( + "fmt" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "sneak.berlin/go/pixa/internal/httpfetcher" + "sneak.berlin/go/pixa/internal/imageprocessor" + "sneak.berlin/go/pixa/internal/imgcache" +) + +// TestUnsupportedSourceFormatAnswers415 checks that both image routes answer +// 415 with a clear error when the upstream answers with a content type pixa +// does not accept, such as SVG, and when orig is asked of a source in a +// format the processor cannot name, wrapped as the service wraps them. +func TestUnsupportedSourceFormatAnswers415(t *testing.T) { + t.Parallel() + + h := &Handlers{log: slog.New(slog.DiscardHandler)} + req := &imgcache.ImageRequest{SourceHost: "img.example.com", SourcePath: "/a.svg"} + + for _, err := range []error{ + fmt.Errorf("upstream fetch failed: %w: image/svg+xml", + httpfetcher.ErrInvalidContentType), + fmt.Errorf("image processing failed: %w: unknown", + imageprocessor.ErrUnsupportedInputFormat), + } { + plain := httptest.NewRecorder() + h.respondImageError(plain, req, err) + + encrypted := httptest.NewRecorder() + h.handleImageError(encrypted, err) + + for route, rec := range map[string]*httptest.ResponseRecorder{ + "HandleImage": plain, "HandleImageEnc": encrypted, + } { + t.Logf("%s for %v: %d %s", route, err, rec.Code, rec.Body.String()) + + if rec.Code != http.StatusUnsupportedMediaType { + t.Errorf("%s for %v: status = %d, want %d", + route, err, rec.Code, http.StatusUnsupportedMediaType) + } + + if !strings.Contains(rec.Body.String(), "unsupported source format") { + t.Errorf("%s for %v: body = %q, want the unsupported source format error", + route, err, rec.Body.String()) + } + } + } +} diff --git a/internal/imageprocessor/orig_format_internal_test.go b/internal/imageprocessor/orig_format_internal_test.go new file mode 100644 index 0000000..f788b7c --- /dev/null +++ b/internal/imageprocessor/orig_format_internal_test.go @@ -0,0 +1,39 @@ +package imageprocessor + +import ( + "bytes" + "errors" + "testing" + + "github.com/davidbyttow/govips/v2/vips" +) + +// TestImageProcessor_OrigOfUnnamedFormatRefused verifies that orig of an +// input in a format the processor decodes but cannot name, here TIFF, is +// an error, and not the image saved as JPEG. +func TestImageProcessor_OrigOfUnnamedFormatRefused(t *testing.T) { + t.Parallel() + + img, err := vips.NewImageFromBuffer(createTestPNG(t, 20, 20)) + if err != nil { + t.Fatalf("failed to decode test PNG: %v", err) + } + + t.Cleanup(img.Close) + + tiff, _, err := img.ExportTiff(vips.NewTiffExportParams()) + if err != nil { + t.Fatalf("failed to save test TIFF: %v", err) + } + + result, err := New(Params{}).Process( + t.Context(), bytes.NewReader(tiff), &Request{Format: FormatOriginal}, + ) + if result != nil { + t.Logf("Process() returned %s", result.ContentType) + } + + if !errors.Is(err, ErrUnsupportedInputFormat) { + t.Errorf("Process() error = %v, want %v", err, ErrUnsupportedInputFormat) + } +} diff --git a/internal/server/svg_internal_test.go b/internal/server/svg_internal_test.go new file mode 100644 index 0000000..95b46fa --- /dev/null +++ b/internal/server/svg_internal_test.go @@ -0,0 +1,41 @@ +package server + +import ( + "io" + "net/http" + "net/http/httptest" + "testing" +) + +// TestImageProxyRefusesSVG requests an image the upstream answers as SVG, +// in its own format (orig) and resized as PNG, through pixa's router, +// upstream fetcher and image processor. pixa does not accept SVG as a +// source, so each request is answered 415, never with an image. +func TestImageProxyRefusesSVG(t *testing.T) { + t.Parallel() + + upstream := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "image/svg+xml") + _, _ = io.WriteString(w, ``) + })) + t.Cleanup(upstream.Close) + + s, _, _ := startImageProxy(t, upstream) + + for _, sizeFormat := range []string{"64x48.orig", "32x24.png"} { + target := "/v1/image/" + upstreamHost + "/drawing.svg/" + sizeFormat + + rec := httptest.NewRecorder() + s.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, target, nil)) + t.Logf("GET %s: %d, Content-Type %s, body %s", + target, rec.Code, rec.Header().Get("Content-Type"), rec.Body.String()) + + if rec.Code != http.StatusUnsupportedMediaType { + t.Errorf("GET %s status = %d, want %d", + target, rec.Code, http.StatusUnsupportedMediaType) + } + } +} -- 2.54.0 From 06e537c4580bdcced5e5186a5d3c59531c3d40dc Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 13 Oct 2026 01:35:35 +0000 Subject: [PATCH 2/5] Refuse SVG at the fetcher and never save an unknown input as JPEG (closes #68) The fetcher no longer accepts image/svg+xml, so an SVG source gets the refusal every unaccepted upstream type gets, which both image routes now answer with 415 where they answered 500. orig of an input the processor cannot name is ErrUnsupportedInputFormat, also answered 415, where it was saved as JPEG. README.md drops SVG from the source formats and lists the 415. Model: opus-5-5 --- README.md | 17 +++++++----- TODO.md | 10 +++++++ internal/handlers/image.go | 8 ++++++ internal/handlers/imageenc.go | 4 +++ internal/httpfetcher/httpfetcher.go | 1 - internal/imageprocessor/imageprocessor.go | 32 +++++++++++++++-------- 6 files changed, 53 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index e8e8df4..8200596 100644 --- a/README.md +++ b/README.md @@ -190,9 +190,11 @@ path under `/v1/` answers 200, in maintenance mode too. 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. + `image_requests_per_minute` (below); 415 when the upstream answers with a + `Content-Type` that is not one of the source formats (below), or for `orig` of + a source pixa cannot save in its own format; 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//` — 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 @@ -200,7 +202,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, 415, 429, 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`, @@ -260,9 +262,10 @@ once, is refused with 400. both optional (values under Signature Specification). Both are part of what is cached, so each value of either is a separate cached image. -The source image may be JPEG, PNG, GIF, WebP, AVIF, JPEG XL or SVG. The -upstream's `Content-Type` must name its format, and the image's first bytes must -match it. +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. With the format `auto`, pixa chooses the format for each request from its `Accept` header, in this order: diff --git a/TODO.md b/TODO.md index 66067f4..1a104cf 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,16 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-13 SVG sources are refused, and `orig` is never saved as JPEG in place + of the source's own format (closes #68): the upstream fetch no longer accepts + `image/svg+xml`, which libvips would render. Both image routes answer 415 for + an upstream `Content-Type` that is not one of the source formats, SVG + included, where they answered 500. `orig` of a source in a format the image + processor cannot name is `ErrUnsupportedInputFormat`, also answered 415, where + it was saved as JPEG. `README.md` no longer lists SVG. The SVG detection in + `internal/magic` stays: the request path does not call it, and it goes with + the unused code of https://git.eeqj.de/sneak/pixa/issues/168. An SVG sent + under another image type still fails the first-bytes check and answers 500. - 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 diff --git a/internal/handlers/image.go b/internal/handlers/image.go index 848a252..9978565 100644 --- a/internal/handlers/image.go +++ b/internal/handlers/image.go @@ -247,6 +247,14 @@ func (s *Handlers) respondImageError( return } + if errors.Is(err, httpfetcher.ErrInvalidContentType) || + errors.Is(err, imageprocessor.ErrUnsupportedInputFormat) { + s.respondError(w, "unsupported source format", + http.StatusUnsupportedMediaType) + + return + } + s.respondError(w, "internal error", http.StatusInternalServerError) } diff --git a/internal/handlers/imageenc.go b/internal/handlers/imageenc.go index bf79054..816e83f 100644 --- a/internal/handlers/imageenc.go +++ b/internal/handlers/imageenc.go @@ -161,6 +161,10 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) { errors.Is(err, imageprocessor.ErrTooManyImages): s.respondError(w, "server busy, try again later", http.StatusServiceUnavailable) + case errors.Is(err, httpfetcher.ErrInvalidContentType), + errors.Is(err, imageprocessor.ErrUnsupportedInputFormat): + s.respondError(w, "unsupported source format", + http.StatusUnsupportedMediaType) default: s.log.Error("image request failed", "error", err) s.respondError(w, "internal error", http.StatusInternalServerError) diff --git a/internal/httpfetcher/httpfetcher.go b/internal/httpfetcher/httpfetcher.go index dd37489..9d09295 100644 --- a/internal/httpfetcher/httpfetcher.go +++ b/internal/httpfetcher/httpfetcher.go @@ -158,7 +158,6 @@ func DefaultConfig() *Config { contentTypeWebP, contentTypeAVIF, contentTypeJXL, - contentTypeSVG, }, AllowHTTP: false, MaxConnectionsPerHost: DefaultMaxConnectionsPerHost, diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index 6d59b34..af58690 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -137,6 +137,11 @@ var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size") // not supported. var ErrUnsupportedOutputFormat = errors.New("unsupported output format") +// ErrUnsupportedInputFormat is returned when the requested format is orig and +// the input is in a format the processor cannot name, so it cannot be saved +// in that format. +var ErrUnsupportedInputFormat = errors.New("unsupported input format") + // ErrTooManyImages is returned when MaxConcurrentProcessing images are being // processed and none finishes within ProcessingWaitTimeout. var ErrTooManyImages = errors.New("too many images being processed at once") @@ -256,10 +261,14 @@ func (p *ImageProcessor) Process( } } - // orig is the source's own format; encode refuses an empty format + // orig is the source's own format, and an error when the processor + // cannot name it; encode refuses an empty format outputFormat := req.Format if outputFormat == FormatOriginal { - outputFormat = p.formatFromString(inputFormat) + outputFormat, err = p.formatFromString(inputFormat) + if err != nil { + return nil, err + } } // Encode to target format @@ -687,22 +696,23 @@ func jxlDistance(quality int) float64 { return 53.0/3000.0*q*q - 23.0/20.0*q + 25.0 } -// formatFromString converts a format string to Format. -func (p *ImageProcessor) formatFromString(format string) Format { +// formatFromString converts a format string from detectFormat to Format. A +// format detectFormat could not name is ErrUnsupportedInputFormat. +func (p *ImageProcessor) formatFromString(format string) (Format, error) { switch format { case "jpeg": - return FormatJPEG + return FormatJPEG, nil case "png": - return FormatPNG + return FormatPNG, nil case "gif": - return FormatGIF + return FormatGIF, nil case "webp": - return FormatWebP + return FormatWebP, nil case string(FormatAVIF): - return FormatAVIF + return FormatAVIF, nil case string(FormatJXL): - return FormatJXL + return FormatJXL, nil default: - return FormatJPEG + return "", fmt.Errorf("%w: %s", ErrUnsupportedInputFormat, format) } } -- 2.54.0 From d67b6d5051907dae59e3306d7732daafd092cc5d Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 13 Oct 2026 03:18:22 +0000 Subject: [PATCH 3/5] Test that an SVG served as AVIF is refused, and that each source format loads An SVG starting with an XML comment holding "ftypavis" passes the first-bytes check for AVIF; resized as PNG, it is answered 200 with the SVG rendered. The new server test asks for it through both image routes, resized and as orig, and wants 415 for each. The new processor test checks that an image in each of the six source formats loads and is saved again in its own format. Model: opus-5-5 --- .../source_formats_internal_test.go | 46 ++++++++++++ internal/server/svg_as_avif_internal_test.go | 71 +++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 internal/imageprocessor/source_formats_internal_test.go create mode 100644 internal/server/svg_as_avif_internal_test.go diff --git a/internal/imageprocessor/source_formats_internal_test.go b/internal/imageprocessor/source_formats_internal_test.go new file mode 100644 index 0000000..5f1dbbd --- /dev/null +++ b/internal/imageprocessor/source_formats_internal_test.go @@ -0,0 +1,46 @@ +package imageprocessor + +import ( + "bytes" + "io" + "testing" +) + +// TestImageProcessor_LoadsEachSourceFormat verifies that an image in each of +// the six source formats, saved by the processor itself, is loaded and saved +// again in its own format (orig). +func TestImageProcessor_LoadsEachSourceFormat(t *testing.T) { + t.Parallel() + + proc := New(Params{}) + + for _, format := range []Format{ + FormatJPEG, FormatPNG, FormatGIF, FormatWebP, FormatAVIF, FormatJXL, + } { + saved, err := proc.Process(t.Context(), + bytes.NewReader(createTestPNG(t, 20, 20)), &Request{Format: format}) + if err != nil { + t.Fatalf("saving the test image as %s: %v", format, err) + } + + source, err := io.ReadAll(saved.Content) + if err != nil { + t.Fatalf("reading the %s image: %v", format, err) + } + + result, err := proc.Process(t.Context(), bytes.NewReader(source), + &Request{Format: FormatOriginal}) + if err != nil { + t.Errorf("orig of the %s image: %v", format, err) + + continue + } + + t.Logf("orig of the %s image: %s", format, result.ContentType) + + if result.ContentType != FormatToMIME(format) { + t.Errorf("orig of the %s image is %s, want %s", + format, result.ContentType, FormatToMIME(format)) + } + } +} diff --git a/internal/server/svg_as_avif_internal_test.go b/internal/server/svg_as_avif_internal_test.go new file mode 100644 index 0000000..cd05947 --- /dev/null +++ b/internal/server/svg_as_avif_internal_test.go @@ -0,0 +1,71 @@ +package server + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/imgcache" +) + +// svgServedAsAVIF is an SVG whose first bytes pass the first-bytes check for +// AVIF: its XML comment puts "ftypavis" at bytes 4 to 11, where an AVIF file +// has its ftyp box and brand. libvips would load it as SVG. +const svgServedAsAVIF = `` + +// TestImageProxyRefusesSVGServedAsAVIF requests svgServedAsAVIF, which the +// upstream answers as image/avif, through both image routes, resized as PNG +// and in its own format (orig). Each request is answered 415, as for an SVG +// source, never with an image. +func TestImageProxyRefusesSVGServedAsAVIF(t *testing.T) { + t.Parallel() + + upstream := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "image/avif") + _, _ = io.WriteString(w, svgServedAsAVIF) + })) + t.Cleanup(upstream.Close) + + s, _, _ := startImageProxy(t, upstream) + + generator, err := encurl.NewGenerator(testSigningKey) + if err != nil { + t.Fatalf("encurl.NewGenerator: %v", err) + } + + encryptedURL := func(format imgcache.ImageFormat) string { + token, err := generator.Generate(&encurl.Payload{ + SourceHost: upstreamHost, SourcePath: "/drawing.avif", + Width: 32, Height: 24, Format: format, + }) + if err != nil { + t.Fatalf("encurl Generate: %v", err) + } + + return "/v1/e/" + token + "/drawing." + string(format) + } + + for _, target := range []string{ + "/v1/image/" + upstreamHost + "/drawing.avif/32x24.png", + "/v1/image/" + upstreamHost + "/drawing.avif/64x48.orig", + encryptedURL(imgcache.FormatPNG), + encryptedURL(imgcache.FormatOriginal), + } { + rec := httptest.NewRecorder() + s.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, target, nil)) + t.Logf("GET %s: %d, Content-Type %s, body %q", + target, rec.Code, rec.Header().Get("Content-Type"), rec.Body.String()) + + if rec.Code != http.StatusUnsupportedMediaType || + !strings.Contains(rec.Body.String(), "unsupported source format") { + t.Errorf("GET %s: %d %q, want %d unsupported source format", + target, rec.Code, rec.Body.String(), http.StatusUnsupportedMediaType) + } + } +} -- 2.54.0 From 6241c6a4250786beb6cae8dcf5439fc12f39220a Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 13 Oct 2026 03:20:52 +0000 Subject: [PATCH 4/5] Refuse, before decoding, input libvips would not load as a source format govips picks the libvips loader for the input with DetermineImageType. The image processor now names the format from that type before decoding, and refuses with ErrUnsupportedInputFormat, answered 415, any input it cannot name as one of the six source formats. An SVG whose first bytes pass the first-bytes check for AVIF therefore never reaches librsvg. orig is the format named there. README.md and TODO.md say so. Model: opus-5-5 --- README.md | 11 ++++---- TODO.md | 14 ++++++---- internal/imageprocessor/imageprocessor.go | 33 ++++++++++++----------- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 8200596..d1ab548 100644 --- a/README.md +++ b/README.md @@ -191,10 +191,10 @@ 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), or for `orig` of - a source pixa cannot save in its own format; 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. + `Content-Type` that is not one of the source formats (below), or with an image + libvips would not load as one of them; 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//` — 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 @@ -265,7 +265,8 @@ once, is refused with 400. 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. +with 415, and so is an image that libvips would not load as one of these +formats, 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: diff --git a/TODO.md b/TODO.md index 1a104cf..6cd9a0e 100644 --- a/TODO.md +++ b/TODO.md @@ -34,12 +34,16 @@ P2: security: per-origin rate limiting of the source's own format (closes #68): the upstream fetch no longer accepts `image/svg+xml`, which libvips would render. Both image routes answer 415 for an upstream `Content-Type` that is not one of the source formats, SVG - included, where they answered 500. `orig` of a source in a format the image - processor cannot name is `ErrUnsupportedInputFormat`, also answered 415, where - it was saved as JPEG. `README.md` no longer lists SVG. The SVG detection in + included, where they answered 500. Before decoding, the image processor + refuses with `ErrUnsupportedInputFormat`, also answered 415, any input that + govips' `DetermineImageType`, which picks the libvips loader, does not name as + a source format. That catches an SVG whose first bytes pass the first-bytes + check for another type, as an XML comment can make them do for AVIF; an SVG + whose first bytes do not pass that check answers 500, as before. `orig` is + always the source's own format, where a source the processor could not name + was saved as JPEG. `README.md` no longer lists SVG. The SVG detection in `internal/magic` stays: the request path does not call it, and it goes with - the unused code of https://git.eeqj.de/sneak/pixa/issues/168. An SVG sent - under another image type still fails the first-bytes check and answers 500. + the unused code of https://git.eeqj.de/sneak/pixa/issues/168. - 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 diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index af58690..1898ce7 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -137,9 +137,8 @@ var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size") // not supported. var ErrUnsupportedOutputFormat = errors.New("unsupported output format") -// ErrUnsupportedInputFormat is returned when the requested format is orig and -// the input is in a format the processor cannot name, so it cannot be saved -// in that format. +// ErrUnsupportedInputFormat is returned, before the input is decoded, when +// libvips would not load it as one of the source formats. var ErrUnsupportedInputFormat = errors.New("unsupported input format") // ErrTooManyImages is returned when MaxConcurrentProcessing images are being @@ -224,6 +223,16 @@ func (p *ImageProcessor) Process( return nil, ErrInputDataTooLarge } + // govips picks the libvips loader with DetermineImageType. Input it + // would not load as a source format, such as SVG, which librsvg would + // render, is refused before libvips reads it. + inputFormat := p.detectFormat(vips.DetermineImageType(data)) + + sourceFormat, err := p.formatFromString(inputFormat) + if err != nil { + return nil, err + } + // Decode image img, err := vips.NewImageFromBuffer(data) if err != nil { @@ -242,9 +251,6 @@ func (p *ImageProcessor) Process( origWidth := img.Width() origHeight := img.Height() - // Detect input format - inputFormat := p.detectFormat(img) - // Validate input dimensions to prevent DoS via decompression bombs if origWidth > MaxInputDimension || origHeight > MaxInputDimension { return nil, ErrInputTooLarge @@ -261,14 +267,10 @@ func (p *ImageProcessor) Process( } } - // orig is the source's own format, and an error when the processor - // cannot name it; encode refuses an empty format + // orig is the source's own format; encode refuses an empty format outputFormat := req.Format if outputFormat == FormatOriginal { - outputFormat, err = p.formatFromString(inputFormat) - if err != nil { - return nil, err - } + outputFormat = sourceFormat } // Encode to target format @@ -412,10 +414,9 @@ func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) { } } -// detectFormat returns the format string from a vips image. -func (p *ImageProcessor) detectFormat(img *vips.ImageRef) string { - format := img.Format() - switch format { +// detectFormat returns the format string for a govips image type. +func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string { + switch imageType { case vips.ImageTypeJPEG: return "jpeg" case vips.ImageTypePNG: -- 2.54.0 From c80e0e48b1369b099aa930f3091b501d4f3daac2 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 13 Oct 2026 04:24:12 +0000 Subject: [PATCH 5/5] Say which input the image processor refuses before decoding README.md, the doc comment of ErrUnsupportedInputFormat and the TODO.md entry say the 415 is for input whose first bytes libvips reads as another format, and no longer read as refusing a damaged image in a source format, which is decoded and answers 500. The doc comment of TestImageProcessor_OrigOfUnnamedFormatRefused says TIFF is refused before it is decoded. Model: opus-5-5 --- README.md | 11 ++++++----- TODO.md | 18 ++++++++++-------- internal/imageprocessor/imageprocessor.go | 11 +++++++---- .../orig_format_internal_test.go | 6 +++--- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index d1ab548..49c74bb 100644 --- a/README.md +++ b/README.md @@ -192,9 +192,10 @@ path under `/v1/` answers 200, in maintenance mode too. 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), or with an image - libvips would not load as one of them; 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. + whose first bytes libvips reads as another format; 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//` — 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 @@ -265,8 +266,8 @@ once, is refused with 400. 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, and so is an image that libvips would not load as one of these -formats, such as an SVG whose first bytes pass the check for AVIF. +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: diff --git a/TODO.md b/TODO.md index 6cd9a0e..f239092 100644 --- a/TODO.md +++ b/TODO.md @@ -36,14 +36,16 @@ P2: security: per-origin rate limiting an upstream `Content-Type` that is not one of the source formats, SVG included, where they answered 500. Before decoding, the image processor refuses with `ErrUnsupportedInputFormat`, also answered 415, any input that - govips' `DetermineImageType`, which picks the libvips loader, does not name as - a source format. That catches an SVG whose first bytes pass the first-bytes - check for another type, as an XML comment can make them do for AVIF; an SVG - whose first bytes do not pass that check answers 500, as before. `orig` is - always the source's own format, where a source the processor could not name - was saved as JPEG. `README.md` no longer lists SVG. The SVG detection in - `internal/magic` stays: the request path does not call it, and it goes with - the unused code of https://git.eeqj.de/sneak/pixa/issues/168. + govips' `DetermineImageType`, which picks the libvips loader from the first + bytes, does not name as a source format. That catches an SVG whose first bytes + pass the first-bytes check for another type, as an XML comment can make them + do for AVIF; an SVG whose first bytes do not pass that check answers 500, as + before, and so does a damaged image in a source format, whose first bytes + `DetermineImageType` names. `orig` is always the source's own format, where a + source the processor could not name was saved as JPEG. `README.md` no longer + lists SVG. The SVG detection in `internal/magic` stays: the request path does + not call it, and it goes with the unused code of + https://git.eeqj.de/sneak/pixa/issues/168. - 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 diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index 1898ce7..9f08387 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -138,7 +138,9 @@ var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size") var ErrUnsupportedOutputFormat = errors.New("unsupported output format") // ErrUnsupportedInputFormat is returned, before the input is decoded, when -// libvips would not load it as one of the source formats. +// libvips reads the input's first bytes as another format than the source +// formats, or as none: for example an SVG whose first bytes pass the +// first-bytes check for AVIF. var ErrUnsupportedInputFormat = errors.New("unsupported input format") // ErrTooManyImages is returned when MaxConcurrentProcessing images are being @@ -223,9 +225,10 @@ func (p *ImageProcessor) Process( return nil, ErrInputDataTooLarge } - // govips picks the libvips loader with DetermineImageType. Input it - // would not load as a source format, such as SVG, which librsvg would - // render, is refused before libvips reads it. + // govips picks the libvips loader from the input's first bytes with + // DetermineImageType. Input whose first bytes it reads as no source + // format, such as SVG, which librsvg would render, is refused before + // libvips reads it. inputFormat := p.detectFormat(vips.DetermineImageType(data)) sourceFormat, err := p.formatFromString(inputFormat) diff --git a/internal/imageprocessor/orig_format_internal_test.go b/internal/imageprocessor/orig_format_internal_test.go index f788b7c..453d176 100644 --- a/internal/imageprocessor/orig_format_internal_test.go +++ b/internal/imageprocessor/orig_format_internal_test.go @@ -8,9 +8,9 @@ import ( "github.com/davidbyttow/govips/v2/vips" ) -// TestImageProcessor_OrigOfUnnamedFormatRefused verifies that orig of an -// input in a format the processor decodes but cannot name, here TIFF, is -// an error, and not the image saved as JPEG. +// TestImageProcessor_OrigOfUnnamedFormatRefused verifies that orig of a TIFF +// input, which is not a source format, is refused with +// ErrUnsupportedInputFormat before it is decoded, and not saved as JPEG. func TestImageProcessor_OrigOfUnnamedFormatRefused(t *testing.T) { t.Parallel() -- 2.54.0