Refuse SVG at the fetcher and never save an unknown input as JPEG (part of #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

@@ -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/<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
@@ -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:

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)
 }
 

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)

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())
+			}
+		}
+	}
+}

internal/httpfetcher/httpfetcher.go

@@ -158,7 +158,6 @@ func DefaultConfig() *Config {
 			contentTypeWebP,
 			contentTypeAVIF,
 			contentTypeJXL,
-			contentTypeSVG,
 		},
 		AllowHTTP:             false,
 		MaxConnectionsPerHost: DefaultMaxConnectionsPerHost,

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)
 	}
 }

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)
+	}
+}

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, `<svg xmlns="http://www.w3.org/2000/svg" `+
+				`width="64" height="48"><rect width="64" height="48"/></svg>`)
+		}))
+	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)
+		}
+	}
+}