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