From b55b75cbe7a87feb92ace6748ad00751277dd55d Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 8 Jan 2026 11:08:22 -0800 Subject: [PATCH] Fix silent fallbacks for unsupported formats and fit modes - Return ErrUnsupportedOutputFormat for WebP/AVIF encoding - Return ErrInvalidFitMode for unknown fit mode values - Add ValidateFitMode() for input validation - Validate fit mode at handler level before processing Silent fallbacks violate the principle of least surprise and mask bugs. When a user explicitly specifies a value, we should either honor it or return an error - never silently substitute a different value. --- internal/handlers/image.go | 5 ++++ internal/imgcache/imgcache.go | 15 ++++++++++++ internal/imgcache/processor.go | 44 ++++++++++++++++------------------ 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/internal/handlers/image.go b/internal/handlers/image.go index 4f71217..d82c34f 100644 --- a/internal/handlers/image.go +++ b/internal/handlers/image.go @@ -54,6 +54,11 @@ func (s *Handlers) HandleImage() http.HandlerFunc { if fit := query.Get("fit"); fit != "" { req.FitMode = imgcache.FitMode(fit) + if err := imgcache.ValidateFitMode(req.FitMode); err != nil { + s.respondError(w, "invalid fit mode: "+fit, http.StatusBadRequest) + + return + } } // Default quality if not set diff --git a/internal/imgcache/imgcache.go b/internal/imgcache/imgcache.go index 7307dab..62d533d 100644 --- a/internal/imgcache/imgcache.go +++ b/internal/imgcache/imgcache.go @@ -3,11 +3,15 @@ package imgcache import ( "context" + "errors" "io" "net/url" "time" ) +// ErrInvalidFitMode is returned when an invalid fit mode is provided. +var ErrInvalidFitMode = errors.New("invalid fit mode") + // ImageFormat represents supported output image formats. type ImageFormat string @@ -44,6 +48,17 @@ const ( FitOutside FitMode = "outside" ) +// ValidateFitMode checks if the given fit mode is valid. +// Returns ErrInvalidFitMode for unrecognized fit modes. +func ValidateFitMode(fit FitMode) error { + switch fit { + case FitCover, FitContain, FitFill, FitInside, FitOutside, "": + return nil + default: + return ErrInvalidFitMode + } +} + // ImageRequest represents a request for a processed image type ImageRequest struct { // SourceHost is the origin host (e.g., "cdn.example.com") diff --git a/internal/imgcache/processor.go b/internal/imgcache/processor.go index caca8f3..dc95f59 100644 --- a/internal/imgcache/processor.go +++ b/internal/imgcache/processor.go @@ -22,6 +22,9 @@ const MaxInputDimension = 8192 // ErrInputTooLarge is returned when input image dimensions exceed MaxInputDimension. var ErrInputTooLarge = errors.New("input image dimensions exceed maximum") +// ErrUnsupportedOutputFormat is returned when the requested output format is not supported. +var ErrUnsupportedOutputFormat = errors.New("unsupported output format") + // ImageProcessor implements the Processor interface using pure Go libraries. type ImageProcessor struct{} @@ -70,7 +73,11 @@ func (p *ImageProcessor) Process( // Resize if needed if targetWidth != origWidth || targetHeight != origHeight { - img = p.resize(img, targetWidth, targetHeight, req.FitMode) + var resizeErr error + img, resizeErr = p.resize(img, targetWidth, targetHeight, req.FitMode) + if resizeErr != nil { + return nil, fmt.Errorf("failed to resize: %w", resizeErr) + } } // Determine output format @@ -115,7 +122,6 @@ func (p *ImageProcessor) SupportedOutputFormats() []ImageFormat { FormatJPEG, FormatPNG, FormatGIF, - // WebP encoding not supported in pure Go, will fall back to PNG } } @@ -141,19 +147,19 @@ func (p *ImageProcessor) decode(data []byte) (image.Image, string, error) { } // resize resizes the image according to the fit mode. -func (p *ImageProcessor) resize(img image.Image, width, height int, fit FitMode) image.Image { +func (p *ImageProcessor) resize(img image.Image, width, height int, fit FitMode) (image.Image, error) { switch fit { - case FitCover: - // Resize and crop to fill exact dimensions - return imaging.Fill(img, width, height, imaging.Center, imaging.Lanczos) + case FitCover, "": + // Resize and crop to fill exact dimensions (default) + return imaging.Fill(img, width, height, imaging.Center, imaging.Lanczos), nil case FitContain: // Resize to fit within dimensions, maintaining aspect ratio - return imaging.Fit(img, width, height, imaging.Lanczos) + return imaging.Fit(img, width, height, imaging.Lanczos), nil case FitFill: // Resize to exact dimensions (may distort) - return imaging.Resize(img, width, height, imaging.Lanczos) + return imaging.Resize(img, width, height, imaging.Lanczos), nil case FitInside: // Same as contain, but only shrink @@ -162,10 +168,10 @@ func (p *ImageProcessor) resize(img image.Image, width, height int, fit FitMode) origH := bounds.Dy() if origW <= width && origH <= height { - return img // Already fits + return img, nil // Already fits } - return imaging.Fit(img, width, height, imaging.Lanczos) + return imaging.Fit(img, width, height, imaging.Lanczos), nil case FitOutside: // Resize so smallest dimension fits, may exceed target on other dimension @@ -184,11 +190,10 @@ func (p *ImageProcessor) resize(img image.Image, width, height int, fit FitMode) newW := int(float64(origW) * scale) newH := int(float64(origH) * scale) - return imaging.Resize(img, newW, newH, imaging.Lanczos) + return imaging.Resize(img, newW, newH, imaging.Lanczos), nil default: - // Default to cover - return imaging.Fill(img, width, height, imaging.Center, imaging.Lanczos) + return nil, fmt.Errorf("%w: %s", ErrInvalidFitMode, fit) } } @@ -222,19 +227,10 @@ func (p *ImageProcessor) encode(img image.Image, format ImageFormat, quality int } case FormatWebP: - // Pure Go doesn't have WebP encoder, fall back to PNG - // TODO: Add WebP encoding support via external library - err := png.Encode(&buf, img) - if err != nil { - return nil, err - } + return nil, fmt.Errorf("%w: WebP encoding not supported", ErrUnsupportedOutputFormat) case FormatAVIF: - // AVIF not supported in pure Go, fall back to JPEG - err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: quality}) - if err != nil { - return nil, err - } + return nil, fmt.Errorf("%w: AVIF encoding not supported", ErrUnsupportedOutputFormat) default: return nil, fmt.Errorf("unsupported output format: %s", format)