Refuse a size whose computed side exceeds the maximum (closes #241) #258

Merged
clawbot merged 2 commits from issue-241-bound-computed-side into next 2026-10-13 09:44:45 +02:00
8 changed files with 268 additions and 15 deletions
+6 -1
View File
@@ -263,6 +263,10 @@ once, is refused with 400.
both optional (values under Signature Specification). Both are part of what is
cached, so each value of either is a separate cached image.
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.
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.
An upstream answering with any other `Content-Type`, SVG included, is refused
@@ -345,7 +349,8 @@ nor change what it asks for.
(`POST /generate`). The format is JPEG XL unless another is chosen; a form
sent with an empty format, or none, also makes a JPEG XL URL. Width and
height both empty or `0` keep the original size; if only one of them is empty
or `0`, that side is scaled to keep the image's proportions.
or `0`, that side is scaled to keep the image's proportions. A URL whose
scaled side would come out above 8192 answers 400.
3. The page shows the URL, `https://<host>/v1/e/<token>/img.<format>`, and when
it expires. `<host>` is the host the page was opened on, and the URL starts
with `http` instead while `debug` is on. The name after the token is ignored
+9
View File
@@ -30,6 +30,15 @@ P2: security: per-origin rate limiting
# Completed Steps
- 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
3000x600 source was resized to 40960x8192 and encoded. The image processor now
refuses such a size with `ErrOutputTooLarge` once it knows the source's
dimensions, before resizing, and both image routes answer it 400 with the JSON
error body. The refusal is not cached as a failed fetch, so the same source is
still served at other sizes. `imgcache.MaxDimension` is now the image
processor's `MaxOutputDimension`, so the two cannot differ.
- 2026-10-13 the unused cache lifetime is removed (closes #69):
`CacheConfig.CacheTTL`, which pixad set to `DefaultCacheTTL` (24 hours), was
never read, so cached images never expired. The field and the constant are
+8
View File
@@ -255,6 +255,14 @@ func (s *Handlers) respondImageError(
return
}
if errors.Is(err, imageprocessor.ErrOutputTooLarge) {
s.respondError(w, fmt.Sprintf("size too large for the source: the side "+
"computed from its proportions is above %d",
imageprocessor.MaxOutputDimension), http.StatusBadRequest)
return
}
s.respondError(w, "internal error", http.StatusInternalServerError)
}
+4
View File
@@ -165,6 +165,10 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
errors.Is(err, imageprocessor.ErrUnsupportedInputFormat):
s.respondError(w, "unsupported source format",
http.StatusUnsupportedMediaType)
case errors.Is(err, imageprocessor.ErrOutputTooLarge):
s.respondError(w, fmt.Sprintf("size too large for the source: the side "+
"computed from its proportions is above %d",
imageprocessor.MaxOutputDimension), http.StatusBadRequest)
default:
s.log.Error("image request failed", "error", err)
s.respondError(w, "internal error", http.StatusInternalServerError)
@@ -0,0 +1,52 @@
package imageprocessor
import (
"bytes"
"errors"
"testing"
)
// TestImageProcessor_RefusesComputedSideAboveMaximum verifies that a size
// with one side 0, which Process computes from the input's proportions, is
// served when that side comes out at MaxOutputDimension and refused with
// ErrOutputTooLarge when it would be above. A 4096x1 or 1x4096 input given
// 2 for its short side has 8192 for its long side.
func TestImageProcessor_RefusesComputedSideAboveMaximum(t *testing.T) {
t.Parallel()
tests := []struct {
name string
inputWidth, inputHeight int
size Size
wantErr error
}{
{"width at the maximum", 4096, 1, Size{Height: 2}, nil},
{"width above the maximum", 4096, 1, Size{Height: 3}, ErrOutputTooLarge},
{"height at the maximum", 1, 4096, Size{Width: 2}, nil},
{"height above the maximum", 1, 4096, Size{Width: 3}, ErrOutputTooLarge},
}
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 result != nil {
t.Logf("Process() made %dx%d", result.Width, result.Height)
if max(result.Width, result.Height) != MaxOutputDimension {
t.Errorf("Process() made %dx%d, want a side of %d",
result.Width, result.Height, MaxOutputDimension)
}
}
if !errors.Is(err, tt.wantErr) {
t.Errorf("Process() error = %v, want %v", err, tt.wantErr)
}
})
}
}
+31 -12
View File
@@ -133,6 +133,14 @@ var ErrInputTooLarge = errors.New("input image dimensions exceed maximum")
// configured byte limit.
var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size")
// MaxOutputDimension is the maximum width or height of an output image, and
// 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.
var ErrOutputTooLarge = errors.New("output image dimensions exceed maximum")
// ErrUnsupportedOutputFormat is returned when the requested output format is
// not supported.
var ErrUnsupportedOutputFormat = errors.New("unsupported output format")
@@ -259,8 +267,11 @@ func (p *ImageProcessor) Process(
return nil, ErrInputTooLarge
}
// Determine target dimensions
targetWidth, targetHeight := targetDimensions(req.Size, origWidth, origHeight)
// Determine target dimensions; too large ones are refused before resizing
targetWidth, targetHeight, err := targetDimensions(req.Size, origWidth, origHeight)
if err != nil {
return nil, err
}
// Resize if needed
if targetWidth != origWidth || targetHeight != origHeight {
@@ -296,21 +307,29 @@ 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.
func targetDimensions(size Size, origWidth, origHeight int) (int, int) {
// original dimensions when both are zero. A side scaled proportionally can
// come out above MaxOutputDimension, which is ErrOutputTooLarge.
func targetDimensions(size Size, origWidth, origHeight int) (int, int, error) {
width, height := size.Width, size.Height
switch {
case size.Width == 0 && size.Height == 0:
case width == 0 && height == 0:
// Both are 0: keep original size
return origWidth, origHeight
case size.Width == 0:
return origWidth, origHeight, nil
case width == 0:
// Only height specified: calculate width proportionally
return origWidth * size.Height / origHeight, size.Height
case size.Height == 0:
width = origWidth * height / origHeight
case height == 0:
// Only width specified: calculate height proportionally
return size.Width, origHeight * size.Width / origWidth
default:
return size.Width, size.Height
height = origHeight * width / origWidth
}
if width > MaxOutputDimension || height > MaxOutputDimension {
return 0, 0, fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge,
width, height, MaxOutputDimension)
}
return width, height, nil
}
// MIME types for the supported image formats.
+5 -2
View File
@@ -7,6 +7,8 @@ import (
"regexp"
"strconv"
"strings"
"sneak.berlin/go/pixa/internal/imageprocessor"
)
// URL parsing errors.
@@ -20,8 +22,9 @@ var (
ErrPathTraversal = errors.New("path traversal detected")
)
// MaxDimension is the maximum allowed width or height.
const MaxDimension = 8192
// MaxDimension is the maximum allowed width or height, the same as the image
// processor's maximum for an output image.
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.
@@ -0,0 +1,153 @@
package server
import (
"bytes"
"encoding/json"
"image/jpeg"
"net/http"
"net/http/httptest"
"testing"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imgcache"
)
// TestImageProxyRefusesComputedSideAboveMaximum requests sizes of a 3000x600
// and a 600x3000 PNG through both image routes. A size with one side 0, whose
// side computed from the source's proportions would be above 8192, is refused
// with 400 and the JSON error body. The sizes after those refusals are served
// as before, which also shows that a refusal is not cached as a failed fetch of
// the source: orig, a size with both sides given, and a size with one side 0
// whose computed side stays within 8192.
func TestImageProxyRefusesComputedSideAboveMaximum(t *testing.T) {
t.Parallel()
// The upstream paths of the two sources
const (
widePath = "/wide.png"
tallPath = "/tall.png"
)
wide := encodeTestPNG(t, 3000, 600)
tall := encodeTestPNG(t, 600, 3000)
upstream := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "image/png")
if r.URL.Path == tallPath {
_, _ = w.Write(tall)
return
}
_, _ = w.Write(wide)
}))
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
// size is the size of the /v1/image/ URL, and width and height are
// those of the encrypted URL
size string
width, height int
// wantWidth and wantHeight are the served image's, 0 for a refusal
wantWidth, wantHeight int
}{
{widePath, "0x8192", 0, 8192, 0, 0},
{tallPath, "8192x0", 8192, 0, 0, 0},
{widePath, "orig", 0, 0, 3000, 600},
{widePath, "300x200", 300, 200, 300, 200},
{widePath, "0x300", 0, 300, 1500, 300},
{tallPath, "300x0", 300, 0, 300, 1500},
}
for _, tt := range tests {
token, err := generator.Generate(&encurl.Payload{
SourceHost: upstreamHost, SourcePath: tt.path,
Width: tt.width, Height: tt.height, Format: imgcache.FormatJPEG,
})
if err != nil {
t.Fatalf("encurl Generate: %v", err)
}
for _, target := range []string{
"/v1/image/" + upstreamHost + tt.path + "/" + tt.size + ".jpeg",
"/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)
}
}
}
}
// checkSizeRefused checks that rec answers 400 with the JSON error body that
// refuses a size whose side computed from the source's proportions is above
// 8192.
func checkSizeRefused(t *testing.T, rec *httptest.ResponseRecorder) {
t.Helper()
const wantError = "size too large for the source: " +
"the side computed from its proportions is above 8192"
var body struct {
Error string `json:"error"`
Status int `json:"status"`
}
err := json.Unmarshal(rec.Body.Bytes(), &body)
if err != nil {
t.Errorf("status %d, body is not JSON: %v", rec.Code, err)
return
}
if rec.Code != http.StatusBadRequest || body.Status != http.StatusBadRequest ||
body.Error != wantError {
t.Errorf("status %d, body %+v, want %d and the error %q",
rec.Code, body, http.StatusBadRequest, wantError)
}
}
// checkServedSize checks that rec answers 200 with a JPEG of wantWidth by
// wantHeight.
func checkServedSize(
t *testing.T, rec *httptest.ResponseRecorder, wantWidth, wantHeight int,
) {
t.Helper()
if rec.Code != http.StatusOK {
t.Errorf("status %d, want %d; body %s",
rec.Code, http.StatusOK, rec.Body.String())
return
}
served, err := jpeg.DecodeConfig(bytes.NewReader(rec.Body.Bytes()))
if err != nil {
t.Errorf("decoding the served JPEG: %v", err)
return
}
if served.Width != wantWidth || served.Height != wantHeight {
t.Errorf("served %dx%d, want %dx%d",
served.Width, served.Height, wantWidth, wantHeight)
}
}