From 4515d09c497465e7e7c617d086c2322a0d97a3c3 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 26 Oct 2026 19:46:01 +0000 Subject: [PATCH] Detect a fetched image's format from its bytes once (closes #319) magic.DetectFormat returns an imagerequest.Format. ValidateMagicBytes returns the format it checked against the upstream Content-Type. The cache reads a cached source's format from its file's first bytes, as it reads a variant's. The proxy hands the format to Process. Process refuses a format that is not a source format, and input libvips would load with another loader, such as an SVG passing as AVIF. Format.MIMEType declares each MIME string once, and the fetcher, the cache's variant read and the Accept matching use it. magic.MIMEType, magic.ImageFormat, MIMEToImageFormat, magic's SVG detection, detectFormat, formatFromString, FormatToMIME and Result.InputFormat are gone. Model: opus-5-5 --- README.md | 9 +- TODO.md | 24 +++ internal/handlers/format_auto.go | 8 +- .../unsupported_source_internal_test.go | 6 +- internal/httpfetcher/fetch_internal_test.go | 6 +- internal/httpfetcher/httpfetcher.go | 27 +-- .../httpfetcher/httpfetcher_internal_test.go | 22 +- internal/httpfetcher/mock.go | 18 +- .../httpfetcher/request_id_internal_test.go | 3 +- .../avif_too_large_internal_test.go | 33 +-- .../computed_side_internal_test.go | 4 +- .../imageprocessor/deadline_internal_test.go | 5 +- .../empty_format_internal_test.go | 2 +- .../encoder_settings_internal_test.go | 17 +- .../imageprocessor/fit_cover_internal_test.go | 6 +- .../fit_outside_internal_test.go | 4 +- internal/imageprocessor/imageprocessor.go | 147 +++++-------- .../imageprocessor_internal_test.go | 60 +++--- .../jpegxl_output_internal_test.go | 5 +- ...max_concurrent_processing_internal_test.go | 10 +- .../orig_format_internal_test.go | 7 +- .../source_formats_internal_test.go | 33 ++- internal/imageproxy/imageproxy.go | 50 +++-- .../imageproxy/imageproxy_internal_test.go | 17 +- ...max_concurrent_processing_internal_test.go | 14 +- .../negative_cache_internal_test.go | 2 +- .../partial_cache_files_internal_test.go | 37 +++- internal/imagerequest/imagerequest.go | 34 ++- .../imagerequest/mimetype_internal_test.go | 30 +++ internal/imgcache/cache.go | 56 +++-- internal/imgcache/cache_hit_internal_test.go | 6 +- internal/imgcache/eviction_internal_test.go | 2 +- .../pendingaccesstimes_internal_test.go | 9 +- .../imgcache/source_content_internal_test.go | 105 +++++++++- internal/magic/jpegxl_internal_test.go | 27 +-- internal/magic/magic.go | 119 ++--------- internal/magic/magic_internal_test.go | 195 +++--------------- 37 files changed, 570 insertions(+), 589 deletions(-) create mode 100644 internal/imagerequest/mimetype_internal_test.go diff --git a/README.md b/README.md index 0f3271a..5e1437c 100644 --- a/README.md +++ b/README.md @@ -191,10 +191,11 @@ allowlisted source hosts for open access. name. Each source image and each transformed image has one row in the database, -holding its size and when it was last used. Two source URLs that serve the same -bytes are stored twice, once under each URL. A source image is stored by writing -its file and then its row; when the row cannot be written, the file is removed -again. +holding its size and when it was last used. pixa reads a cached source image's +format from its file's first bytes. When they match none of the source formats, +pixa fetches the source again. Two source URLs that serve the same bytes are +stored twice, once under each URL. A source image is stored by writing its file +and then its row; when the row cannot be written, the file is removed again. Source and transformed image files are written under a temporary name, synced to disk, and only then renamed into place, so a crash leaves no file under its own diff --git a/TODO.md b/TODO.md index e76626d..39e0f30 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,30 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-26 a fetched image's format is detected from its bytes once, as an + `imagerequest.Format` (closes #319). The upstream `Content-Type` check in the + fetcher stays as the filter before downloading. + - `magic.DetectFormat` returns an `imagerequest.Format`, and + `magic.ValidateMagicBytes` returns the format it checked against the + `Content-Type`. + - `Format.MIMEType` gives each format's MIME type. The fetcher, the mock + fetcher, the cache's read of a variant's type and the handler's `Accept` + matching use it. + - `Cache.Source` returns a source's format, read from its file's first bytes + as `Cache.Variant` reads a variant's. A file in no source format is an + error, and the proxy fetches the source again. + - The proxy hands the format to `ImageProcessor.Process`. The processor + refuses input that libvips would load as another format, as before, with + `ErrUnsupportedInputFormat`. It refuses a format that is not one of the + six source formats the same way. + - `internal/magic` no longer detects SVG, which pixa does not read. SVG data + is `ErrUnknownFormat`. + - Removed: + - `magic.MIMEType`, `magic.ImageFormat` and `magic.MIMEToImageFormat` + - `detectFormat`, `formatFromString`, `FormatToMIME` and + `Result.InputFormat` of the processor + - the fetcher's MIME constants of the six formats + - `errNotAnOutputFormat` of the cache - 2026-10-26 the cache keeps its source images in one `imgcache.FileStore` and its variants in another (closes #318). A `FileStore` is a directory of files, each named by a hex key, at `//`. diff --git a/internal/handlers/format_auto.go b/internal/handlers/format_auto.go index 96a1b06..c54bebd 100644 --- a/internal/handlers/format_auto.go +++ b/internal/handlers/format_auto.go @@ -65,20 +65,20 @@ func formatForAccept(accept string) (imagerequest.Format, error) { return imagerequest.FormatJPEG, nil } - if qualities["image/jxl"] > 0 { + if qualities[imagerequest.FormatJXL.MIMEType()] > 0 { return imagerequest.FormatJXL, nil } - if qualities["image/avif"] > 0 { + if qualities[imagerequest.FormatAVIF.MIMEType()] > 0 { return imagerequest.FormatAVIF, nil } - if qualities["image/webp"] > 0 { + if qualities[imagerequest.FormatWebP.MIMEType()] > 0 { return imagerequest.FormatWebP, nil } // For JPEG, the most specific entry the header has decides - quality, named := qualities["image/jpeg"] + quality, named := qualities[imagerequest.FormatJPEG.MIMEType()] if !named { quality, named = qualities["image/*"] } diff --git a/internal/handlers/unsupported_source_internal_test.go b/internal/handlers/unsupported_source_internal_test.go index 030274f..dbb05b3 100644 --- a/internal/handlers/unsupported_source_internal_test.go +++ b/internal/handlers/unsupported_source_internal_test.go @@ -16,8 +16,8 @@ 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, -// and when orig is asked of a source in a format the processor cannot name, -// wrapped as the service wraps them. +// 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) { t.Parallel() @@ -29,7 +29,7 @@ func TestUnsupportedSourceFormatAnswers415(t *testing.T) { for _, err := range []error{ fmt.Errorf("upstream fetch failed: %w: image/svg+xml", httpfetcher.ErrInvalidContentType), - fmt.Errorf("image processing failed: %w: unknown", + fmt.Errorf("image processing failed: %w: libvips does not read the input as avif", imageprocessor.ErrUnsupportedInputFormat), } { rec := httptest.NewRecorder() diff --git a/internal/httpfetcher/fetch_internal_test.go b/internal/httpfetcher/fetch_internal_test.go index 0d515d7..7c5199f 100644 --- a/internal/httpfetcher/fetch_internal_test.go +++ b/internal/httpfetcher/fetch_internal_test.go @@ -83,7 +83,7 @@ func startUpstream(t *testing.T) *httptest.Server { mux := http.NewServeMux() mux.HandleFunc("/image", func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", contentTypeJPEG) + w.Header().Set("Content-Type", imagerequest.FormatJPEG.MIMEType()) _, _ = io.WriteString(w, imagePayload) }) mux.HandleFunc("/status/500", func(w http.ResponseWriter, _ *http.Request) { @@ -358,7 +358,7 @@ func TestFetchContentLengthBodyAtMaxResponseSize(t *testing.T) { res := fetchWithMaxResponseSize(t, maxResponseSize, func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", contentTypeJPEG) + w.Header().Set("Content-Type", imagerequest.FormatJPEG.MIMEType()) w.Header().Set("Content-Length", strconv.Itoa(tc.size)) _, _ = io.WriteString(w, body) }) @@ -406,7 +406,7 @@ func TestFetchChunkedBodyAtMaxResponseSize(t *testing.T) { res := fetchWithMaxResponseSize(t, maxResponseSize, func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", contentTypeJPEG) + w.Header().Set("Content-Type", imagerequest.FormatJPEG.MIMEType()) _, _ = io.WriteString(w, body) // With no Content-Length, flushing sends the body // chunked; its end is sent when the handler returns. diff --git a/internal/httpfetcher/httpfetcher.go b/internal/httpfetcher/httpfetcher.go index 08eeee1..d532288 100644 --- a/internal/httpfetcher/httpfetcher.go +++ b/internal/httpfetcher/httpfetcher.go @@ -38,18 +38,6 @@ const userAgent = "pixa/1.0" // MaxConnections fetches are already in progress. const ConnectionWaitTimeout = 10 * time.Second -// MIME content types. -const ( - contentTypeJPEG = "image/jpeg" - contentTypePNG = "image/png" - contentTypeGIF = "image/gif" - contentTypeWebP = "image/webp" - contentTypeAVIF = "image/avif" - contentTypeJXL = "image/jxl" - contentTypeSVG = "image/svg+xml" - contentTypeOctetStream = "application/octet-stream" -) - // Loopback addresses blocked by SSRF protection. const ( localhostIPv4 = "127.0.0.1" @@ -121,15 +109,16 @@ type FetchResult struct { } // allowedContentTypes returns the content types an upstream response may -// have, which Fetch also sends as its Accept header. +// have, which Fetch also sends as its Accept header: the MIME types of the +// formats pixa reads. func allowedContentTypes() []string { return []string{ - contentTypeJPEG, - contentTypePNG, - contentTypeGIF, - contentTypeWebP, - contentTypeAVIF, - contentTypeJXL, + imagerequest.FormatJPEG.MIMEType(), + imagerequest.FormatPNG.MIMEType(), + imagerequest.FormatGIF.MIMEType(), + imagerequest.FormatWebP.MIMEType(), + imagerequest.FormatAVIF.MIMEType(), + imagerequest.FormatJXL.MIMEType(), } } diff --git a/internal/httpfetcher/httpfetcher_internal_test.go b/internal/httpfetcher/httpfetcher_internal_test.go index 8b71419..e1400a2 100644 --- a/internal/httpfetcher/httpfetcher_internal_test.go +++ b/internal/httpfetcher/httpfetcher_internal_test.go @@ -30,9 +30,9 @@ func TestIsAllowedContentType(t *testing.T) { contentType string want bool }{ - {contentTypeJPEG, true}, - {contentTypePNG, true}, - {contentTypeWebP, true}, + {imagerequest.FormatJPEG.MIMEType(), true}, + {imagerequest.FormatPNG.MIMEType(), true}, + {imagerequest.FormatWebP.MIMEType(), true}, {"image/jpeg; charset=utf-8", true}, {"IMAGE/JPEG", true}, {"text/html", false}, @@ -217,7 +217,7 @@ func TestMockFetcher_FetchesFile(t *testing.T) { defer func() { _ = result.Content.Close() }() - if result.ContentType != contentTypeJPEG { + if result.ContentType != imagerequest.FormatJPEG.MIMEType() { t.Errorf("ContentType = %q, want image/jpeg", result.ContentType) } @@ -272,13 +272,13 @@ func TestDetectContentTypeFromPath(t *testing.T) { path string want string }{ - {"foo/bar.jpg", contentTypeJPEG}, - {"foo/bar.JPG", contentTypeJPEG}, - {"foo/bar.jpeg", contentTypeJPEG}, - {"foo/bar.png", contentTypePNG}, - {"foo/bar.gif", contentTypeGIF}, - {"foo/bar.webp", contentTypeWebP}, - {"foo/bar.avif", contentTypeAVIF}, + {"foo/bar.jpg", imagerequest.FormatJPEG.MIMEType()}, + {"foo/bar.JPG", imagerequest.FormatJPEG.MIMEType()}, + {"foo/bar.jpeg", imagerequest.FormatJPEG.MIMEType()}, + {"foo/bar.png", imagerequest.FormatPNG.MIMEType()}, + {"foo/bar.gif", imagerequest.FormatGIF.MIMEType()}, + {"foo/bar.webp", imagerequest.FormatWebP.MIMEType()}, + {"foo/bar.avif", imagerequest.FormatAVIF.MIMEType()}, {"foo/bar.svg", contentTypeSVG}, {"foo/bar.bin", contentTypeOctetStream}, {"foo/bar", contentTypeOctetStream}, diff --git a/internal/httpfetcher/mock.go b/internal/httpfetcher/mock.go index 40ac3e6..1e3268b 100644 --- a/internal/httpfetcher/mock.go +++ b/internal/httpfetcher/mock.go @@ -66,23 +66,29 @@ 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" +) + // detectContentTypeFromPath returns the MIME type based on file extension. func detectContentTypeFromPath(path string) string { path = strings.ToLower(path) switch { case strings.HasSuffix(path, ".jpg"), strings.HasSuffix(path, ".jpeg"): - return contentTypeJPEG + return imagerequest.FormatJPEG.MIMEType() case strings.HasSuffix(path, ".png"): - return contentTypePNG + return imagerequest.FormatPNG.MIMEType() case strings.HasSuffix(path, ".gif"): - return contentTypeGIF + return imagerequest.FormatGIF.MIMEType() case strings.HasSuffix(path, ".webp"): - return contentTypeWebP + return imagerequest.FormatWebP.MIMEType() case strings.HasSuffix(path, ".avif"): - return contentTypeAVIF + return imagerequest.FormatAVIF.MIMEType() case strings.HasSuffix(path, ".jxl"): - return contentTypeJXL + return imagerequest.FormatJXL.MIMEType() case strings.HasSuffix(path, ".svg"): return contentTypeSVG default: diff --git a/internal/httpfetcher/request_id_internal_test.go b/internal/httpfetcher/request_id_internal_test.go index 62a654d..b6e87b8 100644 --- a/internal/httpfetcher/request_id_internal_test.go +++ b/internal/httpfetcher/request_id_internal_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/go-chi/chi/v5/middleware" + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestFetchSendsRequestID verifies that a fetch sends the ID of the request @@ -25,7 +26,7 @@ func TestFetchSendsRequestID(t *testing.T) { func(w http.ResponseWriter, r *http.Request) { received <- r.Header.Get("X-Request-Id") - w.Header().Set("Content-Type", contentTypeJPEG) + w.Header().Set("Content-Type", imagerequest.FormatJPEG.MIMEType()) _, _ = io.WriteString(w, imagePayload) })) t.Cleanup(srv.Close) diff --git a/internal/imageprocessor/avif_too_large_internal_test.go b/internal/imageprocessor/avif_too_large_internal_test.go index 15b184c..7850e66 100644 --- a/internal/imageprocessor/avif_too_large_internal_test.go +++ b/internal/imageprocessor/avif_too_large_internal_test.go @@ -39,30 +39,31 @@ func TestImageProcessor_RefusesAVIFAboveMaxPixels(t *testing.T) { tests := []struct { name string input []byte + sourceFormat imagerequest.Format format imagerequest.Format width, height int fit imagerequest.Fit refused bool }{ - {"AVIF at the limit", cutOff, imagerequest.FormatAVIF, - 4096, 4096, imagerequest.FitCover, false}, - {"AVIF above the limit", cutOff, imagerequest.FormatAVIF, - 4096, 4097, imagerequest.FitCover, true}, - {"WebP above the AVIF limit", cutOff, imagerequest.FormatWebP, - 4096, 4097, imagerequest.FitCover, false}, + {"AVIF at the limit", cutOff, imagerequest.FormatPNG, + imagerequest.FormatAVIF, 4096, 4096, imagerequest.FitCover, false}, + {"AVIF above the limit", cutOff, imagerequest.FormatPNG, + imagerequest.FormatAVIF, 4096, 4097, imagerequest.FitCover, true}, + {"WebP above the AVIF limit", cutOff, imagerequest.FormatPNG, + imagerequest.FormatWebP, 4096, 4097, imagerequest.FitCover, false}, // FitInside does not enlarge, so the output stays 64x32 - {"AVIF inside a larger size", cutOff, imagerequest.FormatAVIF, - 8192, 8192, imagerequest.FitInside, false}, + {"AVIF inside a larger size", cutOff, imagerequest.FormatPNG, + imagerequest.FormatAVIF, 8192, 8192, imagerequest.FitInside, false}, // FitOutside scales 64x32 by 128 to cover 8192x1: 8192x4096 - {"AVIF outside a smaller size", cutOff, imagerequest.FormatAVIF, - 8192, 1, imagerequest.FitOutside, true}, + {"AVIF outside a smaller size", cutOff, imagerequest.FormatPNG, + imagerequest.FormatAVIF, 8192, 1, imagerequest.FitOutside, true}, // With no size, the output keeps the source's 4097x4096 {"AVIF of the source's size", largeAVIF, imagerequest.FormatAVIF, - 0, 0, imagerequest.FitCover, true}, - {"orig of an AVIF source", largeAVIF, imagerequest.FormatOriginal, - 0, 0, imagerequest.FitCover, true}, - {"JPEG of the source's size", largeAVIF, imagerequest.FormatJPEG, - 0, 0, imagerequest.FitCover, false}, + imagerequest.FormatAVIF, 0, 0, imagerequest.FitCover, true}, + {"orig of an AVIF source", largeAVIF, imagerequest.FormatAVIF, + imagerequest.FormatOriginal, 0, 0, imagerequest.FitCover, true}, + {"JPEG of the source's size", largeAVIF, imagerequest.FormatAVIF, + imagerequest.FormatJPEG, 0, 0, imagerequest.FitCover, false}, } for _, tt := range tests { @@ -72,7 +73,7 @@ func TestImageProcessor_RefusesAVIFAboveMaxPixels(t *testing.T) { _, err := New(Params{ MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, - }).Process(t.Context(), bytes.NewReader(tt.input), + }).Process(t.Context(), bytes.NewReader(tt.input), tt.sourceFormat, &imagerequest.Transform{ Size: imagerequest.Size{Width: tt.width, Height: tt.height}, Format: tt.format, diff --git a/internal/imageprocessor/computed_side_internal_test.go b/internal/imageprocessor/computed_side_internal_test.go index 6bfb23a..496b4cb 100644 --- a/internal/imageprocessor/computed_side_internal_test.go +++ b/internal/imageprocessor/computed_side_internal_test.go @@ -42,7 +42,7 @@ func TestImageProcessor_RefusesComputedSideAboveMaximum(t *testing.T) { MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, }).Process(t.Context(), - bytes.NewReader(input), + bytes.NewReader(input), imagerequest.FormatJPEG, &imagerequest.Transform{ Size: tt.size, Format: imagerequest.FormatJPEG, @@ -111,7 +111,7 @@ func TestImageProcessor_ComputedSideIsAtLeastOne(t *testing.T) { MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, }).Process(t.Context(), - bytes.NewReader(input), + bytes.NewReader(input), imagerequest.FormatJPEG, &imagerequest.Transform{ Size: tt.size, Format: imagerequest.FormatJPEG, diff --git a/internal/imageprocessor/deadline_internal_test.go b/internal/imageprocessor/deadline_internal_test.go index 9f6d394..b63e79e 100644 --- a/internal/imageprocessor/deadline_internal_test.go +++ b/internal/imageprocessor/deadline_internal_test.go @@ -7,6 +7,8 @@ import ( "io" "testing" "time" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // readerAfterDeadline is a Process input whose first Read waits until done @@ -86,7 +88,8 @@ func checkProcessStopsAtDeadline( proc := New(Params{MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1}) - result, err := proc.Process(ctx, input, smallJPEGTransform()) + result, err := proc.Process(ctx, input, imagerequest.FormatJPEG, + smallJPEGTransform()) if result != nil { t.Logf("Process() made a %dx%d image after its deadline passed", result.Width, result.Height) diff --git a/internal/imageprocessor/empty_format_internal_test.go b/internal/imageprocessor/empty_format_internal_test.go index 48bba8e..de54111 100644 --- a/internal/imageprocessor/empty_format_internal_test.go +++ b/internal/imageprocessor/empty_format_internal_test.go @@ -19,7 +19,7 @@ func TestImageProcessor_EmptyFormatRefused(t *testing.T) { MaxConcurrentProcessing: 1, }).Process( t.Context(), bytes.NewReader(createTestJPEG(t, 20, 20)), - &imagerequest.Transform{}, + imagerequest.FormatJPEG, &imagerequest.Transform{}, ) if !errors.Is(err, ErrUnsupportedOutputFormat) { t.Errorf("Process() error = %v, want %v", err, ErrUnsupportedOutputFormat) diff --git a/internal/imageprocessor/encoder_settings_internal_test.go b/internal/imageprocessor/encoder_settings_internal_test.go index dc93d76..b414e20 100644 --- a/internal/imageprocessor/encoder_settings_internal_test.go +++ b/internal/imageprocessor/encoder_settings_internal_test.go @@ -11,17 +11,18 @@ import ( "sneak.berlin/go/pixa/internal/imagerequest" ) -// processedSize runs input through Process and returns the output's size in -// bytes. +// processedSize runs input, an image in sourceFormat, through Process and +// returns the output's size in bytes. func processedSize( - t *testing.T, input []byte, transform *imagerequest.Transform, + t *testing.T, input []byte, sourceFormat imagerequest.Format, + transform *imagerequest.Transform, ) int64 { t.Helper() result, err := New(Params{ MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, - }).Process(t.Context(), bytes.NewReader(input), transform) + }).Process(t.Context(), bytes.NewReader(input), sourceFormat, transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -74,7 +75,7 @@ func TestImageProcessor_PNGIsCompressed(t *testing.T) { } } - size := processedSize(t, encodePNG(t, flat), + size := processedSize(t, encodePNG(t, flat), imagerequest.FormatPNG, &imagerequest.Transform{Format: imagerequest.FormatPNG}) const rawBytes = width * height * 3 @@ -92,7 +93,7 @@ func TestImageProcessor_WebPAtEffort2(t *testing.T) { input := createTestJPEG(t, 200, 150) - size := processedSize(t, input, + size := processedSize(t, input, imagerequest.FormatJPEG, &imagerequest.Transform{Format: imagerequest.FormatWebP, Quality: 85}) want, _, err := decode(t, input).ExportWebp(&vips.WebpExportParams{ @@ -134,7 +135,7 @@ func TestImageProcessor_AVIFAtEffort1(t *testing.T) { input := encodePNG(t, source) - size := processedSize(t, input, + size := processedSize(t, input, imagerequest.FormatPNG, &imagerequest.Transform{Format: imagerequest.FormatAVIF, Quality: 85}) want, _, err := decode(t, input).ExportAvif(&vips.AvifExportParams{ @@ -161,7 +162,7 @@ func TestImageProcessor_JPEGXLAtEffort4(t *testing.T) { input := createTestJPEG(t, 200, 150) - size := processedSize(t, input, + size := processedSize(t, input, imagerequest.FormatJPEG, &imagerequest.Transform{Format: imagerequest.FormatJXL, Quality: 85}) params := vips.NewJxlExportParams() diff --git a/internal/imageprocessor/fit_cover_internal_test.go b/internal/imageprocessor/fit_cover_internal_test.go index a12d155..ea347ac 100644 --- a/internal/imageprocessor/fit_cover_internal_test.go +++ b/internal/imageprocessor/fit_cover_internal_test.go @@ -40,7 +40,7 @@ func TestImageProcessor_FitCoverServesThinSource(t *testing.T) { MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, }).Process(t.Context(), - bytes.NewReader(input), + bytes.NewReader(input), imagerequest.FormatJPEG, &imagerequest.Transform{ Size: tt.size, Format: imagerequest.FormatJPEG, @@ -113,7 +113,7 @@ func TestImageProcessor_FitCoverCropsTheCentre(t *testing.T) { MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, }).Process(t.Context(), - bytes.NewReader(input), + bytes.NewReader(input), imagerequest.FormatPNG, &imagerequest.Transform{ Size: imagerequest.Size{Width: 50, Height: 50}, Format: imagerequest.FormatPNG, @@ -240,7 +240,7 @@ func TestImageProcessor_FitCoverKeepsProportions(t *testing.T) { MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, }).Process(t.Context(), - bytes.NewReader(input), + bytes.NewReader(input), imagerequest.FormatPNG, &imagerequest.Transform{ Size: tt.size, Format: imagerequest.FormatPNG, diff --git a/internal/imageprocessor/fit_outside_internal_test.go b/internal/imageprocessor/fit_outside_internal_test.go index 3c72d59..12e7e23 100644 --- a/internal/imageprocessor/fit_outside_internal_test.go +++ b/internal/imageprocessor/fit_outside_internal_test.go @@ -45,7 +45,7 @@ func TestImageProcessor_RefusesFitOutsideAboveMaximum(t *testing.T) { MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, }).Process(t.Context(), - bytes.NewReader(input), + bytes.NewReader(input), imagerequest.FormatJPEG, &imagerequest.Transform{ Size: tt.size, Format: imagerequest.FormatJPEG, @@ -102,7 +102,7 @@ func TestImageProcessor_FitOutsideSideIsAtLeastOne(t *testing.T) { MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, }).Process(t.Context(), - bytes.NewReader(input), + bytes.NewReader(input), imagerequest.FormatJPEG, &imagerequest.Transform{ Size: tt.size, Format: imagerequest.FormatJPEG, diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index afe216e..18fdd12 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -70,8 +70,6 @@ type Result struct { InputWidth int // InputHeight is the original image height before processing. InputHeight int - // InputFormat is the detected input format (e.g., "jpeg", "png"). - InputFormat string } // MaxInputDimension is the maximum allowed width or height for input images. @@ -113,9 +111,10 @@ var ErrAVIFOutputTooLarge = errors.New("AVIF output has too many pixels") 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 the source -// formats, or as none: for example an SVG whose first bytes pass the -// first-bytes check for AVIF. +// 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. var ErrUnsupportedInputFormat = errors.New("unsupported input format") // ErrTooManyImages is returned when MaxConcurrentProcessing images are being @@ -157,9 +156,10 @@ func New(params Params) *ImageProcessor { } } -// Process transforms an image according to transform. When -// MaxConcurrentProcessing images are already being processed, it waits up -// to ProcessingWaitTimeout for one to finish, then fails with +// Process transforms the image read from input according to transform. +// sourceFormat is the input's format, which magic detected from its first +// bytes. When MaxConcurrentProcessing images are already being processed, +// it waits up to ProcessingWaitTimeout for one to finish, then fails with // ErrTooManyImages. When ctx has ended once Process has a slot, or before it // encodes, it returns ctx's error and gives the slot back. govips gives pixa // no way to stop libvips once an encode has begun, so an encode keeps its @@ -168,6 +168,7 @@ func New(params Params) *ImageProcessor { func (p *ImageProcessor) Process( ctx context.Context, input io.Reader, + sourceFormat imagerequest.Format, transform *imagerequest.Transform, ) (*Result, error) { release, err := p.acquireSlot(ctx) @@ -182,13 +183,9 @@ func (p *ImageProcessor) Process( return nil, err } - // 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) + // Input libvips would not load as sourceFormat is refused before it is + // decoded + err = checkInputFormat(data, sourceFormat) if err != nil { return nil, err } @@ -254,12 +251,11 @@ func (p *ImageProcessor) Process( return &Result{ Content: io.NopCloser(bytes.NewReader(output)), ContentLength: int64(len(output)), - ContentType: FormatToMIME(outputFormat), + ContentType: outputFormat.MIMEType(), Width: img.Width(), Height: img.Height(), InputWidth: origWidth, InputHeight: origHeight, - InputFormat: inputFormat, }, nil } @@ -294,38 +290,6 @@ func targetDimensions( return width, height, nil } -// MIME types for the supported image formats. -const ( - mimeJPEG = "image/jpeg" - mimePNG = "image/png" - mimeGIF = "image/gif" - mimeWebP = "image/webp" - mimeAVIF = "image/avif" - mimeJXL = "image/jxl" -) - -// FormatToMIME converts a Format to its MIME type string. -func FormatToMIME(format imagerequest.Format) string { - switch format { - case imagerequest.FormatJPEG: - return mimeJPEG - case imagerequest.FormatPNG: - return mimePNG - case imagerequest.FormatWebP: - return mimeWebP - case imagerequest.FormatGIF: - return mimeGIF - case imagerequest.FormatAVIF: - return mimeAVIF - case imagerequest.FormatJXL: - return mimeJXL - case imagerequest.FormatOriginal, imagerequest.FormatAuto: - return "application/octet-stream" - default: - return "application/octet-stream" - } -} - // 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 @@ -394,27 +358,49 @@ func (p *ImageProcessor) readInput(input io.Reader) ([]byte, error) { return data, nil } -// 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: - return "png" - case vips.ImageTypeGIF: - return "gif" - case vips.ImageTypeWEBP: - return "webp" - case vips.ImageTypeAVIF, vips.ImageTypeHEIF: - return string(imagerequest.FormatAVIF) - case vips.ImageTypeJXL: - return string(imagerequest.FormatJXL) - case vips.ImageTypeUnknown, vips.ImageTypeMagick, vips.ImageTypePDF, - vips.ImageTypeSVG, vips.ImageTypeTIFF, vips.ImageTypeBMP, - vips.ImageTypeJP2K: - return "unknown" +// checkInputFormat returns ErrUnsupportedInputFormat unless libvips would +// load data with the loader of sourceFormat. govips picks the libvips loader +// from the input's first bytes with DetermineImageType. It refuses an SVG +// whose first bytes pass the first-bytes check for AVIF, which librsvg would +// otherwise render. +func checkInputFormat(data []byte, sourceFormat imagerequest.Format) error { + // A format with no loader, such as orig or auto, has the type govips also + // gives input it cannot read, which the check below would let through. + sourceType := vipsImageType(sourceFormat) + if sourceType == vips.ImageTypeUnknown { + return fmt.Errorf("%w: %q is not a source format", + ErrUnsupportedInputFormat, sourceFormat) + } + + if vips.DetermineImageType(data) != sourceType { + return fmt.Errorf("%w: libvips does not read the input as %s", + ErrUnsupportedInputFormat, sourceFormat) + } + + return nil +} + +// vipsImageType returns the govips image type whose libvips loader reads +// format. orig and auto stand for another format, so they have +// vips.ImageTypeUnknown, which govips has no loader for. +func vipsImageType(format imagerequest.Format) vips.ImageType { + switch format { + case imagerequest.FormatJPEG: + return vips.ImageTypeJPEG + case imagerequest.FormatPNG: + return vips.ImageTypePNG + case imagerequest.FormatGIF: + return vips.ImageTypeGIF + case imagerequest.FormatWebP: + return vips.ImageTypeWEBP + case imagerequest.FormatAVIF: + return vips.ImageTypeAVIF + case imagerequest.FormatJXL: + return vips.ImageTypeJXL + case imagerequest.FormatOriginal, imagerequest.FormatAuto: + return vips.ImageTypeUnknown default: - return "unknown" + return vips.ImageTypeUnknown } } @@ -768,26 +754,3 @@ func jxlDistance(quality int) float64 { return 53.0/3000.0*q*q - 23.0/20.0*q + 25.0 } - -// formatFromString converts a format string from detectFormat to Format. A -// format detectFormat could not name is ErrUnsupportedInputFormat. -func (p *ImageProcessor) formatFromString( - format string, -) (imagerequest.Format, error) { - switch format { - case "jpeg": - return imagerequest.FormatJPEG, nil - case "png": - return imagerequest.FormatPNG, nil - case "gif": - return imagerequest.FormatGIF, nil - case "webp": - return imagerequest.FormatWebP, nil - case string(imagerequest.FormatAVIF): - return imagerequest.FormatAVIF, nil - case string(imagerequest.FormatJXL): - return imagerequest.FormatJXL, nil - default: - return "", fmt.Errorf("%w: %s", ErrUnsupportedInputFormat, format) - } -} diff --git a/internal/imageprocessor/imageprocessor_internal_test.go b/internal/imageprocessor/imageprocessor_internal_test.go index e9b3656..dca5645 100644 --- a/internal/imageprocessor/imageprocessor_internal_test.go +++ b/internal/imageprocessor/imageprocessor_internal_test.go @@ -94,23 +94,23 @@ func isAVIF(data []byte) bool { // detectMIME is a minimal magic-byte detector for test assertions. func detectMIME(data []byte) string { if len(data) >= 3 && data[0] == 0xFF && data[1] == 0xD8 && data[2] == 0xFF { - return mimeJPEG + return imagerequest.FormatJPEG.MIMEType() } if len(data) >= 8 && string(data[:8]) == "\x89PNG\r\n\x1a\n" { - return mimePNG + return imagerequest.FormatPNG.MIMEType() } if len(data) >= 4 && string(data[:4]) == "GIF8" { - return mimeGIF + return imagerequest.FormatGIF.MIMEType() } if len(data) >= 12 && string(data[:4]) == "RIFF" && string(data[8:12]) == "WEBP" { - return mimeWebP + return imagerequest.FormatWebP.MIMEType() } if isAVIF(data) { - return mimeAVIF + return imagerequest.FormatAVIF.MIMEType() } return "" @@ -131,7 +131,8 @@ func TestImageProcessor_ResizeJPEG(t *testing.T) { FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), transform) + result, err := proc.Process(ctx, bytes.NewReader(input), + imagerequest.FormatJPEG, transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -157,7 +158,7 @@ func TestImageProcessor_ResizeJPEG(t *testing.T) { } mime := detectMIME(data) - if mime != mimeJPEG { + if mime != imagerequest.FormatJPEG.MIMEType() { t.Errorf("Output format = %v, want image/jpeg", mime) } } @@ -176,7 +177,8 @@ func TestImageProcessor_ConvertToPNG(t *testing.T) { FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), transform) + result, err := proc.Process(ctx, bytes.NewReader(input), + imagerequest.FormatJPEG, transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -189,7 +191,7 @@ func TestImageProcessor_ConvertToPNG(t *testing.T) { } mime := detectMIME(data) - if mime != mimePNG { + if mime != imagerequest.FormatPNG.MIMEType() { t.Errorf("Output format = %v, want image/png", mime) } } @@ -213,7 +215,8 @@ func processAndCheckSize( FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), transform) + result, err := proc.Process(ctx, bytes.NewReader(input), + imagerequest.FormatJPEG, transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -253,7 +256,8 @@ func TestImageProcessor_FitContain(t *testing.T) { FitMode: imagerequest.FitContain, } - result, err := proc.Process(ctx, bytes.NewReader(input), transform) + result, err := proc.Process(ctx, bytes.NewReader(input), + imagerequest.FormatJPEG, transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -296,7 +300,8 @@ func TestImageProcessor_ProcessPNG(t *testing.T) { FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), transform) + result, err := proc.Process(ctx, bytes.NewReader(input), + imagerequest.FormatPNG, transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -341,7 +346,8 @@ func TestImageProcessor_RejectsOversizedInput(t *testing.T) { FitMode: imagerequest.FitCover, } - _, err := proc.Process(ctx, bytes.NewReader(input), transform) + _, err := proc.Process(ctx, bytes.NewReader(input), + imagerequest.FormatJPEG, transform) if err == nil { t.Error("Process() should reject oversized input images") } @@ -369,7 +375,8 @@ func TestImageProcessor_AcceptsMaxDimensionInput(t *testing.T) { FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), transform) + result, err := proc.Process(ctx, bytes.NewReader(input), + imagerequest.FormatJPEG, transform) if err != nil { t.Fatalf( "Process() should accept images at MaxInputDimension, got error: %v", @@ -399,7 +406,8 @@ func encodeAndCheck( FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), transform) + result, err := proc.Process(ctx, bytes.NewReader(input), + imagerequest.FormatJPEG, transform) if err != nil { t.Fatalf("Process() error = %v, want nil", err) } @@ -430,7 +438,7 @@ func encodeAndCheck( func TestImageProcessor_EncodeWebP(t *testing.T) { t.Parallel() - encodeAndCheck(t, imagerequest.FormatWebP, 80, mimeWebP) + encodeAndCheck(t, imagerequest.FormatWebP, 80, imagerequest.FormatWebP.MIMEType()) } func TestImageProcessor_DecodeAVIF(t *testing.T) { @@ -453,7 +461,8 @@ func TestImageProcessor_DecodeAVIF(t *testing.T) { FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), transform) + result, err := proc.Process(ctx, bytes.NewReader(input), + imagerequest.FormatAVIF, transform) if err != nil { t.Fatalf("Process() error = %v, want nil (AVIF decoding should work)", err) } @@ -467,7 +476,7 @@ func TestImageProcessor_DecodeAVIF(t *testing.T) { } mime := detectMIME(data) - if mime != mimeJPEG { + if mime != imagerequest.FormatJPEG.MIMEType() { t.Errorf("Output format = %v, want image/jpeg", mime) } } @@ -494,7 +503,8 @@ func TestImageProcessor_RejectsOversizedInputData(t *testing.T) { FitMode: imagerequest.FitCover, } - _, err := proc.Process(ctx, bytes.NewReader(input), transform) + _, err := proc.Process(ctx, bytes.NewReader(input), + imagerequest.FormatJPEG, transform) if err == nil { t.Fatal("Process() should reject input exceeding maxInputBytes") } @@ -521,7 +531,8 @@ func TestImageProcessor_AcceptsInputWithinLimit(t *testing.T) { FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), transform) + result, err := proc.Process(ctx, bytes.NewReader(input), + imagerequest.FormatJPEG, transform) if err != nil { t.Fatalf("Process() error = %v, want nil", err) } @@ -532,11 +543,11 @@ func TestImageProcessor_AcceptsInputWithinLimit(t *testing.T) { func TestImageProcessor_EncodeAVIF(t *testing.T) { t.Parallel() - encodeAndCheck(t, imagerequest.FormatAVIF, 85, mimeAVIF) + encodeAndCheck(t, imagerequest.FormatAVIF, 85, imagerequest.FormatAVIF.MIMEType()) } -// processAndDecode runs input through Process and decodes the output with -// vips, so a test can inspect the image a client would receive. +// processAndDecode runs input, a JPEG, through Process and decodes the output +// with vips, so a test can inspect the image a client would receive. func processAndDecode( t *testing.T, input []byte, transform *imagerequest.Transform, ) *vips.ImageRef { @@ -546,7 +557,8 @@ func processAndDecode( MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, }).Process( - context.Background(), bytes.NewReader(input), transform, + context.Background(), bytes.NewReader(input), imagerequest.FormatJPEG, + transform, ) if err != nil { t.Fatalf("Process() error = %v", err) diff --git a/internal/imageprocessor/jpegxl_output_internal_test.go b/internal/imageprocessor/jpegxl_output_internal_test.go index 29afb32..1f041ee 100644 --- a/internal/imageprocessor/jpegxl_output_internal_test.go +++ b/internal/imageprocessor/jpegxl_output_internal_test.go @@ -25,7 +25,8 @@ func TestImageProcessor_EncodeJPEGXL(t *testing.T) { MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, }).Process( - t.Context(), bytes.NewReader(createTestJPEG(t, 200, 150)), transform, + t.Context(), bytes.NewReader(createTestJPEG(t, 200, 150)), + imagerequest.FormatJPEG, transform, ) if err != nil { t.Fatalf("Process() error = %v", err) @@ -71,7 +72,7 @@ func TestImageProcessor_JPEGXLQuality(t *testing.T) { MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, }).Process(t.Context(), - bytes.NewReader(input), + bytes.NewReader(input), imagerequest.FormatJPEG, &imagerequest.Transform{Format: imagerequest.FormatJXL, Quality: quality}) if err != nil { t.Fatalf("Process() at quality %d error = %v", quality, err) diff --git a/internal/imageprocessor/max_concurrent_processing_internal_test.go b/internal/imageprocessor/max_concurrent_processing_internal_test.go index fff69d0..e9ce734 100644 --- a/internal/imageprocessor/max_concurrent_processing_internal_test.go +++ b/internal/imageprocessor/max_concurrent_processing_internal_test.go @@ -93,7 +93,8 @@ func processInBackground( proc *ImageProcessor, reader *gatedReader, results chan<- error, ) { go func() { - result, err := proc.Process(context.Background(), reader, smallJPEGTransform()) + result, err := proc.Process(context.Background(), reader, + imagerequest.FormatJPEG, smallJPEGTransform()) if err == nil { _ = result.Content.Close() } @@ -206,7 +207,7 @@ func TestProcessWaitsThenFailsWhenNoSlotFrees(t *testing.T) { start := time.Now() _, err := proc.Process(context.Background(), bytes.NewReader(input), - smallJPEGTransform()) + imagerequest.FormatJPEG, smallJPEGTransform()) if !errors.Is(err, ErrTooManyImages) { t.Fatalf("Process() error = %v, want ErrTooManyImages", err) } @@ -274,7 +275,8 @@ func TestProcessReleasesSlotOnError(t *testing.T) { proc := New(Params{MaxInputBytes: 4096, MaxConcurrentProcessing: 1}) proc.processingWaitTimeout = 100 * time.Millisecond - _, err := proc.Process(context.Background(), tc.input, tc.transform) + _, err := proc.Process(context.Background(), tc.input, + imagerequest.FormatJPEG, tc.transform) if err == nil || (tc.want != nil && !errors.Is(err, tc.want)) { t.Fatalf("Process() error = %v, want %v", err, tc.want) } @@ -284,7 +286,7 @@ func TestProcessReleasesSlotOnError(t *testing.T) { } result, err := proc.Process(context.Background(), bytes.NewReader(valid), - smallJPEGTransform()) + imagerequest.FormatJPEG, smallJPEGTransform()) if err != nil { t.Fatalf("Process() after the error = %v, want nil", err) } diff --git a/internal/imageprocessor/orig_format_internal_test.go b/internal/imageprocessor/orig_format_internal_test.go index 70b97ef..142eabb 100644 --- a/internal/imageprocessor/orig_format_internal_test.go +++ b/internal/imageprocessor/orig_format_internal_test.go @@ -10,8 +10,9 @@ import ( ) // 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. +// input is refused with ErrUnsupportedInputFormat before it is decoded, and +// not saved as JPEG. TIFF is not a source format, and the test passes the +// input to Process as a JPEG. func TestImageProcessor_OrigOfUnnamedFormatRefused(t *testing.T) { t.Parallel() @@ -31,7 +32,7 @@ func TestImageProcessor_OrigOfUnnamedFormatRefused(t *testing.T) { MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1, }).Process( - t.Context(), bytes.NewReader(tiff), + t.Context(), bytes.NewReader(tiff), imagerequest.FormatJPEG, &imagerequest.Transform{Format: imagerequest.FormatOriginal}, ) if result != nil { diff --git a/internal/imageprocessor/source_formats_internal_test.go b/internal/imageprocessor/source_formats_internal_test.go index aaf3aec..ac0255a 100644 --- a/internal/imageprocessor/source_formats_internal_test.go +++ b/internal/imageprocessor/source_formats_internal_test.go @@ -2,6 +2,7 @@ package imageprocessor import ( "bytes" + "errors" "io" "testing" @@ -21,7 +22,7 @@ func TestImageProcessor_LoadsEachSourceFormat(t *testing.T) { imagerequest.FormatWebP, imagerequest.FormatAVIF, imagerequest.FormatJXL, } { saved, err := proc.Process(t.Context(), - bytes.NewReader(createTestPNG(t, 20, 20)), + bytes.NewReader(createTestPNG(t, 20, 20)), imagerequest.FormatPNG, &imagerequest.Transform{Format: format}) if err != nil { t.Fatalf("saving the test image as %s: %v", format, err) @@ -32,7 +33,7 @@ func TestImageProcessor_LoadsEachSourceFormat(t *testing.T) { t.Fatalf("reading the %s image: %v", format, err) } - result, err := proc.Process(t.Context(), bytes.NewReader(source), + result, err := proc.Process(t.Context(), bytes.NewReader(source), format, &imagerequest.Transform{Format: imagerequest.FormatOriginal}) if err != nil { t.Errorf("orig of the %s image: %v", format, err) @@ -42,9 +43,33 @@ func TestImageProcessor_LoadsEachSourceFormat(t *testing.T) { t.Logf("orig of the %s image: %s", format, result.ContentType) - if result.ContentType != FormatToMIME(format) { + if result.ContentType != format.MIMEType() { t.Errorf("orig of the %s image is %s, want %s", - format, result.ContentType, FormatToMIME(format)) + format, result.ContentType, format.MIMEType()) + } + } +} + +// TestImageProcessor_NotASourceFormatRefused verifies that a source format +// other than the six, 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) { + t.Parallel() + + proc := New(Params{MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1}) + + for _, sourceFormat := range []imagerequest.Format{ + imagerequest.FormatOriginal, imagerequest.FormatAuto, "", + } { + result, err := proc.Process(t.Context(), + bytes.NewReader([]byte("this is text, not an image")), sourceFormat, + &imagerequest.Transform{Format: imagerequest.FormatJPEG}) + t.Logf("source format %q: error = %v", sourceFormat, err) + + if result != nil || !errors.Is(err, ErrUnsupportedInputFormat) { + t.Errorf("Process() with source format %q = %v, error %v; want %v", + sourceFormat, result, err, ErrUnsupportedInputFormat) } } } diff --git a/internal/imageproxy/imageproxy.go b/internal/imageproxy/imageproxy.go index 7df0381..ae17947 100644 --- a/internal/imageproxy/imageproxy.go +++ b/internal/imageproxy/imageproxy.go @@ -303,21 +303,22 @@ func withoutCancelKeepingDeadline( return context.WithDeadline(context.WithoutCancel(ctx), deadline) } -// loadCachedSource opens src's cached source, without reading it, and -// returns it with its size; nil if it is not in the cache, cannot be read, -// is empty or exceeds maxResponseSize. +// loadCachedSource opens src's cached source, reading only the first bytes, +// which give its format, and returns it with its size and its format. It +// returns nil if the source is not in the cache, cannot be read, is in no +// source format or exceeds maxResponseSize. func (p *Proxy) loadCachedSource( ctx context.Context, src imagerequest.Source, -) (io.ReadCloser, int64) { - reader, size, err := p.cache.Source(ctx, src) +) (io.ReadCloser, int64, imagerequest.Format) { + reader, size, format, err := p.cache.Source(ctx, src) if errors.Is(err, imgcache.ErrNotFound) { - return nil, 0 + return nil, 0, "" } if err != nil { p.log.Warn("failed to load cached source, fetching", "error", err) - return nil, 0 + return nil, 0, "" } p.log.Debug("using cached source", "source_key", src.Key()) @@ -330,16 +331,10 @@ func (p *Proxy) loadCachedSource( "max_bytes", p.maxResponseSize, ) - return nil, 0 + return nil, 0, "" } - if size == 0 { - _ = reader.Close() - - return nil, 0 - } - - return reader, size + return reader, size, format } // processFromSourceOrFetch processes an image, using cached source content @@ -349,7 +344,7 @@ func (p *Proxy) processFromSourceOrFetch( req *imagerequest.Request, cacheKey imagerequest.VariantKey, ) (*processedVariant, error) { - source, sourceSize := p.loadCachedSource(ctx, req.Source) + source, sourceSize, sourceFormat := p.loadCachedSource(ctx, req.Source) // Fetch from upstream if we don't have source data or it's empty if source == nil { @@ -361,7 +356,7 @@ func (p *Proxy) processFromSourceOrFetch( // Process using cached source; nothing was fetched from upstream. The // image processor reads the source only once it has a processing slot, // so a request waiting for one holds none of it in memory. - return p.processAndStore(ctx, req, cacheKey, source, sourceSize) + return p.processAndStore(ctx, req, cacheKey, source, sourceFormat, sourceSize) } // fetchAndProcess fetches from upstream, processes, and caches the result. @@ -442,10 +437,11 @@ func (p *Proxy) fetchAndProcess( "cipher", fetchResult.TLSCipherSuite, ) - // Validate magic bytes match content type. A body that does not is the - // upstream's error, so it gets a negative cache entry, as an upstream - // error status does. - err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType) + // The source's format, detected from its first bytes, which must match + // its content type. A body that does not is the upstream's error, so it + // gets a negative cache entry, as an upstream error status does. + sourceFormat, err := magic.ValidateMagicBytes(sourceData, + fetchResult.ContentType) if err != nil { p.log.Warn("source refused", "request_id", middleware.GetReqID(ctx), "host", req.Source.Host, "path", req.Source.Path, @@ -463,7 +459,7 @@ func (p *Proxy) fetchAndProcess( } return p.processAndStore( - ctx, req, cacheKey, bytes.NewReader(sourceData), fetchBytes, + ctx, req, cacheKey, bytes.NewReader(sourceData), sourceFormat, fetchBytes, ) } @@ -497,13 +493,14 @@ type processedVariant struct { fetchedBytes int64 } -// processAndStore processes the image read from source and stores the -// result. +// processAndStore processes the image read from source, which is in +// sourceFormat, and stores the result. func (p *Proxy) processAndStore( ctx context.Context, req *imagerequest.Request, cacheKey imagerequest.VariantKey, source io.Reader, + sourceFormat imagerequest.Format, fetchBytes int64, ) (*processedVariant, error) { // Process the image @@ -511,7 +508,8 @@ func (p *Proxy) processAndStore( // ctx has the deadline of the request doing the processing, which can // end while a request waiting for its image has time left - processResult, err := p.processor.Process(ctx, source, &req.Transform) + processResult, err := p.processor.Process(ctx, source, sourceFormat, + &req.Transform) if errors.Is(err, context.DeadlineExceeded) { return nil, fmt.Errorf("%w: %w", ErrProcessingTimedOut, err) } @@ -546,7 +544,7 @@ func (p *Proxy) processAndStore( "request_id", middleware.GetReqID(ctx), "host", req.Source.Host, "path", req.Source.Path, - "src_format", processResult.InputFormat, + "src_format", sourceFormat, "dst_format", req.Transform.Format, "src_bytes", fetchBytes, "dst_bytes", outputSize, diff --git a/internal/imageproxy/imageproxy_internal_test.go b/internal/imageproxy/imageproxy_internal_test.go index 5e392ea..be2e46a 100644 --- a/internal/imageproxy/imageproxy_internal_test.go +++ b/internal/imageproxy/imageproxy_internal_test.go @@ -191,23 +191,14 @@ func TestProxy_Get_FormatConversion(t *testing.T) { t.Fatalf("failed to read response: %v", err) } - detectedMIME, err := magic.DetectFormat(data) + detectedFormat, err := magic.DetectFormat(data) if err != nil { t.Fatalf("failed to detect format: %v", err) } - expectedFormat, ok := magic.MIMEToImageFormat(tt.wantMIME) - if !ok { - t.Fatalf("unknown format for MIME type: %s", tt.wantMIME) - } - - detectedFormat, ok := magic.MIMEToImageFormat(string(detectedMIME)) - if !ok { - t.Fatalf("unknown format for detected MIME type: %s", detectedMIME) - } - - if detectedFormat != expectedFormat { - t.Errorf("detected format = %q, want %q", detectedFormat, expectedFormat) + if detectedFormat.MIMEType() != tt.wantMIME { + t.Errorf("detected format = %q, want the format of %s", + detectedFormat, tt.wantMIME) } }) } diff --git a/internal/imageproxy/max_concurrent_processing_internal_test.go b/internal/imageproxy/max_concurrent_processing_internal_test.go index 5d49d16..affa3cb 100644 --- a/internal/imageproxy/max_concurrent_processing_internal_test.go +++ b/internal/imageproxy/max_concurrent_processing_internal_test.go @@ -41,7 +41,8 @@ func holdProcessingSlot( input, feed := io.Pipe() go func() { - _, _ = proc.Process(t.Context(), input, &imagerequest.Transform{}) + _, _ = proc.Process(t.Context(), input, imagerequest.FormatJPEG, + &imagerequest.Transform{}) }() _, err := feed.Write([]byte{0}) @@ -57,10 +58,11 @@ func holdProcessingSlot( // TestProxy_Get_WaitsForSlotBeforeReadingCachedSource checks that a // request whose source is cached holds none of it while it waits for a -// processing slot: it reads the cached file only once it has a slot. With -// the only slot held, a request for a new width of the cached 100x100 photo -// waits; the cached file is then rewritten as a 100x50 image before the slot -// is freed, so the request must answer with that image scaled to 40x20. +// processing slot: past the first bytes, which give the source's format, it +// reads the cached file only once it has a slot. With the only slot held, a +// request for a new width of the cached 100x100 photo waits; the cached file +// is then rewritten as a 100x50 image before the slot is freed, so the +// request must answer with that image scaled to 40x20. func TestProxy_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) { t.Parallel() @@ -79,7 +81,7 @@ func TestProxy_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) { cachedSource := widthOnlyRequest(fixtures, 50).Source - reader, _, err := svc.cache.Source(t.Context(), cachedSource) + reader, _, _, err := svc.cache.Source(t.Context(), cachedSource) if err != nil { t.Fatalf("Source() error = %v; want the cached source", err) } diff --git a/internal/imageproxy/negative_cache_internal_test.go b/internal/imageproxy/negative_cache_internal_test.go index 23ca916..96257cc 100644 --- a/internal/imageproxy/negative_cache_internal_test.go +++ b/internal/imageproxy/negative_cache_internal_test.go @@ -55,7 +55,7 @@ func TestProxy_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) { source := photoVariant(fixtures, 85, imagerequest.FitCover).Source - reader, _, err := svc.cache.Source(t.Context(), source) + reader, _, _, err := svc.cache.Source(t.Context(), source) if err != nil { t.Fatalf("Source() error = %v, want the cached source", err) } diff --git a/internal/imageproxy/partial_cache_files_internal_test.go b/internal/imageproxy/partial_cache_files_internal_test.go index f1b7c4f..13f37bc 100644 --- a/internal/imageproxy/partial_cache_files_internal_test.go +++ b/internal/imageproxy/partial_cache_files_internal_test.go @@ -7,6 +7,7 @@ import ( "testing" "sneak.berlin/go/pixa/internal/imagerequest" + "sneak.berlin/go/pixa/internal/imgcache" ) // getImage calls Get for req and returns the image it answered with and @@ -49,7 +50,7 @@ func cachedSourcePath( ) string { t.Helper() - reader, _, err := svc.cache.Source(t.Context(), req.Source) + reader, _, _, err := svc.cache.Source(t.Context(), req.Source) if err != nil { t.Fatalf("Source() error = %v; want the cached source", err) } @@ -104,6 +105,40 @@ func TestProxy_Get_FetchesAgainForRemovedCachedSource(t *testing.T) { } } +// TestProxy_Get_FetchesAgainSourceOfNoSourceFormat replaces the cached +// source's file with text. The next request for a new size of the source +// must fetch it again and cache it whole. +func TestProxy_Get_FetchesAgainSourceOfNoSourceFormat(t *testing.T) { + t.Parallel() + + svc, fixtures := setupTestProxy(t) + + getImage(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover)) + + path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover)) + wholeSize := fileSize(t, path) + + err := os.WriteFile(path, []byte("this is text, not an image"), + imgcache.StorageFilePerm) + if err != nil { + t.Fatalf("replacing the source's file: %v", err) + } + + getImage(t, svc, photoVariant(fixtures, 40, imagerequest.FitCover)) + + fetches := readCounters(t, svc).upstreamFetchCount + t.Logf("upstream fetches = %d", fetches) + + if fetches != 2 { + t.Errorf("upstream fetches = %d, want 2", fetches) + } + + size := fileSize(t, path) + if size != wholeSize { + t.Errorf("cached source is %d bytes, want %d", size, wholeSize) + } +} + // TestProxy_Get_ConvertsAgainForRemovedCachedVariant caches a variant of // the photo and removes its row and file, as the reconciliation pass does // when the file is empty or cut short. internal/imgcache tests that the diff --git a/internal/imagerequest/imagerequest.go b/internal/imagerequest/imagerequest.go index d9b2bac..3ae3c6c 100644 --- a/internal/imagerequest/imagerequest.go +++ b/internal/imagerequest/imagerequest.go @@ -1,7 +1,8 @@ // Package imagerequest holds the image request type, its parts, its checks, // the parsing of the /v1/image/ path, the variant key, the source key and the -// source's upstream URL. It imports no other package of this repo, so every -// package that handles an image request can import it. +// source's upstream URL. It also holds the image format type and the MIME type +// of each format. It imports no other package of this repo, so every package +// that handles an image request can import it. package imagerequest import ( @@ -12,10 +13,11 @@ import ( // ErrInvalidFitMode is returned when an invalid fit mode is provided. var ErrInvalidFitMode = errors.New("invalid fit mode") -// Format represents supported output image formats. +// 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. type Format string -// Supported image output formats. +// Supported image formats. const ( FormatOriginal Format = "orig" FormatJPEG Format = "jpeg" @@ -31,6 +33,30 @@ const ( FormatAuto Format = "auto" ) +// MIMEType returns the MIME type of the format, such as image/jpeg for +// FormatJPEG. orig and auto stand for another format, so they have none: +// MIMEType returns "" for them. +func (f Format) MIMEType() string { + switch f { + case FormatJPEG: + return "image/jpeg" + case FormatPNG: + return "image/png" + case FormatWebP: + return "image/webp" + case FormatAVIF: + return "image/avif" + case FormatJXL: + return "image/jxl" + case FormatGIF: + return "image/gif" + case FormatOriginal, FormatAuto: + return "" + default: + return "" + } +} + // Size represents requested image dimensions type Size struct { Width int diff --git a/internal/imagerequest/mimetype_internal_test.go b/internal/imagerequest/mimetype_internal_test.go new file mode 100644 index 0000000..172c5e9 --- /dev/null +++ b/internal/imagerequest/mimetype_internal_test.go @@ -0,0 +1,30 @@ +package imagerequest + +import "testing" + +// TestFormat_MIMEType checks the MIME type of each format pixa reads and +// writes, and that orig and auto have none. +func TestFormat_MIMEType(t *testing.T) { + t.Parallel() + + tests := []struct { + format Format + want string + }{ + {FormatJPEG, "image/jpeg"}, + {FormatPNG, "image/png"}, + {FormatWebP, "image/webp"}, + {FormatAVIF, "image/avif"}, + {FormatJXL, "image/jxl"}, + {FormatGIF, "image/gif"}, + {FormatOriginal, ""}, + {FormatAuto, ""}, + } + + for _, tt := range tests { + got := tt.format.MIMEType() + if got != tt.want { + t.Errorf("%s MIMEType() = %q, want %q", tt.format, got, tt.want) + } + } +} diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index d1376b2..ab8a5d2 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -175,7 +175,7 @@ func (c *Cache) Variant( return nil, 0, "", err } - contentType, err := outputContentType(file) + format, err := readFormat(file) if err != nil { _ = file.Close() @@ -184,7 +184,7 @@ func (c *Cache) Variant( c.touchVariant(cacheKey) - return file, size, contentType, nil + return file, size, format.MIMEType(), nil } // PutVariant stores a processed variant by its cache key and records @@ -227,15 +227,17 @@ func (c *Cache) PutVariant( return nil } -// Source opens the cached source of src and returns a reader and its size -// in bytes. A source whose row or file is missing is ErrNotFound, as is any -// source of a disabled cache. A source it opens has its access time handed -// to the background writer without waiting (see touchSourceContent). +// Source opens the cached source of src and returns a reader, its size in +// bytes and its format, read from the file's first bytes. A source whose row +// or file is missing is ErrNotFound, as is any source of a disabled cache; a +// file whose first bytes are not those of a source format is another error. +// A source it opens has its access time handed to the background writer +// without waiting (see touchSourceContent). func (c *Cache) Source( ctx context.Context, src imagerequest.Source, -) (io.ReadCloser, int64, error) { +) (io.ReadCloser, int64, imagerequest.Format, error) { if c.config.MaxBytes == 0 { - return nil, 0, ErrNotFound + return nil, 0, "", ErrNotFound } key := src.Key() @@ -246,21 +248,28 @@ func (c *Cache) Source( SELECT EXISTS(SELECT 1 FROM source_content WHERE source_key = ?) `, string(key)).Scan(&rowExists) if err != nil { - return nil, 0, fmt.Errorf("failed to lookup source: %w", err) + return nil, 0, "", fmt.Errorf("failed to lookup source: %w", err) } if !rowExists { - return nil, 0, ErrNotFound + return nil, 0, "", ErrNotFound } file, size, err := c.srcContent.Open(string(key)) if err != nil { - return nil, 0, err + return nil, 0, "", err + } + + format, err := readFormat(file) + if err != nil { + _ = file.Close() + + return nil, 0, "", err } c.touchSourceContent(key) - return file, size, nil + return file, size, format, nil } // PutSource stores fetched source content: it writes the file named by @@ -440,13 +449,10 @@ func (c *Cache) cleanExpiredBatch(ctx context.Context) (int64, error) { return deleted, nil } -// errNotAnOutputFormat is returned for a variant file whose first bytes are -// those of a format pixa reads but never writes, such as SVG. -var errNotAnOutputFormat = errors.New("variant file is not in an output format") - -// outputContentType reads the first bytes of content, without moving its -// offset, and returns the content type of the output format they show. -func outputContentType(content io.ReaderAt) (string, error) { +// 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. +func readFormat(content io.ReaderAt) (imagerequest.Format, error) { firstBytes := make([]byte, magic.MinMagicBytes) _, err := content.ReadAt(firstBytes, 0) @@ -454,15 +460,5 @@ func outputContentType(content io.ReaderAt) (string, error) { return "", fmt.Errorf("failed to read the first bytes: %w", err) } - detected, err := magic.DetectFormat(firstBytes) - if err != nil { - return "", err - } - - _, isOutputFormat := magic.MIMEToImageFormat(string(detected)) - if !isOutputFormat { - return "", fmt.Errorf("%w: %s", errNotAnOutputFormat, detected) - } - - return string(detected), nil + return magic.DetectFormat(firstBytes) } diff --git a/internal/imgcache/cache_hit_internal_test.go b/internal/imgcache/cache_hit_internal_test.go index 23e2e07..828ce50 100644 --- a/internal/imgcache/cache_hit_internal_test.go +++ b/internal/imgcache/cache_hit_internal_test.go @@ -32,7 +32,7 @@ func TestVariant_ContentTypeFromFirstBytes(t *testing.T) { imagerequest.FormatWebP, imagerequest.FormatAVIF, imagerequest.FormatJXL, } { result, err := processor.Process(t.Context(), bytes.NewReader(source), - &imagerequest.Transform{ + imagerequest.FormatJPEG, &imagerequest.Transform{ Size: imagerequest.Size{Width: 20, Height: 15}, Format: format, Quality: 85, @@ -68,7 +68,7 @@ func TestVariant_ContentTypeFromFirstBytes(t *testing.T) { t.Logf("%s: %d bytes served as %s", format, size, contentType) - if want := imageprocessor.FormatToMIME(format); contentType != want { + if want := format.MIMEType(); contentType != want { t.Errorf("%s variant content type = %q, want %q", format, contentType, want) } @@ -96,7 +96,7 @@ func TestVariant_RefusesFileOfNoOutputFormat(t *testing.T) { }{ {"text", []byte("this is text, not an image"), magic.ErrUnknownFormat}, {"SVG", []byte(``), - errNotAnOutputFormat}, + magic.ErrUnknownFormat}, {"JPEG cut to 10 bytes", jpegImage[:10], io.EOF}, {"empty", nil, io.EOF}, } diff --git a/internal/imgcache/eviction_internal_test.go b/internal/imgcache/eviction_internal_test.go index 7be430e..15b2ebc 100644 --- a/internal/imgcache/eviction_internal_test.go +++ b/internal/imgcache/eviction_internal_test.go @@ -455,7 +455,7 @@ func assertDisabledCacheReadsAlwaysMiss( ctx := t.Context() - reader, _, err := cache.Source(ctx, req.Source) + reader, _, _, err := cache.Source(ctx, req.Source) if !errors.Is(err, ErrNotFound) { t.Fatalf("Source on disabled cache error = %v, want %v", err, ErrNotFound) } diff --git a/internal/imgcache/pendingaccesstimes_internal_test.go b/internal/imgcache/pendingaccesstimes_internal_test.go index b21ca78..d9ceb65 100644 --- a/internal/imgcache/pendingaccesstimes_internal_test.go +++ b/internal/imgcache/pendingaccesstimes_internal_test.go @@ -5,6 +5,7 @@ import ( "context" "database/sql" "errors" + "image/color" "io" "log/slog" "strings" @@ -204,8 +205,8 @@ func TestSource_HandsAccessTimeToTheBackgroundWriter(t *testing.T) { t.Parallel() cache, _ := newEvictionTestCache(t, 1<<30) - key := storeEvictionTestSource(t, cache, "example.com", "/photo.jpg", - []byte("source bytes")) + content := generateTestJPEG(t, 10, 10, color.RGBA{0, 0, 255, 255}) + key := storeEvictionTestSource(t, cache, "example.com", "/photo.jpg", content) anHourAgo := time.Now().Add(-time.Hour) setSourceLastAccessed(t, cache, key, anHourAgo) @@ -215,7 +216,7 @@ func TestSource_HandsAccessTimeToTheBackgroundWriter(t *testing.T) { ` anHourAgoInDatabase := anHourAgo.UTC().Format(sqliteTimestampFormat) - reader, _, err := cache.Source(t.Context(), + reader, _, _, err := cache.Source(t.Context(), imagerequest.Source{Host: "example.com", Path: "/photo.jpg"}) if err != nil { t.Fatalf("Source() error = %v; want the source of %q", err, key) @@ -224,7 +225,7 @@ func TestSource_HandsAccessTimeToTheBackgroundWriter(t *testing.T) { found, err := io.ReadAll(reader) _ = reader.Close() - if err != nil || string(found) != "source bytes" { + if err != nil || !bytes.Equal(found, content) { t.Fatalf("Source() reads %q, %v; want the source of %q", found, err, key) } diff --git a/internal/imgcache/source_content_internal_test.go b/internal/imgcache/source_content_internal_test.go index b9d7adb..60a9be4 100644 --- a/internal/imgcache/source_content_internal_test.go +++ b/internal/imgcache/source_content_internal_test.go @@ -2,12 +2,17 @@ package imgcache import ( "bytes" + "errors" + "fmt" + "image/color" "io" "os" "path/filepath" "testing" + "sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imagerequest" + "sneak.berlin/go/pixa/internal/magic" ) // TestPutSource_StoresIdenticalBytesOfTwoURLsTwice stores the same bytes @@ -64,7 +69,7 @@ func TestEvictSourceBlob_LeavesSourceWithIdenticalBytesServed(t *testing.T) { t.Parallel() cache, _ := newEvictionTestCache(t, 1<<30) - content := bytes.Repeat([]byte{0x5B}, 400) + content := generateTestJPEG(t, 20, 20, color.RGBA{0x5B, 0x5B, 0x5B, 255}) evicted := storeEvictionTestSource(t, cache, testHostCDN, "/evicted.jpg", content) storeEvictionTestSource(t, cache, testHostCDN, "/kept.jpg", content) @@ -85,7 +90,7 @@ func TestEvictSourceBlob_LeavesSourceWithIdenticalBytesServed(t *testing.T) { t.Errorf("the evicted source still has %d source_content rows", n) } - reader, _, err := cache.Source(t.Context(), + reader, _, _, err := cache.Source(t.Context(), imagerequest.Source{Host: testHostCDN, Path: "/kept.jpg"}) t.Logf("Source() of the kept source: error %v", err) @@ -135,3 +140,99 @@ func TestPutSource_FailedRowWriteLeavesNoFile(t *testing.T) { t.Error("the source's file is on disk although its row was not written") } } + +// TestSource_FormatFromFirstBytes stores an image in each source format +// under one source URL, in turn. After each store, Source must return that +// image whole, with its format read from the file. +func TestSource_FormatFromFirstBytes(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1<<30) + processor := imageprocessor.New(imageprocessor.Params{ + MaxInputBytes: 50 << 20, + MaxConcurrentProcessing: 1, + }) + jpegImage := generateTestJPEG(t, 40, 30, color.RGBA{200, 40, 40, 255}) + src := imagerequest.Source{Host: testHostCDN, Path: testPathCat} + + for _, format := range []imagerequest.Format{ + imagerequest.FormatJPEG, imagerequest.FormatPNG, imagerequest.FormatGIF, + imagerequest.FormatWebP, imagerequest.FormatAVIF, imagerequest.FormatJXL, + } { + result, err := processor.Process(t.Context(), bytes.NewReader(jpegImage), + imagerequest.FormatJPEG, &imagerequest.Transform{Format: format}) + if err != nil { + t.Fatalf("making a %s image: %v", format, err) + } + + image, err := io.ReadAll(result.Content) + if err != nil { + t.Fatalf("reading the %s image: %v", format, err) + } + + _, err = cache.PutSource(t.Context(), src, bytes.NewReader(image)) + if err != nil { + t.Fatalf("PutSource(%s) error = %v", format, err) + } + + reader, size, got, err := cache.Source(t.Context(), src) + if err != nil { + t.Fatalf("Source() of the %s image error = %v", format, err) + } + + served, err := io.ReadAll(reader) + _ = reader.Close() + + if err != nil { + t.Fatalf("reading the %s source: %v", format, err) + } + + t.Logf("%s: %d bytes read as %s", format, size, got) + + if got != format { + t.Errorf("Source() of the %s image format = %q, want %q", + format, got, format) + } + + if size != int64(len(image)) || !bytes.Equal(served, image) { + t.Errorf("%s source read as %d bytes, size %d, want the %d bytes stored", + format, len(served), size, len(image)) + } + } +} + +// TestSource_RefusesFileOfNoSourceFormat checks that Source refuses a +// cached source whose file's first bytes are not those of a source format, +// with an error other than ErrNotFound. +func TestSource_RefusesFileOfNoSourceFormat(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1<<30) + jpegImage := generateTestJPEG(t, 10, 10, color.RGBA{0, 0, 255, 255}) + + tests := []struct { + name string + content []byte + 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}, + } + + for i, tt := range tests { + path := fmt.Sprintf("/refused%d.jpg", i) + storeEvictionTestSource(t, cache, testHostCDN, path, tt.content) + + reader, _, format, err := cache.Source(t.Context(), + imagerequest.Source{Host: testHostCDN, Path: path}) + t.Logf("%s: Source() error = %v", tt.name, err) + + if reader != nil || format != "" || !errors.Is(err, tt.wantErr) { + t.Errorf("%s: Source() = reader %v, format %q, error %v; "+ + "want no reader and %v", tt.name, reader, format, err, tt.wantErr) + } + } +} diff --git a/internal/magic/jpegxl_internal_test.go b/internal/magic/jpegxl_internal_test.go index 45d9197..e7aa910 100644 --- a/internal/magic/jpegxl_internal_test.go +++ b/internal/magic/jpegxl_internal_test.go @@ -1,12 +1,17 @@ package magic -import "testing" +import ( + "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" +) // testMIMEJXL is the MIME type of JPEG XL. const testMIMEJXL = "image/jxl" // TestDetectFormatJPEGXL verifies that both JPEG XL signatures, that of a -// bare codestream and that of the container, are detected as image/jxl. +// bare codestream and that of the container, are detected as JPEG XL and +// match the MIME type image/jxl. func TestDetectFormatJPEGXL(t *testing.T) { t.Parallel() @@ -25,25 +30,15 @@ func TestDetectFormatJPEGXL(t *testing.T) { t.Parallel() got, err := DetectFormat(tt.data) - if err != nil || got != MIMETypeJXL { - t.Errorf("DetectFormat() = %q, %v, want %q", got, err, testMIMEJXL) + if err != nil || got != imagerequest.FormatJXL { + t.Errorf("DetectFormat() = %q, %v, want %q", + got, err, imagerequest.FormatJXL) } - err = ValidateMagicBytes(tt.data, testMIMEJXL) + _, err = ValidateMagicBytes(tt.data, testMIMEJXL) if err != nil { t.Errorf("ValidateMagicBytes(%q) error = %v", testMIMEJXL, err) } }) } } - -// TestJPEGXLMIMEType verifies that image/jxl maps to the format jxl. -func TestJPEGXLMIMEType(t *testing.T) { - t.Parallel() - - format, ok := MIMEToImageFormat(testMIMEJXL) - if format != FormatJXL || !ok { - t.Errorf("MIMEToImageFormat(%q) = %q, %v, want %q, true", - testMIMEJXL, format, ok, FormatJXL) - } -} diff --git a/internal/magic/magic.go b/internal/magic/magic.go index db7781a..4d069ba 100644 --- a/internal/magic/magic.go +++ b/internal/magic/magic.go @@ -6,6 +6,8 @@ import ( "bytes" "errors" "strings" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // Magic byte errors. @@ -15,34 +17,6 @@ var ( ErrNotEnoughData = errors.New("not enough data to detect format") ) -// MIMEType represents a supported MIME type for input images. -type MIMEType string - -// Supported input MIME types. -const ( - MIMETypeJPEG = MIMEType("image/jpeg") - MIMETypePNG = MIMEType("image/png") - MIMETypeWebP = MIMEType("image/webp") - MIMETypeGIF = MIMEType("image/gif") - MIMETypeAVIF = MIMEType("image/avif") - MIMETypeJXL = MIMEType("image/jxl") - MIMETypeSVG = MIMEType("image/svg+xml") -) - -// ImageFormat represents supported output image formats. -// This mirrors the type in imgcache to avoid circular imports. -type ImageFormat string - -// Supported image output formats. -const ( - FormatJPEG ImageFormat = "jpeg" - FormatPNG ImageFormat = "png" - FormatWebP ImageFormat = "webp" - FormatAVIF ImageFormat = "avif" - FormatJXL ImageFormat = "jxl" - FormatGIF ImageFormat = "gif" -) - // MinMagicBytes is the minimum number of bytes needed to detect format. const MinMagicBytes = 12 @@ -78,32 +52,32 @@ var ( avisBrand = []byte{0x61, 0x76, 0x69, 0x73} // "avis" (AVIF sequence) ) -// DetectFormat detects the image format from magic bytes. -// Returns the MIME type and nil error on success. -func DetectFormat(data []byte) (MIMEType, error) { +// DetectFormat detects the image format from magic bytes. Data in none of +// the formats pixa reads, SVG included, is ErrUnknownFormat. +func DetectFormat(data []byte) (imagerequest.Format, error) { if len(data) < MinMagicBytes { return "", ErrNotEnoughData } // Check JPEG (FFD8FF) if bytes.HasPrefix(data, magicJPEG) { - return MIMETypeJPEG, nil + return imagerequest.FormatJPEG, nil } // Check PNG (89504E47 0D0A1A0A) if bytes.HasPrefix(data, magicPNG) { - return MIMETypePNG, nil + return imagerequest.FormatPNG, nil } // Check GIF (GIF87a or GIF89a) if bytes.HasPrefix(data, magicGIF) { - return MIMETypeGIF, nil + return imagerequest.FormatGIF, nil } // Check WebP (RIFF....WEBP) if bytes.HasPrefix(data, magicWebP) && len(data) >= 12 { if bytes.Equal(data[8:12], webpIdent) { - return MIMETypeWebP, nil + return imagerequest.FormatWebP, nil } } @@ -112,66 +86,39 @@ func DetectFormat(data []byte) (MIMEType, error) { if len(data) >= 12 && bytes.Equal(data[4:8], magicFtyp) { brand := data[8:12] if bytes.Equal(brand, avifBrand) || bytes.Equal(brand, avisBrand) { - return MIMETypeAVIF, nil + return imagerequest.FormatAVIF, nil } } // Check JPEG XL (FF0A, or the 12-byte container signature) if bytes.HasPrefix(data, magicJXLCodestream) || bytes.HasPrefix(data, magicJXLContainer) { - return MIMETypeJXL, nil - } - - // Check SVG - look for XML declaration or SVG tag - if detectSVG(data) { - return MIMETypeSVG, nil + return imagerequest.FormatJXL, nil } return "", ErrUnknownFormat } -// detectSVG checks if data appears to be SVG content. -func detectSVG(data []byte) bool { - // Skip BOM if present - content := skipBOM(data) - - // Convert to string for easier pattern matching - s := strings.ToLower(string(content)) - - // Skip leading whitespace - s = strings.TrimSpace(s) - - // Check for XML declaration or SVG element - return strings.HasPrefix(s, "= 3 && data[0] == 0xEF && data[1] == 0xBB && data[2] == 0xBF { - return data[3:] - } - - return data -} - -// ValidateMagicBytes validates that the content matches the declared MIME type. -func ValidateMagicBytes(data []byte, declaredType string) error { +// ValidateMagicBytes detects the format of data from its magic bytes, and +// returns it when it is the format the declared MIME type names. Data in +// another format is ErrMagicByteMismatch. +func ValidateMagicBytes( + data []byte, declaredType string, +) (imagerequest.Format, error) { detected, err := DetectFormat(data) if err != nil { - return err + return "", err } // Normalize the declared type (remove parameters like charset) normalizedDeclared := normalizeMIMEType(declaredType) // Check if they match - if string(detected) != normalizedDeclared { - return ErrMagicByteMismatch + if detected.MIMEType() != normalizedDeclared { + return "", ErrMagicByteMismatch } - return nil + return detected, nil } // normalizeMIMEType extracts just the media type, removing parameters. @@ -183,27 +130,3 @@ func normalizeMIMEType(mimeType string) string { return strings.TrimSpace(strings.ToLower(mimeType)) } - -// MIMEToImageFormat converts a MIME type to an ImageFormat. -func MIMEToImageFormat(mimeType string) (ImageFormat, bool) { - normalized := normalizeMIMEType(mimeType) - switch MIMEType(normalized) { - case MIMETypeJPEG: - return FormatJPEG, true - case MIMETypePNG: - return FormatPNG, true - case MIMETypeWebP: - return FormatWebP, true - case MIMETypeGIF: - return FormatGIF, true - case MIMETypeAVIF: - return FormatAVIF, true - case MIMETypeJXL: - return FormatJXL, true - case MIMETypeSVG: - // SVG has no corresponding output format. - return "", false - default: - return "", false - } -} diff --git a/internal/magic/magic_internal_test.go b/internal/magic/magic_internal_test.go index fc081ea..5d9b25f 100644 --- a/internal/magic/magic_internal_test.go +++ b/internal/magic/magic_internal_test.go @@ -1,9 +1,10 @@ package magic import ( - "bytes" "errors" "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // Shared test fixture strings. @@ -12,9 +13,6 @@ const ( testMIMEJPEG = "image/jpeg" testMIMEJPEGParams = "image/jpeg; charset=utf-8" testMIMEPNG = "image/png" - testMIMEWebP = "image/webp" - testMIMEGIF = "image/gif" - testMIMEAVIF = "image/avif" ) // pad appends zero bytes so data is comfortably above MinMagicBytes. @@ -36,37 +34,22 @@ func TestDetectFormat(t *testing.T) { avis := pad(0x00, 0x00, 0x00, 0x1C, 0x66, 0x74, 0x79, 0x70, 0x61, 0x76, 0x69, 0x73) tests := []struct { - name string - data []byte - wantMIME MIMEType - wantErr error + name string + data []byte + wantFormat imagerequest.Format + wantErr error }{ - {name: "JPEG", data: jpeg, wantMIME: MIMETypeJPEG}, - {name: "PNG", data: png, wantMIME: MIMETypePNG}, - {name: "GIF87a", data: gif87a, wantMIME: MIMETypeGIF}, - {name: "GIF89a", data: gif89a, wantMIME: MIMETypeGIF}, - {name: "WebP", data: webp, wantMIME: MIMETypeWebP}, - {name: "AVIF", data: avif, wantMIME: MIMETypeAVIF}, - {name: "AVIF sequence", data: avis, wantMIME: MIMETypeAVIF}, + {name: "JPEG", data: jpeg, wantFormat: imagerequest.FormatJPEG}, + {name: "PNG", data: png, wantFormat: imagerequest.FormatPNG}, + {name: "GIF87a", data: gif87a, wantFormat: imagerequest.FormatGIF}, + {name: "GIF89a", data: gif89a, wantFormat: imagerequest.FormatGIF}, + {name: "WebP", data: webp, wantFormat: imagerequest.FormatWebP}, + {name: "AVIF", data: avif, wantFormat: imagerequest.FormatAVIF}, + {name: "AVIF sequence", data: avis, wantFormat: imagerequest.FormatAVIF}, { - name: "SVG with XML declaration", - data: []byte(``), - wantMIME: MIMETypeSVG, - }, - { - name: "SVG without declaration", - data: []byte(``), - wantMIME: MIMETypeSVG, - }, - { - name: "SVG with whitespace", - data: []byte(` `), - wantMIME: MIMETypeSVG, - }, - { - name: "SVG with BOM", - data: append([]byte{0xEF, 0xBB, 0xBF}, []byte(``)...), - wantMIME: MIMETypeSVG, + name: "SVG", + data: []byte(``), + wantErr: ErrUnknownFormat, }, { name: "unknown format", @@ -88,8 +71,8 @@ func TestDetectFormat(t *testing.T) { return } - if got != tt.wantMIME { - t.Errorf("DetectFormat() = %v, want %v", got, tt.wantMIME) + if got != tt.wantFormat { + t.Errorf("DetectFormat() = %v, want %v", got, tt.wantFormat) } }) } @@ -105,24 +88,28 @@ func TestValidateMagicBytes(t *testing.T) { name string data []byte declaredType string + wantFormat imagerequest.Format wantErr error }{ { name: "matching JPEG", data: jpegData, declaredType: testMIMEJPEG, + wantFormat: imagerequest.FormatJPEG, wantErr: nil, }, { name: "matching JPEG with params", data: jpegData, declaredType: testMIMEJPEGParams, + wantFormat: imagerequest.FormatJPEG, wantErr: nil, }, { name: "matching PNG", data: pngData, declaredType: testMIMEPNG, + wantFormat: imagerequest.FormatPNG, wantErr: nil, }, { @@ -143,45 +130,14 @@ func TestValidateMagicBytes(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := ValidateMagicBytes(tt.data, tt.declaredType) + got, err := ValidateMagicBytes(tt.data, tt.declaredType) if !errors.Is(err, tt.wantErr) { t.Errorf("ValidateMagicBytes() error = %v, wantErr %v", err, tt.wantErr) } - }) - } -} - -func TestMIMEToImageFormat(t *testing.T) { - t.Parallel() - - tests := []struct { - mimeType string - wantFormat ImageFormat - wantOk bool - }{ - {testMIMEJPEG, FormatJPEG, true}, - {testMIMEPNG, FormatPNG, true}, - {testMIMEWebP, FormatWebP, true}, - {testMIMEGIF, FormatGIF, true}, - {testMIMEAVIF, FormatAVIF, true}, - {"image/svg+xml", "", false}, // SVG doesn't convert to ImageFormat - {"image/tiff", "", false}, - {"text/plain", "", false}, - } - - for _, tt := range tests { - t.Run(tt.mimeType, func(t *testing.T) { - t.Parallel() - - got, ok := MIMEToImageFormat(tt.mimeType) - - if ok != tt.wantOk { - t.Errorf("MIMEToImageFormat(%q) ok = %v, want %v", tt.mimeType, ok, tt.wantOk) - } if got != tt.wantFormat { - t.Errorf("MIMEToImageFormat(%q) = %v, want %v", tt.mimeType, got, tt.wantFormat) + t.Errorf("ValidateMagicBytes() = %q, want %q", got, tt.wantFormat) } }) } @@ -214,109 +170,6 @@ func TestNormalizeMIMEType(t *testing.T) { } } -func TestDetectSVG(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - data string - want bool - }{ - {"xml declaration", ``, true}, - {"svg element", ``, true}, - { - "doctype", - ``, - true, - }, - {"with whitespace", ` - `, true}, - {"uppercase", ``, true}, - {"not svg", ``, false}, - {"random text", `hello world`, false}, - {testNameEmpty, ``, false}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - got := detectSVG([]byte(tt.data)) - - if got != tt.want { - t.Errorf("detectSVG(%q) = %v, want %v", tt.data, got, tt.want) - } - }) - } -} - -func TestSkipBOM(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - data []byte - want []byte - }{ - {"with BOM", []byte{0xEF, 0xBB, 0xBF, 'h', 'e', 'l', 'l', 'o'}, []byte("hello")}, - {"without BOM", []byte("hello"), []byte("hello")}, - {testNameEmpty, []byte{}, []byte{}}, - {"only BOM", []byte{0xEF, 0xBB, 0xBF}, []byte{}}, - {"partial BOM", []byte{0xEF, 0xBB}, []byte{0xEF, 0xBB}}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - got := skipBOM(tt.data) - - if !bytes.Equal(got, tt.want) { - t.Errorf("skipBOM() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestRealWorldSVGPatterns(t *testing.T) { - t.Parallel() - - // Test various real-world SVG patterns - svgPatterns := []string{ - ` - - -`, - ` - -`, - `` + ` - -`, - } - - for i, pattern := range svgPatterns { - data := []byte(pattern) - if len(data) < MinMagicBytes { - // Pad short SVGs for detection - data = append(data, make([]byte, MinMagicBytes-len(data))...) - } - - got, err := DetectFormat(data) - if err != nil { - t.Errorf("Pattern %d: DetectFormat() error = %v", i, err) - - continue - } - - if got != MIMETypeSVG { - t.Errorf("Pattern %d: DetectFormat() = %v, want %v", i, got, MIMETypeSVG) - } - } -} - func TestDetectFormatRIFFNotWebP(t *testing.T) { t.Parallel() -- 2.54.0