diff --git a/internal/imgcache/processor.go b/internal/imgcache/processor.go index 846983e..16e9de0 100644 --- a/internal/imgcache/processor.go +++ b/internal/imgcache/processor.go @@ -26,20 +26,36 @@ func initVips() { // Images larger than this are rejected to prevent DoS via decompression bombs. const MaxInputDimension = 8192 +// DefaultMaxInputBytes is the default maximum input size in bytes (50 MiB). +// This matches the default upstream fetcher limit. +const DefaultMaxInputBytes = 50 << 20 + // ErrInputTooLarge is returned when input image dimensions exceed MaxInputDimension. var ErrInputTooLarge = errors.New("input image dimensions exceed maximum") +// ErrInputDataTooLarge is returned when the raw input data exceeds the configured byte limit. +var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size") + // ErrUnsupportedOutputFormat is returned when the requested output format is not supported. var ErrUnsupportedOutputFormat = errors.New("unsupported output format") // ImageProcessor implements the Processor interface using libvips via govips. -type ImageProcessor struct{} +type ImageProcessor struct { + maxInputBytes int64 +} -// NewImageProcessor creates a new image processor. -func NewImageProcessor() *ImageProcessor { +// NewImageProcessor creates a new image processor with the given maximum input +// size in bytes. If maxInputBytes is <= 0, DefaultMaxInputBytes is used. +func NewImageProcessor(maxInputBytes int64) *ImageProcessor { initVips() - return &ImageProcessor{} + if maxInputBytes <= 0 { + maxInputBytes = DefaultMaxInputBytes + } + + return &ImageProcessor{ + maxInputBytes: maxInputBytes, + } } // Process transforms an image according to the request. @@ -48,12 +64,20 @@ func (p *ImageProcessor) Process( input io.Reader, req *ImageRequest, ) (*ProcessResult, error) { - // Read input - data, err := io.ReadAll(input) + // Read input with a size limit to prevent unbounded memory consumption. + // We read at most maxInputBytes+1 so we can detect if the input exceeds + // the limit without consuming additional memory. + limited := io.LimitReader(input, p.maxInputBytes+1) + + data, err := io.ReadAll(limited) if err != nil { return nil, fmt.Errorf("failed to read input: %w", err) } + if int64(len(data)) > p.maxInputBytes { + return nil, ErrInputDataTooLarge + } + // Decode image img, err := vips.NewImageFromBuffer(data) if err != nil { diff --git a/internal/imgcache/processor_test.go b/internal/imgcache/processor_test.go index 374826d..d27bd74 100644 --- a/internal/imgcache/processor_test.go +++ b/internal/imgcache/processor_test.go @@ -71,7 +71,7 @@ func createTestPNG(t *testing.T, width, height int) []byte { } func TestImageProcessor_ResizeJPEG(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() input := createTestJPEG(t, 800, 600) @@ -118,7 +118,7 @@ func TestImageProcessor_ResizeJPEG(t *testing.T) { } func TestImageProcessor_ConvertToPNG(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() input := createTestJPEG(t, 200, 150) @@ -151,7 +151,7 @@ func TestImageProcessor_ConvertToPNG(t *testing.T) { } func TestImageProcessor_OriginalSize(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() input := createTestJPEG(t, 640, 480) @@ -179,7 +179,7 @@ func TestImageProcessor_OriginalSize(t *testing.T) { } func TestImageProcessor_FitContain(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() // 800x400 image (2:1 aspect) into 400x400 box with contain @@ -206,7 +206,7 @@ func TestImageProcessor_FitContain(t *testing.T) { } func TestImageProcessor_ProportionalScale_WidthOnly(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() // 800x600 image, request width=400 height=0 @@ -236,7 +236,7 @@ func TestImageProcessor_ProportionalScale_WidthOnly(t *testing.T) { } func TestImageProcessor_ProportionalScale_HeightOnly(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() // 800x600 image, request width=0 height=300 @@ -266,7 +266,7 @@ func TestImageProcessor_ProportionalScale_HeightOnly(t *testing.T) { } func TestImageProcessor_ProcessPNG(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() input := createTestPNG(t, 400, 300) @@ -298,7 +298,7 @@ func TestImageProcessor_ImplementsInterface(t *testing.T) { } func TestImageProcessor_SupportedFormats(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) inputFormats := proc.SupportedInputFormats() if len(inputFormats) == 0 { @@ -312,7 +312,7 @@ func TestImageProcessor_SupportedFormats(t *testing.T) { } func TestImageProcessor_RejectsOversizedInput(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() // Create an image that exceeds MaxInputDimension (e.g., 10000x100) @@ -337,7 +337,7 @@ func TestImageProcessor_RejectsOversizedInput(t *testing.T) { } func TestImageProcessor_RejectsOversizedInputHeight(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() // Create an image with oversized height @@ -361,7 +361,7 @@ func TestImageProcessor_RejectsOversizedInputHeight(t *testing.T) { } func TestImageProcessor_AcceptsMaxDimensionInput(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() // Create an image at exactly MaxInputDimension - should be accepted @@ -383,7 +383,7 @@ func TestImageProcessor_AcceptsMaxDimensionInput(t *testing.T) { } func TestImageProcessor_EncodeWebP(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() input := createTestJPEG(t, 200, 150) @@ -426,7 +426,7 @@ func TestImageProcessor_EncodeWebP(t *testing.T) { } func TestImageProcessor_DecodeAVIF(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() // Load test AVIF file @@ -465,8 +465,73 @@ func TestImageProcessor_DecodeAVIF(t *testing.T) { } } +func TestImageProcessor_RejectsOversizedInputData(t *testing.T) { + // Create a processor with a very small byte limit + const limit = 1024 + proc := NewImageProcessor(limit) + ctx := context.Background() + + // Create a valid JPEG that exceeds the byte limit + input := createTestJPEG(t, 800, 600) // will be well over 1 KiB + if int64(len(input)) <= limit { + t.Fatalf("test JPEG must exceed %d bytes, got %d", limit, len(input)) + } + + req := &ImageRequest{ + Size: Size{Width: 100, Height: 75}, + Format: FormatJPEG, + Quality: 85, + FitMode: FitCover, + } + + _, err := proc.Process(ctx, bytes.NewReader(input), req) + if err == nil { + t.Fatal("Process() should reject input exceeding maxInputBytes") + } + + if err != ErrInputDataTooLarge { + t.Errorf("Process() error = %v, want ErrInputDataTooLarge", err) + } +} + +func TestImageProcessor_AcceptsInputWithinLimit(t *testing.T) { + // Create a small image and set limit well above its size + input := createTestJPEG(t, 10, 10) + limit := int64(len(input)) * 10 // 10× headroom + + proc := NewImageProcessor(limit) + ctx := context.Background() + + req := &ImageRequest{ + Size: Size{Width: 10, Height: 10}, + Format: FormatJPEG, + Quality: 85, + FitMode: FitCover, + } + + result, err := proc.Process(ctx, bytes.NewReader(input), req) + if err != nil { + t.Fatalf("Process() error = %v, want nil", err) + } + defer result.Content.Close() +} + +func TestImageProcessor_DefaultMaxInputBytes(t *testing.T) { + // Passing 0 should use the default + proc := NewImageProcessor(0) + if proc.maxInputBytes != DefaultMaxInputBytes { + t.Errorf("maxInputBytes = %d, want %d", proc.maxInputBytes, DefaultMaxInputBytes) + } + + // Passing negative should also use the default + proc = NewImageProcessor(-1) + if proc.maxInputBytes != DefaultMaxInputBytes { + t.Errorf("maxInputBytes = %d, want %d", proc.maxInputBytes, DefaultMaxInputBytes) + } +} + func TestImageProcessor_EncodeAVIF(t *testing.T) { - proc := NewImageProcessor() + proc := NewImageProcessor(0) ctx := context.Background() input := createTestJPEG(t, 200, 150) diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index 53b99d2..2000f6c 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -15,13 +15,14 @@ import ( // Service implements the ImageCache interface, orchestrating cache, fetcher, and processor. type Service struct { - cache *Cache - fetcher Fetcher - processor Processor - signer *Signer - whitelist *HostWhitelist - log *slog.Logger - allowHTTP bool + cache *Cache + fetcher Fetcher + processor Processor + signer *Signer + whitelist *HostWhitelist + log *slog.Logger + allowHTTP bool + maxResponseSize int64 } // ServiceConfig holds configuration for the image service. @@ -50,15 +51,17 @@ func NewService(cfg *ServiceConfig) (*Service, error) { return nil, errors.New("signing key is required") } + // Resolve fetcher config for defaults + fetcherCfg := cfg.FetcherConfig + if fetcherCfg == nil { + fetcherCfg = DefaultFetcherConfig() + } + // Use custom fetcher if provided, otherwise create HTTP fetcher var fetcher Fetcher if cfg.Fetcher != nil { fetcher = cfg.Fetcher } else { - fetcherCfg := cfg.FetcherConfig - if fetcherCfg == nil { - fetcherCfg = DefaultFetcherConfig() - } fetcher = NewHTTPFetcher(fetcherCfg) } @@ -74,14 +77,17 @@ func NewService(cfg *ServiceConfig) (*Service, error) { allowHTTP = cfg.FetcherConfig.AllowHTTP } + maxResponseSize := fetcherCfg.MaxResponseSize + return &Service{ - cache: cfg.Cache, - fetcher: fetcher, - processor: NewImageProcessor(), - signer: signer, - whitelist: NewHostWhitelist(cfg.Whitelist), - log: log, - allowHTTP: allowHTTP, + cache: cfg.Cache, + fetcher: fetcher, + processor: NewImageProcessor(maxResponseSize), + signer: signer, + whitelist: NewHostWhitelist(cfg.Whitelist), + log: log, + allowHTTP: allowHTTP, + maxResponseSize: maxResponseSize, }, nil } @@ -146,6 +152,40 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e return response, nil } +// loadCachedSource attempts to load source content from cache, returning nil +// if the cached data is unavailable or exceeds maxResponseSize. +func (s *Service) loadCachedSource(contentHash ContentHash) []byte { + reader, err := s.cache.GetSourceContent(contentHash) + if err != nil { + s.log.Warn("failed to load cached source, fetching", "error", err) + + return nil + } + + // Bound the read to maxResponseSize to prevent unbounded memory use + // from unexpectedly large cached files. + limited := io.LimitReader(reader, s.maxResponseSize+1) + data, err := io.ReadAll(limited) + _ = reader.Close() + + if err != nil { + s.log.Warn("failed to read cached source, fetching", "error", err) + + return nil + } + + if int64(len(data)) > s.maxResponseSize { + s.log.Warn("cached source exceeds max response size, discarding", + "hash", contentHash, + "max_bytes", s.maxResponseSize, + ) + + return nil + } + + return data +} + // processFromSourceOrFetch processes an image, using cached source content if available. func (s *Service) processFromSourceOrFetch( ctx context.Context, @@ -162,22 +202,8 @@ func (s *Service) processFromSourceOrFetch( var fetchBytes int64 if contentHash != "" { - // We have cached source - load it s.log.Debug("using cached source", "hash", contentHash) - - reader, err := s.cache.GetSourceContent(contentHash) - if err != nil { - s.log.Warn("failed to load cached source, fetching", "error", err) - // Fall through to fetch - } else { - sourceData, err = io.ReadAll(reader) - _ = reader.Close() - - if err != nil { - s.log.Warn("failed to read cached source, fetching", "error", err) - // Fall through to fetch - } - } + sourceData = s.loadCachedSource(contentHash) } // Fetch from upstream if we don't have source data or it's empty