From 472d3354bd72d550c60f3623fdcde3109e3896ab Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 00:38:00 +0000 Subject: [PATCH 1/6] Test that a side computed for a size with one side 0 is at least 1 An 8192x1 source at 100x0 and 1300x0 and an 8192x2 source at 100x0, and the same turned on their side, in every fit mode. The test fails until the fix for https://git.eeqj.de/sneak/pixa/issues/283 lands; TODO.md records the bug at the top as P1 until then. Model: opus-5-5 --- TODO.md | 6 +- .../computed_side_internal_test.go | 66 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/TODO.md b/TODO.md index 3817c02..ce15d30 100644 --- a/TODO.md +++ b/TODO.md @@ -26,7 +26,10 @@ resolved. The disk cache is now size-bounded with LRU eviction # Next Step -P2: security: per-origin rate limiting +P1: security: a size with one side `0` whose side computed from the source's +proportions rounds down to 0 comes out square, can make libvips ask for about 39 +GB (an 8192x1 source at `1300x0`), or answers 500 +(https://git.eeqj.de/sneak/pixa/issues/283) # Completed Steps @@ -1015,6 +1018,7 @@ P2: security: per-origin rate limiting # Future Steps +- P2: security: per-origin rate limiting - P2: HTTP response handling - Last-Modified headers - P2: configuration 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) + } + }) + } + } +} -- 2.54.0 From a7bf8d040dccbb8e4e6d6b3684fc0bf03d81b54f Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 00:39:59 +0000 Subject: [PATCH 2/6] Keep a side computed for a size with one side 0 at 1 or more (closes #283) The side was rounded down, so for a thin source it came out at 0, and libvips made the size square: an 8192x1 source at 100x0 came out at 100x100, at 1300x0 with fit=cover libvips asked for about 39 GB, and an 8192x2 source at 100x0 answered 500. fit=contain and fit=inside work both sides out again from the smaller scale and keep each at 1 or more as well, as a 1x8192 source at 0x100 answered 500 there. Model: opus-5-5 --- TODO.md | 17 ++++++++++++----- internal/imageprocessor/imageprocessor.go | 23 ++++++++++++++--------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/TODO.md b/TODO.md index ce15d30..f512c60 100644 --- a/TODO.md +++ b/TODO.md @@ -26,10 +26,7 @@ resolved. The disk cache is now size-bounded with LRU eviction # Next Step -P1: security: a size with one side `0` whose side computed from the source's -proportions rounds down to 0 comes out square, can make libvips ask for about 39 -GB (an 8192x1 source at `1300x0`), or answers 500 -(https://git.eeqj.de/sneak/pixa/issues/283) +P2: security: per-origin rate limiting # Completed Steps @@ -51,6 +48,17 @@ GB (an 8192x1 source at `1300x0`), or answers 500 another way, such as with `%2F` in the path, whatever else the path holds, or with a user name and password. It also answers 400 for a source path holding a `..` segment, as `/v1/image/` refuses one. +- 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 keep each at 1 or more too, so a 1x8192 source at `0x100` + no longer answers 500 there. 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. - 2026-10-15 a thin source requested with `fit=cover` no longer makes libvips build a huge image (closes #282): libvips' thumbnail scaled the whole source to cover the requested size and copied that into memory before it cut out the @@ -1018,7 +1026,6 @@ GB (an 8192x1 source at `1300x0`), or answers 500 # Future Steps -- P2: security: per-origin rate limiting - P2: HTTP response handling - Last-Modified headers - P2: configuration diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index 716b9ad..0c315ea 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,8 +508,8 @@ 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) -- 2.54.0 From 52c77ec4ba5c82cfdcdeec1d66f2bcbbce995475 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 01:10:24 +0000 Subject: [PATCH 3/6] Test that fit=outside keeps each side at 1 or more A 49x8192 source at 0x100 and a 49x49 source at 1x1, both with fit=outside. 49 times 1/49 is just under 1 in floating point, so the width is rounded down to 0 and libvips refuses it. The test fails until the fix for https://git.eeqj.de/sneak/pixa/issues/283 covers fit=outside. Model: opus-5-5 --- .../fit_outside_internal_test.go | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) 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) + } + }) + } +} -- 2.54.0 From 778d3b24bd99e596cb621630faa82b8ce6513206 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 01:11:20 +0000 Subject: [PATCH 4/6] Keep each side at 1 or more with fit=outside too fit=outside works both sides out again from the larger scale, and a side scaled to 1 could come out just under 1 and round down to 0, which libvips refuses: a 49x8192 source at 0x100 and a 49x49 source at 1x1 answered 500. README.md now says what fit=outside does, with the 8192x1 source at 100x0 that comes out at 8192x1, and how a 0 side is worked out from the source's proportions. Model: opus-5-5 --- README.md | 14 ++++++++++--- TODO.md | 25 +++++++++++++---------- internal/imageprocessor/imageprocessor.go | 9 +++++--- 3 files changed, 31 insertions(+), 17 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 f512c60..48b3f72 100644 --- a/TODO.md +++ b/TODO.md @@ -35,6 +35,20 @@ P2: security: per-origin rate limiting (closes #293). A host written in upper and in lower case, or a source URL with no path and with the path `/`, made two cache keys and two upstream requests for one image. +- 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 source URL a request fetches is built with `net/url` from the source host, path and query, not joined as text (closes #288). The source path, decoded, is fetched percent-encoded again, so `100%25.png` fetches @@ -48,17 +62,6 @@ P2: security: per-origin rate limiting another way, such as with `%2F` in the path, whatever else the path holds, or with a user name and password. It also answers 400 for a source path holding a `..` segment, as `/v1/image/` refuses one. -- 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 keep each at 1 or more too, so a 1x8192 source at `0x100` - no longer answers 500 there. 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. - 2026-10-15 a thin source requested with `fit=cover` no longer makes libvips build a huge image (closes #282): libvips' thumbnail scaled the whole source to cover the requested size and copied that into memory before it cut out the diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index 0c315ea..7b83a41 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -514,13 +514,16 @@ func (p *ImageProcessor) resize( 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 { -- 2.54.0 From 0eaad0a3f9033540cb5e3d0de8c8f646fe520e5b Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 01:59:24 +0000 Subject: [PATCH 5/6] Say what a 0 side does in the ValidateDimension doc comment It said 0 meant "original size"; the side is worked out from the source's proportions, as README.md now says. Model: opus-5-5 --- internal/imgcache/urlparser.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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) -- 2.54.0 From b56e6c3d8e7450569d92215efa1abf380ef78cf6 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 02:06:22 +0000 Subject: [PATCH 6/6] Put this change's TODO.md entry back at the top of Completed Steps The union merge of TODO.md left it below the entry next gained. Model: opus-5-5 --- TODO.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/TODO.md b/TODO.md index 48b3f72..a00b1ef 100644 --- a/TODO.md +++ b/TODO.md @@ -30,11 +30,6 @@ P2: security: per-origin rate limiting # Completed Steps -- 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 - no path and with the path `/`, made two cache keys and two upstream requests - for one image. - 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 @@ -49,6 +44,11 @@ P2: security: per-origin rate limiting 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 + no path and with the path `/`, made two cache keys and two upstream requests + for one image. - 2026-10-16 the source URL a request fetches is built with `net/url` from the source host, path and query, not joined as text (closes #288). The source path, decoded, is fetched percent-encoded again, so `100%25.png` fetches -- 2.54.0