diff --git a/README.md b/README.md index 0f3271a..cadb572 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. A source image's row also holds its +format, which pixa reads from the image's first bytes when it fetches it. 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..31959f0 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,28 @@ 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. + - The cache stores a source's format in its `source_content` row, and + `Cache.Source` returns it. `Cache.PutSource` takes it. + - The proxy hands the format to `ImageProcessor.Process`. The processor + refuses input that libvips would load as another format, as before, with + `ErrUnsupportedInputFormat`. + - `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/database/concurrent_writes_internal_test.go b/internal/database/concurrent_writes_internal_test.go index 8dcfdbf..fba1164 100644 --- a/internal/database/concurrent_writes_internal_test.go +++ b/internal/database/concurrent_writes_internal_test.go @@ -122,7 +122,7 @@ func writeConcurrently(t *testing.T, db *sql.DB) { // record again. func writeLikeOneRequest(ctx context.Context, db *sql.DB, key string) error { _, err := db.ExecContext(ctx, `INSERT INTO source_content - (source_key, size_bytes) VALUES (?, 1)`, key) + (source_key, size_bytes, format) VALUES (?, 1, 'jpeg')`, key) if err != nil { return fmt.Errorf("storing a source: %w", err) } diff --git a/internal/db/migrations/001_schema.sql b/internal/db/migrations/001_schema.sql index 8aa1696..efe09f9 100644 --- a/internal/db/migrations/001_schema.sql +++ b/internal/db/migrations/001_schema.sql @@ -3,10 +3,12 @@ -- Source images, one row per source URL -- Files stored at: cache/sources///, where source_key --- is the SHA-256 of the source's host, path and query +-- is the SHA-256 of the source's host, path and query. format is the +-- image's format, detected from its first bytes when it was fetched. CREATE TABLE IF NOT EXISTS source_content ( source_key TEXT PRIMARY KEY, size_bytes INTEGER NOT NULL, + format TEXT NOT NULL, last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ); CREATE INDEX IF NOT EXISTS idx_source_content_last_accessed 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..37d71a4 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,9 @@ 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. var ErrUnsupportedInputFormat = errors.New("unsupported input format") // ErrTooManyImages is returned when MaxConcurrentProcessing images are being @@ -157,9 +155,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 +167,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) @@ -183,14 +183,13 @@ func (p *ImageProcessor) Process( } // 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) - if err != nil { - return nil, err + // DetermineImageType. Input it would not load with the loader of + // sourceFormat is refused before libvips reads it. An SVG whose first + // bytes pass the first-bytes check for AVIF is one, which librsvg would + // otherwise render. + if vips.DetermineImageType(data) != vipsImageType(sourceFormat) { + return nil, fmt.Errorf("%w: libvips does not read the input as %s", + ErrUnsupportedInputFormat, sourceFormat) } // Decode image @@ -254,12 +253,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 +292,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 +360,27 @@ 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" +// 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 +734,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..dd54e0c 100644 --- a/internal/imageprocessor/source_formats_internal_test.go +++ b/internal/imageprocessor/source_formats_internal_test.go @@ -21,7 +21,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 +32,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 +42,9 @@ 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()) } } } diff --git a/internal/imageproxy/imageproxy.go b/internal/imageproxy/imageproxy.go index 7df0381..fd5bf23 100644 --- a/internal/imageproxy/imageproxy.go +++ b/internal/imageproxy/imageproxy.go @@ -304,20 +304,20 @@ func withoutCancelKeepingDeadline( } // 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. +// returns it with its size and its format; nil if it is not in the cache, +// cannot be read, is empty 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 +330,16 @@ func (p *Proxy) loadCachedSource( "max_bytes", p.maxResponseSize, ) - return nil, 0 + return nil, 0, "" } if size == 0 { _ = reader.Close() - return nil, 0 + return nil, 0, "" } - return reader, size + return reader, size, format } // processFromSourceOrFetch processes an image, using cached source content @@ -349,7 +349,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 +361,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 +442,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, @@ -456,14 +457,15 @@ func (p *Proxy) fetchAndProcess( } // Store source content - _, err = p.cache.PutSource(ctx, req.Source, bytes.NewReader(sourceData)) + _, err = p.cache.PutSource(ctx, req.Source, sourceFormat, + bytes.NewReader(sourceData)) if err != nil { p.log.Warn("failed to store source content", "error", err) // Continue even if caching fails } return p.processAndStore( - ctx, req, cacheKey, bytes.NewReader(sourceData), fetchBytes, + ctx, req, cacheKey, bytes.NewReader(sourceData), sourceFormat, fetchBytes, ) } @@ -497,13 +499,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 +514,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 +550,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..872afa9 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}) @@ -79,7 +80,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..48dace1 100644 --- a/internal/imageproxy/partial_cache_files_internal_test.go +++ b/internal/imageproxy/partial_cache_files_internal_test.go @@ -49,7 +49,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) } 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..98caf9b 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,44 +227,46 @@ 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, the one PutSource stored with it. 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). 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() - var rowExists bool + var format imagerequest.Format err := c.db.QueryRowContext(ctx, ` - 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) + SELECT format FROM source_content WHERE source_key = ? + `, string(key)).Scan(&format) + if errors.Is(err, sql.ErrNoRows) { + return nil, 0, "", ErrNotFound } - if !rowExists { - return nil, 0, ErrNotFound + if err != nil { + return nil, 0, "", fmt.Errorf("failed to lookup source: %w", err) } file, size, err := c.srcContent.Open(string(key)) if err != nil { - return nil, 0, err + 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 -// the source's key, then the source's row. When the row cannot be written +// PutSource stores fetched source content and its format, which magic +// detected from its first bytes. It writes the file named by the source's +// key, then the source's row with the format. When the row cannot be written // the file is removed and the store fails, as a file with no row is never // served or evicted; if that removal fails too, the next reconciliation // pass removes the file. On a disabled cache it is a no-op returning an @@ -272,6 +274,7 @@ func (c *Cache) Source( func (c *Cache) PutSource( ctx context.Context, src imagerequest.Source, + format imagerequest.Format, content io.Reader, ) (imagerequest.SourceKey, error) { if c.config.MaxBytes == 0 { @@ -293,12 +296,13 @@ func (c *Cache) PutSource( size := int64(len(data)) _, err = c.db.ExecContext(ctx, ` - INSERT INTO source_content (source_key, size_bytes) - VALUES (?, ?) + INSERT INTO source_content (source_key, size_bytes, format) + VALUES (?, ?, ?) ON CONFLICT(source_key) DO UPDATE SET size_bytes = excluded.size_bytes, + format = excluded.format, last_accessed_at = CURRENT_TIMESTAMP - `, string(key), size) + `, string(key), size, string(format)) if err != nil { deleteErr := c.srcContent.Delete(string(key)) if deleteErr != nil { @@ -440,13 +444,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 content, without moving its offset, +// and returns the format they show. pixa writes each format it reads, so +// that is an output format. +func readFormat(content io.ReaderAt) (imagerequest.Format, error) { firstBytes := make([]byte, magic.MinMagicBytes) _, err := content.ReadAt(firstBytes, 0) @@ -454,15 +455,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..98484ec 100644 --- a/internal/imgcache/eviction_internal_test.go +++ b/internal/imgcache/eviction_internal_test.go @@ -79,8 +79,9 @@ func newEvictionTestCache(t *testing.T, maxBytes int64) (*Cache, string) { return cache, tmpDir } -// storeEvictionTestSource stores content as a fetched source for -// host/path and returns the resulting source key. +// storeEvictionTestSource stores content as a fetched JPEG source for +// host/path and returns the resulting source key. The cache does not read +// the content, so it need not be a JPEG. func storeEvictionTestSource( t *testing.T, cache *Cache, host, path string, content []byte, ) imagerequest.SourceKey { @@ -99,7 +100,8 @@ func storeEvictionTestSource( } key, err := cache.PutSource( - context.Background(), req.Source, bytes.NewReader(content), + context.Background(), req.Source, imagerequest.FormatJPEG, + bytes.NewReader(content), ) if err != nil { t.Fatalf("PutSource(%s%s) failed: %v", host, path, err) @@ -436,7 +438,8 @@ func assertDisabledCacheWritesAreNoOps( t.Fatalf("PutVariant on disabled cache must be a no-op, got error: %v", err) } - key, err := cache.PutSource(ctx, req.Source, bytes.NewReader([]byte("data"))) + key, err := cache.PutSource(ctx, req.Source, imagerequest.FormatJPEG, + bytes.NewReader([]byte("data"))) if err != nil { t.Fatalf("PutSource on disabled cache must be a no-op, got error: %v", err) } @@ -455,7 +458,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..193f4aa 100644 --- a/internal/imgcache/pendingaccesstimes_internal_test.go +++ b/internal/imgcache/pendingaccesstimes_internal_test.go @@ -215,7 +215,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) diff --git a/internal/imgcache/source_content_internal_test.go b/internal/imgcache/source_content_internal_test.go index b9d7adb..eb57e8e 100644 --- a/internal/imgcache/source_content_internal_test.go +++ b/internal/imgcache/source_content_internal_test.go @@ -85,7 +85,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) @@ -122,7 +122,7 @@ func TestPutSource_FailedRowWriteLeavesNoFile(t *testing.T) { req := &imagerequest.Request{ Source: imagerequest.Source{Host: testHostCDN, Path: testPathCat}, } - key, err := cache.PutSource(t.Context(), req.Source, + key, err := cache.PutSource(t.Context(), req.Source, imagerequest.FormatJPEG, bytes.NewReader([]byte("source bytes"))) t.Logf("PutSource() = %q, error = %v", key, err) @@ -135,3 +135,37 @@ func TestPutSource_FailedRowWriteLeavesNoFile(t *testing.T) { t.Error("the source's file is on disk although its row was not written") } } + +// TestSource_ReturnsTheFormatPutSourceStored stores a source as PNG, then +// the same source URL again as WebP. After each store, Source must return +// the format that store gave. +func TestSource_ReturnsTheFormatPutSourceStored(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1<<30) + src := imagerequest.Source{Host: testHostCDN, Path: testPathCat} + + for _, format := range []imagerequest.Format{ + imagerequest.FormatPNG, imagerequest.FormatWebP, + } { + _, err := cache.PutSource(t.Context(), src, format, + bytes.NewReader([]byte("source bytes"))) + if err != nil { + t.Fatalf("PutSource(%s) error = %v", format, err) + } + + reader, _, got, err := cache.Source(t.Context(), src) + if err != nil { + t.Fatalf("Source() after PutSource(%s) error = %v", format, err) + } + + _ = reader.Close() + + t.Logf("Source() after PutSource(%s): format %s", format, got) + + if got != format { + t.Errorf("Source() after PutSource(%s) format = %q, want %q", + format, got, format) + } + } +} 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()