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)