diff --git a/README.md b/README.md index 9525dcc..a9d14e1 100644 --- a/README.md +++ b/README.md @@ -265,7 +265,9 @@ once, is refused with 400. A size with one side `0` has that side scaled to keep the source's proportions, and is refused with 400 when that side would come out above 8192, the most -either side may be. +either side may be; so is a size with `fit=outside`, which scales the image to +cover the size and so past it on one side, when that side would come out +above 8192. The source image may be JPEG, PNG, GIF, WebP, AVIF or JPEG XL. The upstream's `Content-Type` must name its format, and the image's first bytes must match it. diff --git a/TODO.md b/TODO.md index 2ec0e84..190ecec 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,13 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side + (closes #257): it scales the image to cover the requested size, and the side + scaled past that size was held to no maximum, so `1x8192?fit=outside` of a + 3000x600 source was resized to 40960x8192 and encoded. The image processor now + refuses such a size with `ErrOutputTooLarge` before resizing, and both image + routes answer it 400, as for a size with one side `0`. The other fit modes + make the requested size or one within it, so none can go above 8192. - 2026-10-13 a size with one side `0` no longer makes an output above 8192 on a side (closes #241): that side is computed from the source's proportions, and nothing held it to the 8192 a requested side is held to, so `0x8192` of a diff --git a/internal/imageprocessor/fit_outside_internal_test.go b/internal/imageprocessor/fit_outside_internal_test.go new file mode 100644 index 0000000..ab3737f --- /dev/null +++ b/internal/imageprocessor/fit_outside_internal_test.go @@ -0,0 +1,62 @@ +package imageprocessor + +import ( + "bytes" + "errors" + "testing" +) + +// TestImageProcessor_RefusesFitOutsideAboveMaximum verifies that FitOutside, +// which scales the input to cover the requested size and so past it on one +// side, is refused with ErrOutputTooLarge when that side would come out above +// MaxOutputDimension, and otherwise makes the scaled size. A 1024x256 or +// 256x1024 input scaled by 8 has 8192 for its long side. +func TestImageProcessor_RefusesFitOutsideAboveMaximum(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + inputWidth, inputHeight int + size Size + // wantWidth and wantHeight are the output's, 0 for a refusal + wantWidth, wantHeight int + }{ + {"width above the maximum", 3000, 600, Size{Width: 1, Height: 8192}, 0, 0}, + {"height above the maximum", 600, 3000, Size{Width: 8192, Height: 1}, 0, 0}, + {"width at the maximum", 1024, 256, Size{Width: 1, Height: 2048}, 8192, 2048}, + {"height at the maximum", 256, 1024, Size{Width: 2048, Height: 1}, 2048, 8192}, + {"within the maximum", 3000, 600, Size{Width: 1, Height: 1200}, 6000, 1200}, + } + + 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 result != nil { + t.Logf("Process() made %dx%d", result.Width, result.Height) + } + + if tt.wantWidth == 0 { + if !errors.Is(err, ErrOutputTooLarge) { + t.Errorf("Process() error = %v, want %v", err, ErrOutputTooLarge) + } + + return + } + + if err != nil { + t.Fatalf("Process() error = %v", err) + } + + 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 4ad8007..92cee8a 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -137,8 +137,10 @@ var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size") // so of a requested size (imgcache.MaxDimension). const MaxOutputDimension = 8192 -// ErrOutputTooLarge is returned when a size with one side 0 would give that -// side, computed from the input's proportions, more than MaxOutputDimension. +// ErrOutputTooLarge is returned, before resizing, when the output would have a +// side above MaxOutputDimension: the side of a size with one side 0, computed +// from the input's proportions, or with FitOutside, the side scaled past the +// requested size. var ErrOutputTooLarge = errors.New("output image dimensions exceed maximum") // ErrUnsupportedOutputFormat is returned when the requested output format is @@ -460,7 +462,9 @@ func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string { } } -// resize resizes the image according to the fit mode. +// resize resizes the image according to the fit mode. With FitOutside one +// side can come out larger than requested; when it would be above +// MaxOutputDimension, resize returns ErrOutputTooLarge without resizing. func (p *ImageProcessor) resize( img *vips.ImageRef, width, height int, fit FitMode, ) error { @@ -508,6 +512,12 @@ func (p *ImageProcessor) resize( newW := int(float64(imgW) * scale) newH := int(float64(imgH) * scale) + // The dimension that exceeds the target is held to the maximum too + if newW > MaxOutputDimension || newH > MaxOutputDimension { + return fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge, + newW, newH, MaxOutputDimension) + } + return img.Thumbnail(newW, newH, vips.InterestingNone) default: diff --git a/internal/server/fit_outside_internal_test.go b/internal/server/fit_outside_internal_test.go new file mode 100644 index 0000000..f0ada85 --- /dev/null +++ b/internal/server/fit_outside_internal_test.go @@ -0,0 +1,91 @@ +package server + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/imgcache" +) + +// TestImageProxyRefusesFitOutsideAboveMaximum requests sizes with fit=outside +// through both image routes. fit=outside scales the source to cover the size, +// and so past it on one side: a size for which that side would be above 8192 +// is refused with 400 and the JSON error body, whether it is the width or the +// height, and a size for which it stays within 8192 is served at the scaled +// size. +func TestImageProxyRefusesFitOutsideAboveMaximum(t *testing.T) { + t.Parallel() + + // The upstream paths of the sources + const ( + widePath = "/wide.png" + rowPath = "/row.png" + columnPath = "/column.png" + ) + + sources := map[string][]byte{ + widePath: encodeTestPNG(t, 3000, 600), + rowPath: encodeTestPNG(t, 8192, 1), + columnPath: encodeTestPNG(t, 1, 8192), + } + + upstream := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "image/png") + _, _ = w.Write(sources[r.URL.Path]) + })) + t.Cleanup(upstream.Close) + + s, _, _ := startImageProxy(t, upstream) + + generator, err := encurl.NewGenerator(testSigningKey) + if err != nil { + t.Fatalf("encurl.NewGenerator: %v", err) + } + + tests := []struct { + path string + width, height int + // wantWidth and wantHeight are the served image's, 0 for a refusal + wantWidth, wantHeight int + }{ + {widePath, 1, 8192, 0, 0}, + {rowPath, 1, 8192, 0, 0}, + {columnPath, 8192, 1, 0, 0}, + {widePath, 1, 1200, 6000, 1200}, + } + + for _, tt := range tests { + token, err := generator.Generate(&encurl.Payload{ + SourceHost: upstreamHost, SourcePath: tt.path, + Width: tt.width, Height: tt.height, Format: imgcache.FormatJPEG, + FitMode: imgcache.FitOutside, + }) + if err != nil { + t.Fatalf("encurl Generate: %v", err) + } + + size := fmt.Sprintf("%dx%d", tt.width, tt.height) + + for _, target := range []string{ + "/v1/image/" + upstreamHost + tt.path + "/" + size + + ".jpeg?fit=outside", + "/v1/e/" + token + "/img.jpg", + } { + rec := httptest.NewRecorder() + s.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, target, nil)) + t.Logf("GET %s: %d, Content-Type %s", + target, rec.Code, rec.Header().Get("Content-Type")) + + if tt.wantWidth == 0 { + checkSizeRefused(t, rec) + } else { + checkServedSize(t, rec, tt.wantWidth, tt.wantHeight) + } + } + } +}