4 Commits
Author SHA1 Message Date
sneak 5506858834 Test the generator refusals and a "'" in the source query
check / check (push) Canceled after 0s
Through the router: a source query holding "'" must reach the upstream
as %27. The generator page must answer 400, show the URL pixa would
fetch and make no encrypted URL for a source path holding %2F next to a
space, a "|" or a letter outside ASCII, and for a source URL holding a
user name and password. It must also refuse a source path holding a ".."
segment, as /v1/image/ does. These fail on the branch; the fix follows.

Model: opus-5-5
2026-10-16 00:56:05 +00:00
sneak 545507e9d4 Fetch the source path and query URL-encoded, not joined as text (closes #288)
The source path, decoded, is fetched percent-encoded again by net/url,
so 100%25.png fetches /100%25.png, and my%20cat.png and my%2520cat.png
fetch different paths. The source query stays as the source URL writes
it; the characters a browser percent-encodes in a query, such as a
space, are percent-encoded when the image URL is read, so the query is
signed and cached as it is sent. The generator page answers 400 for a
source URL its image URL would fetch written another way, such as with
%2F in the path, and shows the URL pixa would fetch. README.md says how
the source path and query are fetched.

Model: opus-5-5
2026-10-16 00:47:13 +00:00
sneak af5ce499b9 Test that the source URL is fetched with its path and query encoded
Through the router, against a test upstream that records each request
it receives: a space in the source query, 100%25.png, the source paths
written my%20cat.png and my%2520cat.png, one source query written two
ways, and the generator page with %20, %25 and %23 in the source URL,
and with %2F in its path, which the page must refuse. These fail on
next; the fix for #288 follows.

Model: opus-5-5
2026-10-16 00:47:13 +00:00
clawbot 9a96740463 Crop the centre before resizing for fit=cover (closes #282)
check / check (push) Canceled after 0s
For fit=cover, the default fit, libvips' thumbnail scaled the whole
source to cover the requested size and copied that into memory before
cutting out the centre, so an 8192x64 source at 8192x8192 asked for
about 24 GiB, and 8192x1 failed with 500. resize now first cuts the
centre of the source, widened to whole source pixels, and gives only
that to thumbnail, which scales both sides by the same amount as before;
the image it holds exceeds the output by less than one scaled source
pixel on each side. Output size and scale are unchanged; its position
can move by up to one output pixel.

Model: opus-5-5
2026-10-16 02:23:18 +02:00
4 changed files with 377 additions and 28 deletions
+12
View File
@@ -30,6 +30,18 @@ P2: security: per-origin rate limiting
# Completed Steps
- 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
centre, so an 8192x64 source at 8192x8192 made it allocate about 24 GiB, and
an 8192x1 source answered 500. The image processor now first cuts the centre
of the source, widened to whole source pixels, and thumbnail scales and cuts
only that, by the same amount on both sides as before. What it copies into
memory is then larger than the output by less than one scaled source pixel on
each side. With the runtime image's libvips and JPEG output at 8192x8192,
resident memory peaked at about 242 MiB for the 8192x64 source and 431 MiB for
the 8192x1 one. The output can sit a fraction of an output pixel from where it
was.
- 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
@@ -0,0 +1,268 @@
package imageprocessor
import (
"bytes"
"image"
"image/color"
"image/png"
"math"
"testing"
)
// TestImageProcessor_FitCoverServesThinSource verifies that FitCover serves
// a thin source at the requested size. An 8192x1 source scaled to cover
// 8192x8192 is 8192 times as long as the output on its long side. The centre
// of an 8192x1 source with the proportions of 1x8192 is 1/8192 of a pixel
// wide. Each case is also turned on its side.
func TestImageProcessor_FitCoverServesThinSource(t *testing.T) {
t.Parallel()
tests := []struct {
name string
inputWidth, inputHeight int
size Size
}{
{"8192x1 at 8192x8192", 8192, 1, Size{Width: 8192, Height: 8192}},
{"8192x1 at 1x8192", 8192, 1, Size{Width: 1, Height: 8192}},
{"1x8192 at 8192x8192", 1, 8192, Size{Width: 8192, Height: 8192}},
{"1x8192 at 8192x1", 1, 8192, Size{Width: 8192, Height: 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: FitCover})
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.size.Width || result.Height != tt.size.Height {
t.Errorf("Process() made %dx%d, want %dx%d",
result.Width, result.Height, tt.size.Width, tt.size.Height)
}
})
}
}
// createBandsPNG creates a PNG of three equal bands, red, green and blue,
// side by side when it is wider than tall and top to bottom otherwise.
func createBandsPNG(t *testing.T, width, height int) []byte {
t.Helper()
bands := []color.RGBA{
{R: 255, A: 255},
{G: 255, A: 255},
{B: 255, A: 255},
}
img := image.NewRGBA(image.Rect(0, 0, width, height))
for y := range height {
for x := range width {
band := 3 * y / height
if width > height {
band = 3 * x / width
}
img.Set(x, y, bands[band])
}
}
return encodePNG(t, img)
}
// TestImageProcessor_FitCoverCropsTheCentre verifies that FitCover keeps the
// centre of the source: a 300x100 source of red, green and blue bands side
// by side, or a 100x300 one with them top to bottom, comes out at 50x50 as
// its green band.
func TestImageProcessor_FitCoverCropsTheCentre(t *testing.T) {
t.Parallel()
tests := []struct {
name string
inputWidth, inputHeight int
}{
{"bands side by side", 300, 100},
{"bands top to bottom", 100, 300},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
input := createBandsPNG(t, tt.inputWidth, tt.inputHeight)
result, err := New(Params{}).Process(t.Context(),
bytes.NewReader(input),
&Request{
Size: Size{Width: 50, Height: 50},
Format: FormatPNG,
FitMode: FitCover,
})
if err != nil {
t.Fatalf("Process() error = %v", err)
}
defer func() { _ = result.Content.Close() }()
output, err := png.Decode(result.Content)
if err != nil {
t.Fatalf("failed to decode output: %v", err)
}
bounds := output.Bounds()
t.Logf("Process() made %dx%d", bounds.Dx(), bounds.Dy())
if bounds.Dx() != 50 || bounds.Dy() != 50 {
t.Fatalf("Process() made %dx%d, want 50x50",
bounds.Dx(), bounds.Dy())
}
// Resampling may blend a little of the bands beside the
// green one into the output's edges.
for y := bounds.Min.Y; y < bounds.Max.Y; y++ {
for x := bounds.Min.X; x < bounds.Max.X; x++ {
r, g, b, _ := output.At(x, y).RGBA()
if r>>8 > 64 || g>>8 < 192 || b>>8 > 64 {
t.Fatalf("output pixel %d,%d is %d,%d,%d, want green",
x, y, r>>8, g>>8, b>>8)
}
}
}
})
}
}
// The greys of createSquarePNG, and the grey halfway between them.
const (
backgroundGrey = 192
squareGrey = 64
halfwayGrey = (backgroundGrey + squareGrey) / 2
)
// createSquarePNG creates a grey PNG side pixels square with a darker grey
// square squareSide pixels wide in its centre.
func createSquarePNG(t *testing.T, side, squareSide int) []byte {
t.Helper()
first := (side - squareSide) / 2
last := first + squareSide - 1
img := image.NewGray(image.Rect(0, 0, side, side))
for y := range side {
for x := range side {
grey := uint8(backgroundGrey)
if x >= first && x <= last && y >= first && y <= last {
grey = squareGrey
}
img.SetGray(x, y, color.Gray{Y: grey})
}
}
return encodePNG(t, img)
}
// squareLength returns how long the square is along greys, the greys of a
// row or column of the output that crosses it: the distance between the
// first and the last point where greys passes halfwayGrey, or 0 when there
// is no such point. Resampling blurs the square's edges over several
// pixels, and each point is placed between the two pixels on either side
// of it in proportion to their greys, so the length is measured to a
// fraction of a pixel.
func squareLength(greys []float64) float64 {
first, last := -1.0, -1.0
for i := 1; i < len(greys); i++ {
before, after := greys[i-1], greys[i]
if (before > halfwayGrey) == (after > halfwayGrey) {
continue
}
point := float64(i-1) + (before-halfwayGrey)/(before-after)
if first < 0 {
first = point
}
last = point
}
return last - first
}
// TestImageProcessor_FitCoverKeepsProportions verifies that FitCover scales
// both sides of the source by the same amount: a square in the centre of
// the source comes out square. Its width and height are measured along the
// output's middle row and column.
func TestImageProcessor_FitCoverKeepsProportions(t *testing.T) {
t.Parallel()
tests := []struct {
name string
side, squareSide int
size Size
}{
{"16x16 at 100x60", 16, 8, Size{Width: 100, Height: 60}},
{"16x16 at 60x100", 16, 8, Size{Width: 60, Height: 100}},
{"10x10 at 100x35", 10, 2, Size{Width: 100, Height: 35}},
{"10x10 at 35x100", 10, 2, Size{Width: 35, Height: 100}},
{"64x64 at 300x100", 64, 16, Size{Width: 300, Height: 100}},
{"64x64 at 100x300", 64, 16, Size{Width: 100, Height: 300}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
input := createSquarePNG(t, tt.side, tt.squareSide)
result, err := New(Params{}).Process(t.Context(),
bytes.NewReader(input),
&Request{Size: tt.size, Format: FormatPNG, FitMode: FitCover})
if err != nil {
t.Fatalf("Process() error = %v", err)
}
defer func() { _ = result.Content.Close() }()
output, err := png.Decode(result.Content)
if err != nil {
t.Fatalf("failed to decode output: %v", err)
}
bounds := output.Bounds()
if bounds.Dx() != tt.size.Width || bounds.Dy() != tt.size.Height {
t.Fatalf("Process() made %dx%d, want %dx%d",
bounds.Dx(), bounds.Dy(), tt.size.Width, tt.size.Height)
}
row := make([]float64, bounds.Dx())
for x := range row {
r, _, _, _ := output.At(x, bounds.Dy()/2).RGBA()
row[x] = float64(r >> 8)
}
column := make([]float64, bounds.Dy())
for y := range column {
r, _, _, _ := output.At(bounds.Dx()/2, y).RGBA()
column[y] = float64(r >> 8)
}
width, height := squareLength(row), squareLength(column)
t.Logf("square is %.2f wide and %.2f high", width, height)
if width == 0 || math.Abs(width-height) > 0.5 {
t.Errorf("square is %.2f wide and %.2f high, want it square",
width, height)
}
})
}
}
+34 -2
View File
@@ -475,8 +475,8 @@ func (p *ImageProcessor) resize(
) error {
switch fit {
case FitCover, "":
// Resize and crop to fill exact dimensions (default)
return img.Thumbnail(width, height, vips.InterestingCentre)
// Crop the centre and resize to fill exact dimensions (default)
return cropCentreAndResize(img, width, height)
case FitContain:
// Resize to fit within dimensions, maintaining aspect ratio
@@ -530,6 +530,38 @@ func (p *ImageProcessor) resize(
}
}
// cropCentreAndResize scales img by the same amount on both sides, the
// least that makes it cover width x height, and cuts the centre width x
// height out of it, as libvips' thumbnail does with InterestingCentre.
// thumbnail alone scales the whole image and copies the result into memory
// before it cuts, and for a thin image that is far larger than the image or
// the output: 8192x64 at 8192x8192 is 1048576x8192, about 24 GiB. So the
// centre is cut from img first, widened to whole pixels of img, which adds
// less than one pixel of img on each side, and thumbnail scales and cuts
// only that.
func cropCentreAndResize(img *vips.ImageRef, width, height int) error {
imgW, imgH := img.Width(), img.Height()
// An image wider than the requested proportions keeps its height and
// loses width down to imgH*width/height, any other keeps its width and
// loses height down to imgW*height/width. Half of what is lost comes
// off each side, rounded down to whole pixels, so the part kept holds
// all of the centre.
left, top := 0, 0
if imgW*height > imgH*width {
left = (imgW*height - imgH*width) / (2 * height) //nolint:mnd // each side
} else {
top = (imgH*width - imgW*height) / (2 * width) //nolint:mnd // each side
}
err := img.ExtractArea(left, top, imgW-2*left, imgH-2*top)
if err != nil {
return err
}
return img.Thumbnail(width, height, vips.InterestingCentre)
}
const defaultQuality = 85
// encode encodes an image to the specified format. govips gives pixa no way
+63 -26
View File
@@ -157,10 +157,11 @@ func postGenerate(
}
// TestImageURLFetchesItsSourcePathAndQuery requests image URLs whose source
// query holds a space and whose source path holds a "%", each written
// percent-encoded, and checks the one request the upstream receives: the
// source path percent-encoded again, and the source query with the space
// written %20.
// query holds a space, written percent-encoded, or a "'", and whose source
// path holds a "%", written percent-encoded, and checks the one request the
// upstream receives: the source path percent-encoded again, and the source
// query with the space written %20 and the "'" written %27, as a browser
// writes them in a query.
func TestImageURLFetchesItsSourcePathAndQuery(t *testing.T) {
t.Parallel()
@@ -174,6 +175,8 @@ func TestImageURLFetchesItsSourcePathAndQuery(t *testing.T) {
}{
{"space in the query", "/photo.png%3Fname=my%20cat",
"/photo.png?name=my%20cat"},
{"apostrophe in the query", "/photo.png%3Fname=it's",
"/photo.png?name=it%27s"},
{"percent sign in the path", percentSignPath, percentSignPath},
}
@@ -305,30 +308,64 @@ func TestGeneratedURLFetchesTheSourceURLAsEntered(t *testing.T) {
}
}
// TestGeneratorRefusesSourceURLItWouldFetchAnotherWay submits a source URL
// whose path holds %2F. pixa keeps the path decoded and would fetch it with a
// "/" there, which is another URL, so the generator page answers 400, shows
// the URL pixa would fetch, and makes no encrypted URL.
func TestGeneratorRefusesSourceURLItWouldFetchAnotherWay(t *testing.T) {
// TestGeneratorRefusesSourceURLItCannotFetchAsWritten submits source URLs the
// generator page must make no encrypted URL for, and checks that it answers
// 400 and shows why. pixa keeps the source path decoded and fetches it
// percent-encoded again, so a path holding %2F would be fetched with a "/"
// there, whatever else the path holds. pixa sends no user name or password,
// and no "?" with no query after it. For these the page shows the URL pixa
// would fetch. A source path holding a ".." segment is refused, as
// /v1/image/ refuses it.
func TestGeneratorRefusesSourceURLItCannotFetchAsWritten(t *testing.T) {
t.Parallel()
s, _, _ := startImageProxy(t, startRecordingUpstream(t).Server)
rec := postGenerate(t, s, "https://"+upstreamHost+"/a%2Fb.png")
body := rec.Body.String()
if rec.Code != http.StatusBadRequest {
t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
if strings.Contains(body, "/v1/e/") {
t.Error("the page shows an encrypted URL")
}
// The test config sets allow_http, so pixa would fetch over plain HTTP
fetchURL := "http://" + upstreamHost + "/a/b.png"
if !strings.Contains(body, fetchURL) {
t.Errorf("the page does not show the URL pixa would fetch, %s; body %s",
fetchURL, body)
const fetchedFrom = "http://" + upstreamHost
tests := []struct {
name string
// sourceURL is the source URL after "https://"
sourceURL string
// wantShown is what the page must show: the URL pixa would fetch, or
// why it makes no encrypted URL
wantShown string
}{
{"encoded slash", upstreamHost + "/a%2Fb.png",
fetchedFrom + "/a/b.png"},
{"encoded slash and a space", upstreamHost + "/a%2Fb c.png",
fetchedFrom + "/a/b%20c.png"},
{"encoded slash and a bar", upstreamHost + "/a%2Fb|c.png",
fetchedFrom + "/a/b%7Cc.png"},
{"encoded slash and a letter outside ASCII", upstreamHost + "/café%2Fx.png",
fetchedFrom + "/caf%C3%A9/x.png"},
{"user name and password", "user:pass@" + upstreamHost + "/a.png",
fetchedFrom + "/a.png"},
{"question mark with no query", upstreamHost + "/a.png?",
fetchedFrom + "/a.png"},
{"dot dot segment", upstreamHost + "/a/../b.png",
"cannot hold a .. segment"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
s, _, _ := startImageProxy(t, startRecordingUpstream(t).Server)
rec := postGenerate(t, s, "https://"+tt.sourceURL)
body := rec.Body.String()
if rec.Code != http.StatusBadRequest {
t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
if strings.Contains(body, "/v1/e/") {
t.Error("the page shows an encrypted URL")
}
if !strings.Contains(body, tt.wantShown) {
t.Errorf("the page does not show %s; body %s", tt.wantShown, body)
}
})
}
}