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.
This commit is contained in:
@@ -54,6 +54,11 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
|
|||||||
|
|
||||||
if fit := query.Get("fit"); fit != "" {
|
if fit := query.Get("fit"); fit != "" {
|
||||||
req.FitMode = imgcache.FitMode(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
|
// Default quality if not set
|
||||||
|
|||||||
@@ -3,11 +3,15 @@ package imgcache
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"io"
|
"io"
|
||||||
"net/url"
|
"net/url"
|
||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// ErrInvalidFitMode is returned when an invalid fit mode is provided.
|
||||||
|
var ErrInvalidFitMode = errors.New("invalid fit mode")
|
||||||
|
|
||||||
// ImageFormat represents supported output image formats.
|
// ImageFormat represents supported output image formats.
|
||||||
type ImageFormat string
|
type ImageFormat string
|
||||||
|
|
||||||
@@ -44,6 +48,17 @@ const (
|
|||||||
FitOutside FitMode = "outside"
|
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
|
// ImageRequest represents a request for a processed image
|
||||||
type ImageRequest struct {
|
type ImageRequest struct {
|
||||||
// SourceHost is the origin host (e.g., "cdn.example.com")
|
// SourceHost is the origin host (e.g., "cdn.example.com")
|
||||||
|
|||||||
@@ -22,6 +22,9 @@ const MaxInputDimension = 8192
|
|||||||
// ErrInputTooLarge is returned when input image dimensions exceed MaxInputDimension.
|
// ErrInputTooLarge is returned when input image dimensions exceed MaxInputDimension.
|
||||||
var ErrInputTooLarge = errors.New("input image dimensions exceed maximum")
|
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.
|
// ImageProcessor implements the Processor interface using pure Go libraries.
|
||||||
type ImageProcessor struct{}
|
type ImageProcessor struct{}
|
||||||
|
|
||||||
@@ -70,7 +73,11 @@ func (p *ImageProcessor) Process(
|
|||||||
|
|
||||||
// Resize if needed
|
// Resize if needed
|
||||||
if targetWidth != origWidth || targetHeight != origHeight {
|
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
|
// Determine output format
|
||||||
@@ -115,7 +122,6 @@ func (p *ImageProcessor) SupportedOutputFormats() []ImageFormat {
|
|||||||
FormatJPEG,
|
FormatJPEG,
|
||||||
FormatPNG,
|
FormatPNG,
|
||||||
FormatGIF,
|
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.
|
// 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 {
|
switch fit {
|
||||||
case FitCover:
|
case FitCover, "":
|
||||||
// Resize and crop to fill exact dimensions
|
// Resize and crop to fill exact dimensions (default)
|
||||||
return imaging.Fill(img, width, height, imaging.Center, imaging.Lanczos)
|
return imaging.Fill(img, width, height, imaging.Center, imaging.Lanczos), nil
|
||||||
|
|
||||||
case FitContain:
|
case FitContain:
|
||||||
// Resize to fit within dimensions, maintaining aspect ratio
|
// 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:
|
case FitFill:
|
||||||
// Resize to exact dimensions (may distort)
|
// 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:
|
case FitInside:
|
||||||
// Same as contain, but only shrink
|
// 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()
|
origH := bounds.Dy()
|
||||||
|
|
||||||
if origW <= width && origH <= height {
|
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:
|
case FitOutside:
|
||||||
// Resize so smallest dimension fits, may exceed target on other dimension
|
// 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)
|
newW := int(float64(origW) * scale)
|
||||||
newH := int(float64(origH) * 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:
|
||||||
// Default to cover
|
return nil, fmt.Errorf("%w: %s", ErrInvalidFitMode, fit)
|
||||||
return imaging.Fill(img, width, height, imaging.Center, imaging.Lanczos)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -222,19 +227,10 @@ func (p *ImageProcessor) encode(img image.Image, format ImageFormat, quality int
|
|||||||
}
|
}
|
||||||
|
|
||||||
case FormatWebP:
|
case FormatWebP:
|
||||||
// Pure Go doesn't have WebP encoder, fall back to PNG
|
return nil, fmt.Errorf("%w: WebP encoding not supported", ErrUnsupportedOutputFormat)
|
||||||
// TODO: Add WebP encoding support via external library
|
|
||||||
err := png.Encode(&buf, img)
|
|
||||||
if err != nil {
|
|
||||||
return nil, err
|
|
||||||
}
|
|
||||||
|
|
||||||
case FormatAVIF:
|
case FormatAVIF:
|
||||||
// AVIF not supported in pure Go, fall back to JPEG
|
return nil, fmt.Errorf("%w: AVIF encoding not supported", ErrUnsupportedOutputFormat)
|
||||||
err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: quality})
|
|
||||||
if err != nil {
|
|
||||||
return nil, err
|
|
||||||
}
|
|
||||||
|
|
||||||
default:
|
default:
|
||||||
return nil, fmt.Errorf("unsupported output format: %s", format)
|
return nil, fmt.Errorf("unsupported output format: %s", format)
|
||||||
|
|||||||
Reference in New Issue
Block a user