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 stores that format in the source's row, so a variant made from a
cached source uses it too. The proxy hands it to Process, which still
refuses 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

@@ -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,

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

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

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