From 632fe0bd364e7d8eaf59ad1fd73fa77261c7e5b8 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 11 Nov 2026 00:38:16 +0000 Subject: [PATCH] Support SVG sources (closes #68) The fetcher accepts image/svg+xml again and internal/magic detects SVG. A raster format renders the SVG with librsvg at the density that makes it cover the output size, so an enlarged SVG stays sharp; the rendered size is held to MaxInputDimension. orig of an SVG at the orig size is the SVG unchanged, cached as a variant; at any other size it is ErrSVGNotOriginalSize, answered 400. The cache reads up to 1024 first bytes of a file to find its format, as an SVG's root element can come after a prolog. Tests that pinned SVG as refused now expect it to be read. Model: opus-5-5 --- README.md | 23 +- TODO.md | 6 + internal/handlers/image.go | 4 + .../handlers/image_errors_internal_test.go | 2 +- .../unsupported_source_internal_test.go | 6 +- internal/httpfetcher/httpfetcher.go | 1 + .../httpfetcher/httpfetcher_internal_test.go | 2 +- internal/httpfetcher/mock.go | 10 +- internal/imageprocessor/imageprocessor.go | 156 ++++++++++-- .../source_formats_internal_test.go | 2 +- internal/imageprocessor/svg_internal_test.go | 222 ++++++++++++++++++ internal/imagerequest/imagerequest.go | 6 +- internal/imgcache/cache.go | 16 +- internal/imgcache/cache_hit_internal_test.go | 2 - .../imgcache/source_content_internal_test.go | 2 - internal/imgcache/svg_internal_test.go | 54 +++++ internal/magic/magic.go | 28 ++- internal/magic/magic_internal_test.go | 6 +- internal/magic/svg_internal_test.go | 85 +++++++ internal/server/svg_internal_test.go | 112 +++++++-- 20 files changed, 669 insertions(+), 76 deletions(-) create mode 100644 internal/imageprocessor/svg_internal_test.go create mode 100644 internal/imgcache/svg_internal_test.go create mode 100644 internal/magic/svg_internal_test.go diff --git a/README.md b/README.md index c6593a2..2d12232 100644 --- a/README.md +++ b/README.md @@ -307,6 +307,7 @@ path under `/v1/` answers 200, in maintenance mode too. - 304 when `If-None-Match` matches the image's `ETag`. - 400 for a URL or parameter that is not valid. - 400 when the format is `auto` and the `Accept` header is not valid. + - 400 for `orig` of an SVG source at a size other than `orig`. - 406 when the format is `auto` and `Accept` allows none of the formats it chooses from. - 401 for a missing or wrong signature. @@ -344,6 +345,8 @@ path under `/v1/` answers 200, in maintenance mode too. - 304 when `If-None-Match` matches the image's `ETag`. - 400 for a token that does not decrypt. - 400 for a token that asks for a size or fit that is not valid. + - 400 for a token that asks for `orig` of an SVG source at a size other than + `orig`. - 410 once the token has expired. - 400 when the token's format is `auto` and the `Accept` header is not valid. @@ -457,9 +460,9 @@ 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 +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. An upstream answering with any other `Content-Type` is refused with 415. So is a body whose first bytes are not those of the format its `Content-Type` names, such as a JPEG sent as `image/png`, text sent as an image, or an empty body. pixa then does not fetch that source URL again for 5 minutes, @@ -468,6 +471,17 @@ 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. +For any format but `orig`, pixa renders an SVG source at the requested size, so +an enlarged SVG stays sharp. At the `orig` size it is rendered at its own size, +from the `width` and `height` of its root element. Rendering loads no file or +URL that the SVG refers to. + +`orig` of an SVG source is the SVG itself, unchanged, and is cached like any +other image. It is given only at the `orig` size, since pixa does not resize an +SVG as SVG: `orig` of an SVG at any other size is refused with 400. A browser +that opens an SVG from pixa runs none of its scripts, as pixa's +`Content-Security-Policy` allows no inline script. + With the format `auto`, pixa chooses the format for each request from its `Accept` header, in this order: @@ -607,7 +621,8 @@ session. pixa decodes and re-encodes every image it serves, and removes all metadata from the output: EXIF (GPS position, camera make, model and serial number, capture time, embedded thumbnail), XMP, IPTC and the ICC colour profile. This cannot be -turned off. +turned off. The one exception is `orig` of an SVG source, which is the SVG +unchanged, its metadata included. - The `orig` format means the source's own format, not the source's bytes: an `orig` image is re-encoded and stripped like any other. diff --git a/TODO.md b/TODO.md index 8416fe5..3983883 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,12 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-11-11 SVG is a source format again (closes #68), by the owner's ruling. + - The fetcher accepts `image/svg+xml`, and `internal/magic` detects SVG. + - A raster format renders the SVG at the output size, so it stays sharp. + - `orig` of an SVG at the `orig` size is the SVG unchanged, cached as a + variant. At any other size it is `ErrSVGNotOriginalSize`, answered 400. + - The cache reads up to 1024 first bytes of a file to find its format. - 2026-10-27 the reconciliation pass skips a directory it cannot list and a file whose info it cannot read, and goes on (closes #343). It stopped before its later checks at a directory it could not list, or at a source or variant file diff --git a/internal/handlers/image.go b/internal/handlers/image.go index ff11b6e..2d5352a 100644 --- a/internal/handlers/image.go +++ b/internal/handlers/image.go @@ -272,6 +272,10 @@ func (s *Handlers) respondImageError( message = fmt.Sprintf("size too large for the source: the side "+ "computed from its proportions is above %d", imageprocessor.MaxOutputDimension) + case errors.Is(err, imageprocessor.ErrSVGNotOriginalSize): + status = http.StatusBadRequest + message = "an SVG source is returned as SVG only at the orig size; " + + "ask for another format to resize it" case errors.Is(err, imageprocessor.ErrAVIFOutputTooLarge): status = http.StatusBadRequest message = fmt.Sprintf("size too large for AVIF: the output "+ diff --git a/internal/handlers/image_errors_internal_test.go b/internal/handlers/image_errors_internal_test.go index f4fee3c..b388ccc 100644 --- a/internal/handlers/image_errors_internal_test.go +++ b/internal/handlers/image_errors_internal_test.go @@ -467,7 +467,7 @@ func TestImageRoutes_SameErrorAnswers(t *testing.T) { {"no free upstream connection", httpfetcher.ErrTooManyConnections, http.StatusServiceUnavailable, slog.LevelWarn}, {"Content-Type not a source format", - fmt.Errorf("%w: image/svg+xml", httpfetcher.ErrInvalidContentType), + fmt.Errorf("%w: image/tiff", httpfetcher.ErrInvalidContentType), http.StatusUnsupportedMediaType, slog.LevelWarn}, {"any other failure", io.ErrUnexpectedEOF, http.StatusInternalServerError, slog.LevelError}, diff --git a/internal/handlers/unsupported_source_internal_test.go b/internal/handlers/unsupported_source_internal_test.go index dbb05b3..a2438c6 100644 --- a/internal/handlers/unsupported_source_internal_test.go +++ b/internal/handlers/unsupported_source_internal_test.go @@ -15,7 +15,7 @@ import ( // TestUnsupportedSourceFormatAnswers415 checks that respondImageError, which // answers the errors of both image routes, answers 415 with a clear error when -// the upstream answers with a content type pixa does not accept, such as SVG, +// the upstream answers with a content type pixa does not accept, such as TIFF, // and when libvips does not read a source as the format its first bytes // show, wrapped as the service wraps them. func TestUnsupportedSourceFormatAnswers415(t *testing.T) { @@ -23,11 +23,11 @@ func TestUnsupportedSourceFormatAnswers415(t *testing.T) { h := &Handlers{log: slog.New(slog.DiscardHandler)} req := &imagerequest.Request{ - Source: imagerequest.Source{Host: "img.example.com", Path: "/a.svg"}, + Source: imagerequest.Source{Host: "img.example.com", Path: "/a.tiff"}, } for _, err := range []error{ - fmt.Errorf("upstream fetch failed: %w: image/svg+xml", + fmt.Errorf("upstream fetch failed: %w: image/tiff", httpfetcher.ErrInvalidContentType), fmt.Errorf("image processing failed: %w: libvips does not read the input as avif", imageprocessor.ErrUnsupportedInputFormat), diff --git a/internal/httpfetcher/httpfetcher.go b/internal/httpfetcher/httpfetcher.go index d532288..ef52a35 100644 --- a/internal/httpfetcher/httpfetcher.go +++ b/internal/httpfetcher/httpfetcher.go @@ -119,6 +119,7 @@ func allowedContentTypes() []string { imagerequest.FormatWebP.MIMEType(), imagerequest.FormatAVIF.MIMEType(), imagerequest.FormatJXL.MIMEType(), + imagerequest.FormatSVG.MIMEType(), } } diff --git a/internal/httpfetcher/httpfetcher_internal_test.go b/internal/httpfetcher/httpfetcher_internal_test.go index e1400a2..c85d5d7 100644 --- a/internal/httpfetcher/httpfetcher_internal_test.go +++ b/internal/httpfetcher/httpfetcher_internal_test.go @@ -279,7 +279,7 @@ func TestDetectContentTypeFromPath(t *testing.T) { {"foo/bar.gif", imagerequest.FormatGIF.MIMEType()}, {"foo/bar.webp", imagerequest.FormatWebP.MIMEType()}, {"foo/bar.avif", imagerequest.FormatAVIF.MIMEType()}, - {"foo/bar.svg", contentTypeSVG}, + {"foo/bar.svg", imagerequest.FormatSVG.MIMEType()}, {"foo/bar.bin", contentTypeOctetStream}, {"foo/bar", contentTypeOctetStream}, } diff --git a/internal/httpfetcher/mock.go b/internal/httpfetcher/mock.go index 1e3268b..d59f9d1 100644 --- a/internal/httpfetcher/mock.go +++ b/internal/httpfetcher/mock.go @@ -66,11 +66,9 @@ func (m *MockFetcher) Fetch( }, nil } -// The content types the mock fetcher gives files in no format pixa reads. -const ( - contentTypeSVG = "image/svg+xml" - contentTypeOctetStream = "application/octet-stream" -) +// contentTypeOctetStream is the content type the mock fetcher gives files in +// no format pixa reads. +const contentTypeOctetStream = "application/octet-stream" // detectContentTypeFromPath returns the MIME type based on file extension. func detectContentTypeFromPath(path string) string { @@ -90,7 +88,7 @@ func detectContentTypeFromPath(path string) string { case strings.HasSuffix(path, ".jxl"): return imagerequest.FormatJXL.MIMEType() case strings.HasSuffix(path, ".svg"): - return contentTypeSVG + return imagerequest.FormatSVG.MIMEType() default: return contentTypeOctetStream } diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index 18fdd12..e8b3dc8 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "math" "sync" "time" @@ -113,10 +114,16 @@ var ErrUnsupportedOutputFormat = errors.New("unsupported output format") // ErrUnsupportedInputFormat is returned, before the input is decoded, when // libvips reads the input's first bytes as another format than its source // format, or as none. An SVG whose first bytes pass the first-bytes check for -// AVIF is one example. A source format other than the six pixa reads is also -// refused with it. +// AVIF is one example. A source format other than the seven pixa reads is +// also refused with it. var ErrUnsupportedInputFormat = errors.New("unsupported input format") +// ErrSVGNotOriginalSize is returned when orig is asked of an SVG source at a +// size other than orig. pixa returns an SVG only as it is; a raster format +// gives it a size. +var ErrSVGNotOriginalSize = errors.New( + "an SVG is returned as SVG only at the orig size") + // 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") @@ -164,7 +171,9 @@ func New(params Params) *ImageProcessor { // 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 -// refused with ErrAVIFOutputTooLarge before anything is decoded. +// refused with ErrAVIFOutputTooLarge before anything is decoded. orig of an +// SVG source is the SVG as it is, since libvips cannot save SVG; any other +// format renders it at the output size. func (p *ImageProcessor) Process( ctx context.Context, input io.Reader, @@ -190,27 +199,17 @@ func (p *ImageProcessor) Process( return nil, err } - // Decode image - img, err := vips.NewImageFromBuffer(data) + img, err := loadImage(data) if err != nil { - return nil, fmt.Errorf("failed to decode image: %w", err) + return nil, err } defer img.Close() - // Turn the image upright now: encode strips the EXIF orientation tag, - // and sizes below must be worked out on the upright image. - err = img.AutoRotate() - if err != nil { - return nil, fmt.Errorf("failed to auto-rotate: %w", err) - } + origWidth, origHeight := img.Width(), img.Height() + outputFormat := resolveOutputFormat(transform.Format, sourceFormat) - // Get original dimensions - origWidth := img.Width() - origHeight := img.Height() - - // Validate input dimensions to prevent DoS via decompression bombs - if origWidth > MaxInputDimension || origHeight > MaxInputDimension { - return nil, ErrInputTooLarge + if outputFormat == imagerequest.FormatSVG && sourceFormat == imagerequest.FormatSVG { + return svgAsItIs(data, transform.Size, origWidth, origHeight) } // Determine target dimensions; too large ones are refused before resizing @@ -220,12 +219,6 @@ func (p *ImageProcessor) Process( return nil, err } - // orig is the source's own format; encode refuses an empty format - outputFormat := transform.Format - if outputFormat == imagerequest.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, @@ -234,6 +227,19 @@ func (p *ImageProcessor) Process( return nil, err } + // An SVG is rendered again at about the output size, to stay sharp + if sourceFormat == imagerequest.FormatSVG { + rendered, err := renderSVG(data, + svgDensity(origWidth, origHeight, outputWidth, outputHeight)) + if err != nil { + return nil, err + } + + defer rendered.Close() + + img = rendered + } + // Resize if needed if targetWidth != origWidth || targetHeight != origHeight { err := p.resize(img, outputWidth, outputHeight, transform.FitMode) @@ -290,6 +296,100 @@ func targetDimensions( return width, height, nil } +// resolveOutputFormat returns the format of the output for the requested +// format: sourceFormat for orig, and format itself for any other. encode +// refuses an empty format. +func resolveOutputFormat( + format, sourceFormat imagerequest.Format, +) imagerequest.Format { + if format == imagerequest.FormatOriginal { + return sourceFormat + } + + return format +} + +// loadImage decodes data and turns the image upright: encode strips the EXIF +// orientation tag, and sizes must be worked out on the upright image. An +// image with a side above MaxInputDimension is ErrInputTooLarge, which guards +// against decompression bombs. +func loadImage(data []byte) (*vips.ImageRef, error) { + img, err := vips.NewImageFromBuffer(data) + if err != nil { + return nil, fmt.Errorf("failed to decode image: %w", err) + } + + err = img.AutoRotate() + if err != nil { + img.Close() + + return nil, fmt.Errorf("failed to auto-rotate: %w", err) + } + + if img.Width() > MaxInputDimension || img.Height() > MaxInputDimension { + img.Close() + + return nil, ErrInputTooLarge + } + + return img, nil +} + +// svgAsItIs returns the SVG data unchanged, which is orig of an SVG source +// at the orig size. width x height is the SVG's own size. Any other size is +// ErrSVGNotOriginalSize. +func svgAsItIs( + data []byte, size imagerequest.Size, width, height int, +) (*Result, error) { + if !size.OriginalSize() { + return nil, fmt.Errorf("%w: asked for %dx%d", ErrSVGNotOriginalSize, + size.Width, size.Height) + } + + return &Result{ + Content: io.NopCloser(bytes.NewReader(data)), + ContentLength: int64(len(data)), + ContentType: imagerequest.FormatSVG.MIMEType(), + Width: width, + Height: height, + InputWidth: width, + InputHeight: height, + }, nil +} + +// svgDefaultDensity is the density, in dots per inch, at which libvips +// renders an SVG unless told otherwise. At it, an SVG of width="64" is 64 +// pixels wide. +const svgDefaultDensity = 72 + +// svgDensity returns the least density at which an SVG of width x height, +// its size at svgDefaultDensity, covers outputWidth x outputHeight, so that +// resize hardly changes it. Neither side is rendered above +// MaxInputDimension. +func svgDensity(width, height, outputWidth, outputHeight int) int { + scale := max(float64(outputWidth)/float64(width), + float64(outputHeight)/float64(height)) + maxScale := float64(MaxInputDimension) / float64(max(width, height)) + + return min(int(math.Ceil(svgDefaultDensity*scale)), + int(svgDefaultDensity*maxScale)) +} + +// renderSVG loads the SVG data rendered at density, in dots per inch. +// librsvg renders it and, given the SVG as bytes with no file name, loads +// no file or URL the SVG refers to. +func renderSVG(data []byte, density int) (*vips.ImageRef, error) { + params := vips.NewImportParams() + params.Density.Set(density) + + img, err := vips.LoadImageFromBuffer(data, params) + if err != nil { + return nil, fmt.Errorf("failed to render SVG: %w", err) + } + + return img, nil +} + // WaitForProcessing waits until no image is being processed, or until ctx // ends, and returns how many images were still being processed then. It // waits by taking each slot in processingSemaphore as it frees up until it @@ -397,6 +497,8 @@ func vipsImageType(format imagerequest.Format) vips.ImageType { return vips.ImageTypeAVIF case imagerequest.FormatJXL: return vips.ImageTypeJXL + case imagerequest.FormatSVG: + return vips.ImageTypeSVG case imagerequest.FormatOriginal, imagerequest.FormatAuto: return vips.ImageTypeUnknown default: @@ -575,7 +677,9 @@ func (p *ImageProcessor) encode( case imagerequest.FormatJXL: return exportJXL(img, quality) - case imagerequest.FormatOriginal, imagerequest.FormatAuto: + // libvips cannot save SVG; Process returns an SVG source as it is + case imagerequest.FormatSVG, imagerequest.FormatOriginal, + imagerequest.FormatAuto: return nil, fmt.Errorf("%w: %s", ErrUnsupportedOutputFormat, format) default: diff --git a/internal/imageprocessor/source_formats_internal_test.go b/internal/imageprocessor/source_formats_internal_test.go index ac0255a..4e65aad 100644 --- a/internal/imageprocessor/source_formats_internal_test.go +++ b/internal/imageprocessor/source_formats_internal_test.go @@ -51,7 +51,7 @@ func TestImageProcessor_LoadsEachSourceFormat(t *testing.T) { } // TestImageProcessor_NotASourceFormatRefused verifies that a source format -// other than the six, such as orig, auto or none, is refused with +// other than the seven, such as orig, auto or none, is refused with // ErrUnsupportedInputFormat. The input is text, which libvips reads as no // format. func TestImageProcessor_NotASourceFormatRefused(t *testing.T) { diff --git a/internal/imageprocessor/svg_internal_test.go b/internal/imageprocessor/svg_internal_test.go new file mode 100644 index 0000000..d8a2056 --- /dev/null +++ b/internal/imageprocessor/svg_internal_test.go @@ -0,0 +1,222 @@ +package imageprocessor + +import ( + "bytes" + "errors" + "io" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/davidbyttow/govips/v2/vips" + "sneak.berlin/go/pixa/internal/imagerequest" +) + +// testSVG is a 16x12 SVG, black on its left half and white on its right. +const testSVG = `` + + `` + + `` + +// newSVGTestProcessor returns a processor for the SVG tests. +func newSVGTestProcessor() *ImageProcessor { + return New(Params{MaxInputBytes: 1 << 20, MaxConcurrentProcessing: 1}) +} + +// processSVG processes svg, as an SVG source, with transform. +func processSVG( + t *testing.T, svg string, transform imagerequest.Transform, +) (*Result, []byte, error) { + t.Helper() + + result, err := newSVGTestProcessor().Process(t.Context(), + bytes.NewReader([]byte(svg)), imagerequest.FormatSVG, &transform) + if err != nil { + return nil, nil, err + } + + output, err := io.ReadAll(result.Content) + if err != nil { + t.Fatalf("reading the output: %v", err) + } + + return result, output, nil +} + +// TestImageProcessor_SVGOrigIsTheSVG checks that orig of an SVG at the orig +// size is the SVG itself, never another format, and that orig at another +// size is ErrSVGNotOriginalSize. +func TestImageProcessor_SVGOrigIsTheSVG(t *testing.T) { + t.Parallel() + + result, output, err := processSVG(t, testSVG, + imagerequest.Transform{Format: imagerequest.FormatOriginal}) + if err != nil { + t.Fatalf("orig of the SVG: %v", err) + } + + t.Logf("orig of the SVG: %s, %dx%d", result.ContentType, result.Width, + result.Height) + + if result.ContentType != "image/svg+xml" || string(output) != testSVG { + t.Errorf("orig of the SVG = %s %q, want image/svg+xml %q", + result.ContentType, output, testSVG) + } + + _, _, err = processSVG(t, testSVG, imagerequest.Transform{ + Size: imagerequest.Size{Width: 32, Height: 24}, + Format: imagerequest.FormatOriginal, + }) + if !errors.Is(err, ErrSVGNotOriginalSize) { + t.Errorf("orig of the SVG at 32x24: error = %v, want %v", err, + ErrSVGNotOriginalSize) + } +} + +// TestImageProcessor_RendersSVGToEachRasterFormat checks that an SVG is +// rendered to each raster format at the requested size. +func TestImageProcessor_RendersSVGToEachRasterFormat(t *testing.T) { + t.Parallel() + + for _, format := range []imagerequest.Format{ + imagerequest.FormatJPEG, imagerequest.FormatPNG, imagerequest.FormatGIF, + imagerequest.FormatWebP, imagerequest.FormatAVIF, imagerequest.FormatJXL, + } { + result, output, err := processSVG(t, testSVG, imagerequest.Transform{ + Size: imagerequest.Size{Width: 40, Height: 30}, + Format: format, + }) + if err != nil { + t.Errorf("SVG as %s: %v", format, err) + + continue + } + + t.Logf("SVG as %s: %s, %dx%d, %d bytes", format, result.ContentType, + result.Width, result.Height, len(output)) + + if result.ContentType != format.MIMEType() || + vips.DetermineImageType(output) != vipsImageType(format) || + result.Width != 40 || result.Height != 30 { + t.Errorf("SVG as %s = %s, %dx%d; want %s, 40x30", format, + result.ContentType, result.Width, result.Height, format.MIMEType()) + } + } +} + +// TestImageProcessor_RendersSVGSharpWhenEnlarged checks that a 16x12 SVG +// asked for at 1600x1200 is rendered at that size, not rendered at 16x12 and +// enlarged: 10 pixels either side of the edge between its black and white +// halves are black and white, where an enlarged image is grey there. +func TestImageProcessor_RendersSVGSharpWhenEnlarged(t *testing.T) { + t.Parallel() + + _, output, err := processSVG(t, testSVG, imagerequest.Transform{ + Size: imagerequest.Size{Width: 1600, Height: 1200}, + Format: imagerequest.FormatPNG, + }) + if err != nil { + t.Fatalf("SVG as PNG at 1600x1200: %v", err) + } + + img, err := vips.NewImageFromBuffer(output) + if err != nil { + t.Fatalf("decoding the PNG: %v", err) + } + + t.Cleanup(img.Close) + + for x, want := range map[int]float64{790: 0, 810: 255} { + point, err := img.GetPoint(x, 600) + if err != nil { + t.Fatalf("GetPoint(%d, 600): %v", x, err) + } + + t.Logf("pixel at %d,600: %v", x, point) + + if point[0] != want { + t.Errorf("pixel at %d,600 = %v, want %v", x, point[0], want) + } + } +} + +// TestImageProcessor_SVGFetchesNothing checks that rendering an SVG that +// refers to an image and an external entity on a local server makes no +// request to that server. +func TestImageProcessor_SVGFetchesNothing(t *testing.T) { + t.Parallel() + + var requests atomic.Int64 + + png := createTestPNG(t, 4, 4) + + server := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + t.Logf("request for %s", r.URL) + w.Header().Set("Content-Type", "image/png") + _, _ = w.Write(png) + })) + t.Cleanup(server.Close) + + svg := `` + + `]>` + + `` + + `` + + `` + + `&outside;` + + _, _, err := processSVG(t, svg, imagerequest.Transform{ + Format: imagerequest.FormatPNG, + }) + t.Logf("SVG as PNG: error = %v", err) + + if requests.Load() != 0 { + t.Errorf("rendering the SVG made %d requests, want none", requests.Load()) + } +} + +// TestImageProcessor_RefusesSVGTooLarge checks that an SVG wider than +// MaxInputDimension is ErrInputTooLarge, as a raster source is. +func TestImageProcessor_RefusesSVGTooLarge(t *testing.T) { + t.Parallel() + + _, _, err := processSVG(t, + ``, + imagerequest.Transform{ + Size: imagerequest.Size{Width: 100, Height: 100}, + Format: imagerequest.FormatPNG, + }) + if !errors.Is(err, ErrInputTooLarge) { + t.Errorf("SVG of 10000x10: error = %v, want %v", err, ErrInputTooLarge) + } +} + +// TestSVGDensity checks the density at which an SVG is rendered to cover +// an output size, and that it is held to MaxInputDimension. +func TestSVGDensity(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + width, height int + outputWidth, outputHeight int + want int + }{ + {"own size", 16, 12, 16, 12, svgDefaultDensity}, + {"100 times larger", 16, 12, 1600, 1200, 7200}, + {"wider than the SVG's proportions", 16, 12, 160, 12, 720}, + {"10 times smaller", 100, 100, 10, 10, 8}, + {"held to MaxInputDimension", 8192, 1, 8192, 8192, svgDefaultDensity}, + {"held to MaxInputDimension when enlarged", 1024, 1, 8192, 8192, 576}, + } + + for _, tt := range tests { + got := svgDensity(tt.width, tt.height, tt.outputWidth, tt.outputHeight) + if got != tt.want { + t.Errorf("%s: svgDensity(%d, %d, %d, %d) = %d, want %d", tt.name, + tt.width, tt.height, tt.outputWidth, tt.outputHeight, got, tt.want) + } + } +} diff --git a/internal/imagerequest/imagerequest.go b/internal/imagerequest/imagerequest.go index 3ae3c6c..af73905 100644 --- a/internal/imagerequest/imagerequest.go +++ b/internal/imagerequest/imagerequest.go @@ -14,7 +14,8 @@ import ( var ErrInvalidFitMode = errors.New("invalid fit mode") // Format is an image format. pixa reads and writes the formats from JPEG to -// GIF below. In a request, orig and auto stand for one of them. +// GIF below. It reads SVG and writes it only as orig of an SVG source. In a +// request, orig and auto stand for another format. type Format string // Supported image formats. @@ -26,6 +27,7 @@ const ( FormatAVIF Format = "avif" FormatJXL Format = "jxl" FormatGIF Format = "gif" + FormatSVG Format = "svg" // FormatAuto stands for JPEG XL, AVIF, WebP or JPEG, chosen for each // request from its Accept header once the URL's signature or token has @@ -50,6 +52,8 @@ func (f Format) MIMEType() string { return "image/jxl" case FormatGIF: return "image/gif" + case FormatSVG: + return "image/svg+xml" case FormatOriginal, FormatAuto: return "" default: diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index ab8a5d2..39843cf 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -449,16 +449,18 @@ func (c *Cache) cleanExpiredBatch(ctx context.Context) (int64, error) { return deleted, nil } -// readFormat reads the first bytes of a source or variant file, without -// moving its offset, and returns the format they show. Sources and variants -// are in the same six formats. +// readFormat reads up to magic.MaxMagicBytes first bytes of a source or +// variant file, without moving its offset, and returns the format they show. +// Sources and variants are in the same seven formats; a variant is SVG only +// as orig of an SVG source. A file shorter than magic.MinMagicBytes is +// io.EOF. func readFormat(content io.ReaderAt) (imagerequest.Format, error) { - firstBytes := make([]byte, magic.MinMagicBytes) + firstBytes := make([]byte, magic.MaxMagicBytes) - _, err := content.ReadAt(firstBytes, 0) - if err != nil { + n, err := content.ReadAt(firstBytes, 0) + if err != nil && (!errors.Is(err, io.EOF) || n < magic.MinMagicBytes) { return "", fmt.Errorf("failed to read the first bytes: %w", err) } - return magic.DetectFormat(firstBytes) + return magic.DetectFormat(firstBytes[:n]) } diff --git a/internal/imgcache/cache_hit_internal_test.go b/internal/imgcache/cache_hit_internal_test.go index 828ce50..7ec33be 100644 --- a/internal/imgcache/cache_hit_internal_test.go +++ b/internal/imgcache/cache_hit_internal_test.go @@ -95,8 +95,6 @@ func TestVariant_RefusesFileOfNoOutputFormat(t *testing.T) { wantErr error }{ {"text", []byte("this is text, not an image"), magic.ErrUnknownFormat}, - {"SVG", []byte(``), - magic.ErrUnknownFormat}, {"JPEG cut to 10 bytes", jpegImage[:10], io.EOF}, {"empty", nil, io.EOF}, } diff --git a/internal/imgcache/source_content_internal_test.go b/internal/imgcache/source_content_internal_test.go index 60a9be4..420e527 100644 --- a/internal/imgcache/source_content_internal_test.go +++ b/internal/imgcache/source_content_internal_test.go @@ -216,8 +216,6 @@ func TestSource_RefusesFileOfNoSourceFormat(t *testing.T) { wantErr error }{ {"text", []byte("this is text, not an image"), magic.ErrUnknownFormat}, - {"SVG", []byte(``), - magic.ErrUnknownFormat}, {"JPEG cut to 10 bytes", jpegImage[:10], io.EOF}, {"empty file", nil, io.EOF}, } diff --git a/internal/imgcache/svg_internal_test.go b/internal/imgcache/svg_internal_test.go new file mode 100644 index 0000000..7581662 --- /dev/null +++ b/internal/imgcache/svg_internal_test.go @@ -0,0 +1,54 @@ +package imgcache + +import ( + "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" +) + +// testSVG is an SVG with a comment before its root element, so that its +// first magic.MinMagicBytes bytes do not show its format. +const testSVG = `` + +// TestSourceAndVariant_ReadSVG checks that a cached SVG source is read as +// SVG, and a cached SVG variant (orig of an SVG source) as image/svg+xml. +func TestSourceAndVariant_ReadSVG(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1<<30) + + storeEvictionTestSource(t, cache, testHostCDN, "/drawing.svg", []byte(testSVG)) + + reader, _, format, err := cache.Source(t.Context(), + imagerequest.Source{Host: testHostCDN, Path: "/drawing.svg"}) + t.Logf("Source() format = %q, error = %v", format, err) + + if err != nil || format != imagerequest.FormatSVG { + t.Errorf("Source() = format %q, error %v; want %q", format, err, + imagerequest.FormatSVG) + } + + if reader != nil { + _ = reader.Close() + } + + key := imagerequest.VariantKey("aabbccdd3001") + + err = cache.variants.Write(string(key), []byte(testSVG)) + if err != nil { + t.Fatalf("storing the variant file: %v", err) + } + + reader, _, contentType, err := cache.Variant(key) + t.Logf("Variant() content type = %q, error = %v", contentType, err) + + if err != nil || contentType != "image/svg+xml" { + t.Errorf("Variant() = content type %q, error %v; want image/svg+xml", + contentType, err) + } + + if reader != nil { + _ = reader.Close() + } +} diff --git a/internal/magic/magic.go b/internal/magic/magic.go index 4d069ba..1e4de2c 100644 --- a/internal/magic/magic.go +++ b/internal/magic/magic.go @@ -20,6 +20,10 @@ var ( // MinMagicBytes is the minimum number of bytes needed to detect format. const MinMagicBytes = 12 +// MaxMagicBytes is the most bytes DetectFormat looks at. An SVG's root +// element must start within them, as libvips requires. +const MaxMagicBytes = 1024 + // Magic byte signatures for supported formats. // These are effectively constants but Go doesn't support const slices. // @@ -39,6 +43,12 @@ var ( } ) +// utf8BOM is the UTF-8 byte order mark, which a text file such as an SVG may +// start with. +// +//nolint:gochecknoglobals // immutable lookup data +var utf8BOM = []byte{0xEF, 0xBB, 0xBF} + // WebP identifier appears at offset 8 after RIFF header. // //nolint:gochecknoglobals // immutable lookup data @@ -53,7 +63,7 @@ var ( ) // DetectFormat detects the image format from magic bytes. Data in none of -// the formats pixa reads, SVG included, is ErrUnknownFormat. +// the formats pixa reads is ErrUnknownFormat. func DetectFormat(data []byte) (imagerequest.Format, error) { if len(data) < MinMagicBytes { return "", ErrNotEnoughData @@ -96,9 +106,25 @@ func DetectFormat(data []byte) (imagerequest.Format, error) { return imagerequest.FormatJXL, nil } + if isSVG(data) { + return imagerequest.FormatSVG, nil + } + return "", ErrUnknownFormat } +// isSVG reports whether data starts as an SVG does: with a "<", after an +// optional UTF-8 byte order mark and white space, and with "`), - wantErr: ErrUnknownFormat, + name: "SVG", + data: []byte(``), + wantFormat: imagerequest.FormatSVG, }, { name: "unknown format", diff --git a/internal/magic/svg_internal_test.go b/internal/magic/svg_internal_test.go new file mode 100644 index 0000000..fdbf0e3 --- /dev/null +++ b/internal/magic/svg_internal_test.go @@ -0,0 +1,85 @@ +package magic + +import ( + "errors" + "strings" + "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" +) + +// TestDetectFormat_SVG checks that DetectFormat names SVG text with what may +// come before the root element, and does not name as SVG text that does not +// start with "<" or has no "` + + tests := []struct { + name string + data string + wantErr error + }{ + {name: "root element only", data: root}, + {name: "XML declaration", data: `` + "\n" + root}, + {name: "byte order mark and white space", data: "\xEF\xBB\xBF \n\t" + root}, + {name: "comment", data: "" + root}, + { + name: "DOCTYPE", + data: `` + root, + }, + { + name: "root element after the first bytes", + data: "" + root, + wantErr: ErrUnknownFormat, + }, + {name: "text naming ", wantErr: ErrUnknownFormat}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := DetectFormat([]byte(tt.data)) + t.Logf("DetectFormat() = %q, %v", got, err) + + if tt.wantErr != nil { + if !errors.Is(err, tt.wantErr) { + t.Errorf("DetectFormat() error = %v, want %v", err, tt.wantErr) + } + + return + } + + if err != nil || got != imagerequest.FormatSVG { + t.Errorf("DetectFormat() = %q, %v; want %q", got, err, + imagerequest.FormatSVG) + } + }) + } +} + +// TestValidateMagicBytes_SVG checks that SVG data is SVG for the +// Content-Type image/svg+xml, with a charset too, and a mismatch for another. +func TestValidateMagicBytes_SVG(t *testing.T) { + t.Parallel() + + data := []byte(``) + + for _, declared := range []string{"image/svg+xml", "image/svg+xml; charset=utf-8"} { + got, err := ValidateMagicBytes(data, declared) + if err != nil || got != imagerequest.FormatSVG { + t.Errorf("ValidateMagicBytes(%q) = %q, %v; want %q", declared, got, + err, imagerequest.FormatSVG) + } + } + + _, err := ValidateMagicBytes(data, "image/png") + if !errors.Is(err, ErrMagicByteMismatch) { + t.Errorf("ValidateMagicBytes(image/png) error = %v, want %v", err, + ErrMagicByteMismatch) + } +} diff --git a/internal/server/svg_internal_test.go b/internal/server/svg_internal_test.go index 95b46fa..4f28592 100644 --- a/internal/server/svg_internal_test.go +++ b/internal/server/svg_internal_test.go @@ -4,38 +4,114 @@ import ( "io" "net/http" "net/http/httptest" + "strings" + "sync/atomic" "testing" + + "sneak.berlin/go/pixa/internal/imageproxy" + "sneak.berlin/go/pixa/internal/imagerequest" + "sneak.berlin/go/pixa/internal/magic" ) -// 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() +// testSVG is the SVG the upstream serves in the SVG tests below. +const testSVG = `` + + `` + +// startSVGUpstream starts an upstream that answers every request with +// testSVG as image/svg+xml, and returns it with its count of requests. +func startSVGUpstream(t *testing.T) (*httptest.Server, *atomic.Int64) { + t.Helper() + + var requests atomic.Int64 upstream := httptest.NewServer(http.HandlerFunc( func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) w.Header().Set("Content-Type", "image/svg+xml") - _, _ = io.WriteString(w, ``) + _, _ = io.WriteString(w, testSVG) })) t.Cleanup(upstream.Close) + return upstream, &requests +} + +// TestImageProxyServesSVGAsItIs requests an SVG source in its own format +// (orig) at the orig size through pixa's router, upstream fetcher, image +// processor and cache, twice. Both answers are the SVG itself, never another +// format, the second from the cache; the upstream is asked once. Scripts in +// the SVG cannot run, as the Content-Security-Policy allows no inline +// script. orig at another size is refused with 400. +func TestImageProxyServesSVGAsItIs(t *testing.T) { + t.Parallel() + + upstream, requests := startSVGUpstream(t) + s, _, _ := startImageProxy(t, upstream) + target := "/v1/image/" + upstreamHost + "/drawing.svg/orig.orig" + + for _, wantCache := range []imageproxy.CacheStatus{ + imageproxy.CacheMiss, imageproxy.CacheHit, + } { + rec := getImage(t, s, target) + + if rec.Header().Get("Content-Type") != "image/svg+xml" || + rec.Body.String() != testSVG { + t.Errorf("GET %s: %s %q, want image/svg+xml %q", target, + rec.Header().Get("Content-Type"), rec.Body.String(), testSVG) + } + + if got := rec.Header().Get("X-Pixa-Cache"); got != string(wantCache) { + t.Errorf("GET %s: X-Pixa-Cache = %q, want %q", target, got, wantCache) + } + + policy := rec.Header().Get("Content-Security-Policy") + if !strings.Contains(policy, "script-src 'self'") || + strings.Contains(policy, "unsafe-inline") { + t.Errorf("GET %s: Content-Security-Policy = %q, want script-src "+ + "'self' and no unsafe-inline", target, policy) + } + } + + if requests.Load() != 1 { + t.Errorf("upstream requests = %d, want 1", requests.Load()) + } + + resized := "/v1/image/" + upstreamHost + "/drawing.svg/32x24.orig" + + rec := httptest.NewRecorder() + s.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, resized, nil)) + t.Logf("GET %s: %d %s", resized, rec.Code, rec.Body.String()) + + if rec.Code != http.StatusBadRequest { + t.Errorf("GET %s: %d %s, want 400", resized, rec.Code, rec.Body.String()) + } +} + +// TestImageProxyRendersSVG requests an SVG source as each raster format +// through pixa's router, upstream fetcher and image processor. Each answer +// is an image in that format. +func TestImageProxyRendersSVG(t *testing.T) { + t.Parallel() + + upstream, _ := startSVGUpstream(t) s, _, _ := startImageProxy(t, upstream) - for _, sizeFormat := range []string{"64x48.orig", "32x24.png"} { - target := "/v1/image/" + upstreamHost + "/drawing.svg/" + sizeFormat + for _, format := range []imagerequest.Format{ + imagerequest.FormatJPEG, imagerequest.FormatPNG, imagerequest.FormatGIF, + imagerequest.FormatWebP, imagerequest.FormatAVIF, imagerequest.FormatJXL, + } { + target := "/v1/image/" + upstreamHost + "/drawing.svg/32x24." + + string(format) - 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()) + rec := getImage(t, s, target) + detected, err := magic.DetectFormat(rec.Body.Bytes()) + t.Logf("GET %s: %s, body in %q (%v)", target, + rec.Header().Get("Content-Type"), detected, err) - if rec.Code != http.StatusUnsupportedMediaType { - t.Errorf("GET %s status = %d, want %d", - target, rec.Code, http.StatusUnsupportedMediaType) + if rec.Header().Get("Content-Type") != format.MIMEType() || + err != nil || detected != format { + t.Errorf("GET %s: %s, body in %q (%v); want %s", target, + rec.Header().Get("Content-Type"), detected, err, format.MIMEType()) } } }