Detect a fetched image's format from its bytes once (closes #319)

magic.DetectFormat returns an imagerequest.Format. ValidateMagicBytes
returns the format it checked against the upstream Content-Type. The
cache reads a cached source's format from its file's first bytes, as it
reads a variant's. The proxy hands the format to Process. Process
refuses a format that is not a source format, and input libvips would
load with another loader, such as an SVG passing as AVIF.
Format.MIMEType declares each MIME string once, and the fetcher, the
cache's variant read and the Accept matching use it. magic.MIMEType,
magic.ImageFormat, MIMEToImageFormat, magic's SVG detection,
detectFormat, formatFromString, FormatToMIME and Result.InputFormat are
gone.

Model: opus-5-5

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,

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

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

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

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