From 8b4dcda549ad4945d8d0e506d49816d05527eb76 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 16 Oct 2026 04:17:59 +0200 Subject: [PATCH] Keep a side computed for a size with one side 0 at 1 or more (closes #283) A size with one side 0 has that side worked out from the source's proportions by integer division, and nothing kept it at 1 or more. When it rounded down to 0, libvips was asked for a square: an 8192x1 source at 100x0 came out 100x100, at 1300x0 it asked for one block of about 39 GB, and some shapes answered 500. Each computed side, and each side contain, inside and outside work out again from the scale, is now at least 1. README.md now says what fit=outside does and what a 0 side means; with fit=outside an 8192x1 source at 100x0 comes out 8192x1, as covering the size requires. Model: opus-5-5 --- README.md | 14 +++- TODO.md | 14 ++++ .../computed_side_internal_test.go | 66 +++++++++++++++++++ .../fit_outside_internal_test.go | 42 ++++++++++++ internal/imageprocessor/imageprocessor.go | 32 +++++---- internal/imgcache/urlparser.go | 4 +- 6 files changed, 156 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 3ab10a7..d9b3c86 100644 --- a/README.md +++ b/README.md @@ -536,8 +536,13 @@ Where: - `query` — source query string as under Routes, empty string if none, with `%` and `:` written as in `path`; neither then holds a `:`, so path `/a:b` with no query and path `/a` with query `b:` are signed differently -- `width` — requested width in pixels, `0` for original -- `height` — requested height in pixels, `0` for original +- `width` — requested width in pixels, or `0`: the width is then worked out from + the source's proportions and the requested height, rounded down but at least + 1, or is the source's own width when the height is `0` too (the `orig` size) +- `height` — requested height in pixels, or `0`: the height is then worked out + from the source's proportions and the requested width, rounded down but at + least 1, or is the source's own height when the width is `0` too (the `orig` + size) - `format` — output format, one of those listed under Routes, with `original` signed as `orig`, `jpg` as `jpeg`, and no format as `jxl`, so a URL with no format has the signature of the same URL ending in `.jxl`; `auto` is signed as @@ -550,7 +555,10 @@ Where: with 400 - `fit` — the URL's `fit` query parameter (cover, contain, fill, inside, outside), or `cover` when the URL has no `fit`; a request whose `fit` is - anything else, an empty `fit=` included, is refused with 400 + anything else, an empty `fit=` included, is refused with 400. `outside` scales + the image to cover the requested size, keeping its proportions, so it can come + out larger than that size on one side: an 8192x1 source at `100x0`, which is + 100x1 by its proportions, comes out at 8192x1 The URL's `sig` is the HMAC-SHA256 result in base64url (the URL-safe alphabet of RFC 4648) with the trailing `=` padding kept, 44 characters in all. pixa diff --git a/TODO.md b/TODO.md index 3817c02..a00b1ef 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,20 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-16 a size with one side `0` keeps the side computed from the source's + proportions at 1 or more (closes #283): it was rounded down, so for a thin + source it came out at 0, and libvips then made the size square. An 8192x1 + source at `100x0` came out at 100x100, and at `1300x0` with `fit=cover` made + libvips ask for about 39 GB at once; an 8192x2 source at `100x0` answered 500. + `fit=contain` and `fit=inside` work both sides out again from the smaller of + the two scales and `fit=outside` from the larger, and each keeps both sides at + 1 or more too, so a 1x8192 source at `0x100` with `fit=contain`, and a 49x8192 + source at `0x100` or a 49x49 source at `1x1` with `fit=outside`, no longer + answer 500. An 8192x1 source at `1300x0` now comes out at 1300x1, and the + process running it alone peaked at about 34 MiB resident. With `fit=outside` + the image is scaled to cover the size, so that source at `100x0` comes out at + 8192x1. `README.md` now says this under Signature Specification, and says how + a `0` side is worked out where it said `0` meant the original. - 2026-10-16 the generator page lower-cases the source host and writes an empty source path as `/` before it makes the encrypted URL, as `/v1/image/` does (closes #293). A host written in upper and in lower case, or a source URL with diff --git a/internal/imageprocessor/computed_side_internal_test.go b/internal/imageprocessor/computed_side_internal_test.go index f5c3d5a..231087d 100644 --- a/internal/imageprocessor/computed_side_internal_test.go +++ b/internal/imageprocessor/computed_side_internal_test.go @@ -50,3 +50,69 @@ func TestImageProcessor_RefusesComputedSideAboveMaximum(t *testing.T) { }) } } + +// TestImageProcessor_ComputedSideIsAtLeastOne verifies that, for a size with +// one side 0, the side computed from the input's proportions is 1 where it +// would round down to 0, and that Process makes that size in every fit mode +// but FitOutside, which scales the input to cover the size and so can make +// it longer. Each case checks the size targetDimensions gives before it +// calls Process: with that side at 0, an 8192x1 input at 1300x0 makes +// libvips ask for about 39 GB with FitCover. +func TestImageProcessor_ComputedSideIsAtLeastOne(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + inputWidth, inputHeight int + size Size + wantWidth, wantHeight int + }{ + {"8192x1 at 100x0", 8192, 1, Size{Width: 100}, 100, 1}, + {"8192x1 at 1300x0", 8192, 1, Size{Width: 1300}, 1300, 1}, + {"8192x2 at 100x0", 8192, 2, Size{Width: 100}, 100, 1}, + {"1x8192 at 0x100", 1, 8192, Size{Height: 100}, 1, 100}, + {"1x8192 at 0x1300", 1, 8192, Size{Height: 1300}, 1, 1300}, + {"2x8192 at 0x100", 2, 8192, Size{Height: 100}, 1, 100}, + } + + fits := []FitMode{FitCover, FitContain, FitFill, FitInside, FitOutside} + + for _, tt := range tests { + for _, fit := range fits { + t.Run(tt.name+" "+string(fit), func(t *testing.T) { + t.Parallel() + + width, height, err := targetDimensions(tt.size, + tt.inputWidth, tt.inputHeight) + if err != nil || width != tt.wantWidth || height != tt.wantHeight { + t.Fatalf("targetDimensions() = %dx%d, %v, want %dx%d", + width, height, err, tt.wantWidth, tt.wantHeight) + } + + input := createTestJPEG(t, tt.inputWidth, tt.inputHeight) + + result, err := New(Params{}).Process(t.Context(), + bytes.NewReader(input), + &Request{Size: tt.size, Format: FormatJPEG, FitMode: fit}) + if err != nil { + t.Fatalf("Process() error = %v", err) + } + + defer func() { _ = result.Content.Close() }() + + t.Logf("Process() made %dx%d", result.Width, result.Height) + + ok := result.Width == tt.wantWidth && result.Height == tt.wantHeight + if fit == FitOutside { + ok = result.Width >= tt.wantWidth && result.Height >= tt.wantHeight + } + + if !ok { + t.Errorf("Process() made %dx%d, want %dx%d (at least, for %s)", + result.Width, result.Height, tt.wantWidth, tt.wantHeight, + FitOutside) + } + }) + } + } +} diff --git a/internal/imageprocessor/fit_outside_internal_test.go b/internal/imageprocessor/fit_outside_internal_test.go index ab3737f..62d9674 100644 --- a/internal/imageprocessor/fit_outside_internal_test.go +++ b/internal/imageprocessor/fit_outside_internal_test.go @@ -60,3 +60,45 @@ func TestImageProcessor_RefusesFitOutsideAboveMaximum(t *testing.T) { }) } } + +// TestImageProcessor_FitOutsideSideIsAtLeastOne verifies that FitOutside +// keeps each side at 1 or more when it works both sides out again from the +// larger scale. A side of 49 scaled to 1 is 49 times 1/49, which in floating +// point is just under 1 and so rounded down to 0, a width libvips refuses. +func TestImageProcessor_FitOutsideSideIsAtLeastOne(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + inputWidth, inputHeight int + size Size + wantWidth, wantHeight int + }{ + {"49x8192 at 0x100", 49, 8192, Size{Height: 100}, 1, 167}, + {"49x49 at 1x1", 49, 49, Size{Width: 1, Height: 1}, 1, 1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + input := createTestJPEG(t, tt.inputWidth, tt.inputHeight) + + result, err := New(Params{}).Process(t.Context(), + bytes.NewReader(input), + &Request{Size: tt.size, Format: FormatJPEG, FitMode: FitOutside}) + if err != nil { + t.Fatalf("Process() error = %v", err) + } + + defer func() { _ = result.Content.Close() }() + + t.Logf("Process() made %dx%d", result.Width, result.Height) + + if result.Width != tt.wantWidth || result.Height != tt.wantHeight { + t.Errorf("Process() made %dx%d, want %dx%d", + result.Width, result.Height, tt.wantWidth, tt.wantHeight) + } + }) + } +} diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index 716b9ad..7b83a41 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -311,8 +311,10 @@ func (p *ImageProcessor) Process( // targetDimensions calculates the output dimensions for a requested size, // scaling proportionally when only one dimension is given and keeping the -// original dimensions when both are zero. A side scaled proportionally can -// come out above MaxOutputDimension, which is ErrOutputTooLarge. +// original dimensions when both are zero. A side scaled proportionally is +// rounded down but kept at 1 or more, as for a thin image it would round +// down to 0, a side of no pixels. It can come out above MaxOutputDimension, +// which is ErrOutputTooLarge. func targetDimensions(size Size, origWidth, origHeight int) (int, int, error) { width, height := size.Width, size.Height @@ -322,10 +324,10 @@ func targetDimensions(size Size, origWidth, origHeight int) (int, int, error) { return origWidth, origHeight, nil case width == 0: // Only height specified: calculate width proportionally - width = origWidth * height / origHeight + width = max(1, origWidth*height/origHeight) case height == 0: // Only width specified: calculate height proportionally - height = origHeight * width / origWidth + height = max(1, origHeight*width/origWidth) } if width > MaxOutputDimension || height > MaxOutputDimension { @@ -479,13 +481,16 @@ func (p *ImageProcessor) resize( return cropCentreAndResize(img, width, height) case FitContain: - // Resize to fit within dimensions, maintaining aspect ratio + // Resize to fit within dimensions, maintaining aspect ratio. Scaled + // by the smaller scale, the short side of a thin image can round + // down to 0, which libvips refuses as a width, so each side is at + // least 1. imgW, imgH := img.Width(), img.Height() scaleW := float64(width) / float64(imgW) scaleH := float64(height) / float64(imgH) scale := min(scaleW, scaleH) - newW := int(float64(imgW) * scale) - newH := int(float64(imgH) * scale) + newW := max(1, int(float64(imgW)*scale)) + newH := max(1, int(float64(imgH)*scale)) return img.Thumbnail(newW, newH, vips.InterestingNone) @@ -503,19 +508,22 @@ func (p *ImageProcessor) resize( scaleW := float64(width) / float64(imgW) scaleH := float64(height) / float64(imgH) scale := min(scaleW, scaleH) - newW := int(float64(imgW) * scale) - newH := int(float64(imgH) * scale) + newW := max(1, int(float64(imgW)*scale)) + newH := max(1, int(float64(imgH)*scale)) return img.Thumbnail(newW, newH, vips.InterestingNone) case FitOutside: - // Resize so smallest dimension fits, may exceed target on other dimension + // Resize so smallest dimension fits, may exceed target on other + // dimension. A side scaled to 1 can come out just under 1 in + // floating point, as 49 times 1/49 does, and round down to 0, which + // libvips refuses as a width, so each side is at least 1. imgW, imgH := img.Width(), img.Height() scaleW := float64(width) / float64(imgW) scaleH := float64(height) / float64(imgH) scale := max(scaleW, scaleH) - newW := int(float64(imgW) * scale) - newH := int(float64(imgH) * scale) + newW := max(1, int(float64(imgW)*scale)) + newH := max(1, int(float64(imgH)*scale)) // The dimension that exceeds the target is held to the maximum too if newW > MaxOutputDimension || newH > MaxOutputDimension { diff --git a/internal/imgcache/urlparser.go b/internal/imgcache/urlparser.go index 6c58c20..46bba16 100644 --- a/internal/imgcache/urlparser.go +++ b/internal/imgcache/urlparser.go @@ -29,7 +29,9 @@ var ( const MaxDimension = imageprocessor.MaxOutputDimension // ValidateDimension checks one requested width or height; name ("width" or -// "height") appears in the error. 0 means "original size" and is valid. +// "height") appears in the error. 0 is valid: that side is then worked out from +// the source's proportions and the other requested side, rounded down but at +// least 1, or is the source's own side when the other side is 0 too. func ValidateDimension(name string, value int) error { if value < 0 { return fmt.Errorf("%w: %s is negative", ErrInvalidSize, name)